Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When following redirects, I'd like to be able to access the final location #90

Open
QuinnWilton opened this issue Nov 4, 2015 · 13 comments · May be fixed by #96
Open

When following redirects, I'd like to be able to access the final location #90

QuinnWilton opened this issue Nov 4, 2015 · 13 comments · May be fixed by #96

Comments

@QuinnWilton
Copy link
Contributor

As it stands now, following a redirect only returns the final response, and there is no way to determine what URL that response maps to. For example, the following call hits tinfoil-fake-site.com, but follows a redirect, and ends up on https://tinfoilsecurity.com

HTTPoison.request(:get, "tinfoil-fake-site.com", "", [], [hackney: [{:follow_redirect, true}]])

Since the final response does not have a location header (As it shouldn't!), we have no way of determining what page we ended up on.

The Hackney documentation claims that the "Last Location is stored in the location property of the client state," but I don't believe we have access to this from HTTPoison.

I'll let you know if I find a way of accessing this information. I'm also happy to contribute a fix upstream if you have any ideas on a possible solution!

@QuinnWilton
Copy link
Contributor Author

From IRC, benoitrc suggests exposing https://github.com/benoitc/hackney/blob/master/doc/hackney.md#location-1

@edgurgel
Copy link
Owner

edgurgel commented Nov 8, 2015

That's a great addition to the library.

I think we got 2 options:

  • Add another key to the struct (location probably) and populate it if the client has a location to provide.
  • Add the hackney client state that is returned back to the struct and a function to HTTPoison.Response that returns the location. This would work for other cases (if we need to) but it's more complicated to interact with.

Thoughts, ideas?

@QuinnWilton
Copy link
Contributor Author

Your first option is in line with what I had in mind, but I think an
argument could be made for also including the client itself.

You already support passing options to Hackney directly, to handle cases
where you haven't wrapped an option, so I think it makes sense to expose
the client for the same reason.

I'm happy to look into implementing either of these approaches if you have
a lot on your plate.

On Sat, Nov 7, 2015, 21:49 Eduardo Gurgel [email protected] wrote:

That's a great addition to the library.

I think we got 2 options:

  • Add another key to the struct (location probably) and populate it if
    the client has a location to provide.
  • Add the hackney client state that is returned back to the struct and
    a function to HTTPoison.Response that returns the location. This would work
    for other cases (if we need to) but it's more complicated to interact with.

Thoughts, ideas?


Reply to this email directly or view it on GitHub
#90 (comment).

@edgurgel
Copy link
Owner

edgurgel commented Nov 8, 2015

Yeah the former option seems good short-term but the latter seems better long-term as we won't stop people from using other hackney features.

I would be happy to accept a PR with this change! :)

Thank you very much 👍

@QuinnWilton
Copy link
Contributor Author

Awesome! I should have some free time to work on this later this week. Thanks for the guidance.

@QuinnWilton
Copy link
Contributor Author

@edgurgel In the cases where no client is returned do we want to default to the request_url when setting the response location? It'd be nice to keep the API consistent in that regard, and never return a nil location.

EDIT:

Having spoken to @benoitc in IRC, I believe the only cases where a client will not be returned are when making a HEAD request, and when using the with_body option. These are cases where the request_path may still differ, so returning nil is probably safest.

@QuinnWilton QuinnWilton linked a pull request Nov 11, 2015 that will close this issue
@sheldonkreger
Copy link

I am trying to access the body of the response after following a redirect, something like this:

case HTTPoison.get(url, [], [follow_redirect: true, max_redirect: 5]) do
  {:ok, %HTTPoison.Response{status_code: 302, body: body}} ->
    IO.puts("hit 302")
    IO.puts("body: ")
    IO.puts(body)

However, body is empty. Is this related to the issue you are discussing here? Or is there already a way to access the body after a redirect occurs?

@edgurgel
Copy link
Owner

Can you check the headers and see if there's a "location" to follow? I think there's still another redirect to follow as you got a 302 even after 5 redirects.

Also which version of HTTPoison are you using? Just the latests ones accept these options without being part of hackney key.

@sheldonkreger
Copy link

Here are the versions of the dependencies in my project:
certifi: v0.1.1
floki: v0.6.1
hackney: v1.4.4
httpoison: v0.8.0
idna: v1.0.2
mimerl: v1.0.0
mochiweb: v2.12.2
ssl_verify_hostname: v1.0.5

Anyway, I was performing the get on my personal blog homepage, which indeed has a redirect to GitHub pages. However, it is not giving 302 consistently, and I have only gotten 200 tonight! I will try again tomorrow and share the headers here. I wonder if their CDN might create a bunch of redirects if a cache isn't hit right away.

@sheldonkreger
Copy link

I wasn't able to hit the 302 as I did last night, even after setting the redirect limit to 1. I will keep trying but for now I think I see how this is supposed to work. I'm assuming that if I hit a 302, there will not be any response body. Instead, I should follow redirects until I get 200.

@JosephHalter
Copy link

+1

@ivarvong
Copy link

Here's a workaround:

  def follow(url) when is_binary(url) do
    case HTTPoison.get(url) do
      {:ok, %HTTPoison.Response{status_code: status_code, headers: headers}} when status_code > 300 and status_code < 400 ->
        case get_location_header(headers) do
          [url] when is_binary(url) ->
            follow(url)
          _ ->
            {:error, :no_location_header}
        end

      {:ok, %HTTPoison.Response{status_code: 200}} ->
        {:ok, url}

      reason ->
        {:error, reason}
    end
  end

  defp get_location_header(headers) do
    for {key, value} <- headers, String.downcase(key) == "location" do
      value
    end
  end

@ndarilek
Copy link

Was this change reverted? Just ran a test, and I'm not getting the final URL of a redirect. I see there's a workaround, but I'm confused as to why one would be necessary if this change landed.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants