301 permanent redirect not being followed

[reggiejones7]

Hi, We are in the process of upgrading our http servers from Goliath to Falcon. In the meantime we've ran into an issue where the underlying http client for Goliath accepts a redirects param for number of network hops, but the http client in async-http doesn't seem to have the same optionality. Further, it won't follow a 301 redirect at all. This results in an error as we try to parse the response body, but the body is empty because it was a 301 response.

I've looked through docs and can't find anything. Is it possible I'm missing an option that's passed into the http client? The closest thing I can find is the following commit, but can't seem to hit the code in a debugger. Which makes me think the client is either not configured correctly, OR that RelativeLocation is for internal redirects with the same origin instead of external redirects returned from a 3rd party server.

Below is debugger output of a 301 example:

Any help would be greatly appreciated! Thank you so much :)

[ioquatix]

Hi, thanks for your report, I believe we have a client middleware that can do this for you, let me check...

[ioquatix]

Which makes me think the client is either not configured correctly, OR that RelativeLocation is for internal redirects with the same origin instead of external redirects returned from a 3rd party server.

Ah yes, there can be security implications for cross-origin redirects. Imagine someone hacked the service and you ended up sending the request to a completely different endpoint without realising, etc.

[reggiejones7]

Valid point. :) Thank you for the response, I appreciate it. We are convinced.

Would you be able to link an example of redirect middleware in case we need to whitelist valid redirect origins in the future? If you don't have it handy that is okay, we can derive it when we need it. But if you've already written one I'd love to piggy back or use it as a reference!

[jakeonfire]

i was able to add the middeware via config/initializers/falcon.rb:

require "async/http/relative_location"

class Async::HTTP::Internet
  module WithRedirectMiddeware
    def make_client(endpoint)
      Async::HTTP::RelativeLocation.new(super)
    end
  end

  prepend WithRedirectMiddeware
end

but, as you stated, it only follows relative redirects and i don't see a way to configure it otherwise.

[ioquatix]

Following absolute redirects is extremely risky in general, so I'd avoid doing it by default.

[ioquatix]

@reggiejones7 how are you going with the upgrade?

[jakeonfire]

do you think it should follow absolute redirects when the hostname matches? e.g. https://ifttt.com/privacy returns 301 with Location: https://ifttt.com/terms, which the middleware won't follow.

[ioquatix]

I agree, maybe we can follow a higher standard than just my rambling.

What about: https://datatracker.ietf.org/doc/html/rfc6454

If we can agree that it's a suitable trust model for client side redirects, we can assume it's the model we want to implement and then add some default handling into async-http.

Regarding your specific ask @jakeonfire it seems like that would fall under the trust model described in the above RFC. WDYT?

[jakeonfire]

yes, that looks like a good model for redirects. so, if i'm understanding, always follow relative redirects, or follow absolute redirects if the Origin header lists the destination hostname? would be nice to have a better way to include middewares as well.

[ioquatix]

I think in this case, we have to be mindful of whether the server is a hostile actor. Let's say you connect and hit a MITM but only able to do 301 redirects, they could send you to a different server, e.g. you might end up posting data to the wrong place. So, that's what I'm thinking about.