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

Response body for redirects not ignored in streaming mode #1165

Open
fredrikekre opened this issue Apr 10, 2024 · 2 comments
Open

Response body for redirects not ignored in streaming mode #1165

fredrikekre opened this issue Apr 10, 2024 · 2 comments
Labels
bug client About our HTTP client

Comments

@fredrikekre
Copy link
Member

fredrikekre commented Apr 10, 2024

HTTP.jl do not ignore the response body in redirects as discovered here: https://discourse.julialang.org/t/unexpected-end-of-data-when-updating-registries-with-localpackageserver/112782. The is introduced in HTTP.jl 1.6 (1.0 - 1.5 is OK) so the bug is in this range: v1.5.5...v1.6.3 which includes a number of patches related to redirects.


Sample server:

server = HTTP.listen!(8123) do http
    if http.message.target == "/200"
        HTTP.setstatus(http, 200)
        HTTP.startwrite(http)
        HTTP.write(http, "This is /200\n")
    else
        HTTP.setstatus(http, 301)
        HTTP.setheader(http, "Location" => "/200")
        HTTP.startwrite(http)
        HTTP.write(http, "Go to /200\n")
    end
    return
end

HTTP.jl:

julia> HTTP.get("http://localhost:8123"; redirect=false, response_stream = stderr);
Go to /200                                                           # <------

julia> HTTP.get("http://localhost:8123"; response_stream = stderr);
Go to /200                                                           # <------
This is /200                                                         # <------

curl:

$ curl -v http://localhost:8123
*   Trying 127.0.0.1:8123...
* Connected to localhost (127.0.0.1) port 8123 (#0)
> GET / HTTP/1.1
> Host: localhost:8123
> User-Agent: curl/7.81.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Location: /200
< Transfer-Encoding: chunked
<
Go to /200                                                           # <------
* Connection #0 to host localhost left intact

$ curl -Lv http://localhost:8123
*   Trying 127.0.0.1:8123...
* Connected to localhost (127.0.0.1) port 8123 (#0)
> GET / HTTP/1.1
> Host: localhost:8123
> User-Agent: curl/7.81.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 301 Moved Permanently
< Location: /200
< Transfer-Encoding: chunked
<
* Ignoring the response-body                                         # <------
* Connection #0 to host localhost left intact
* Issue another request to this URL: 'http://localhost:8123/200'
* Found bundle for host localhost: 0x55d6fea933c0 [serially]
* Can not multiplex, even if we wanted to!
* Re-using existing connection! (#0) with host localhost
* Connected to localhost (127.0.0.1) port 8123 (#0)
> GET /200 HTTP/1.1
> Host: localhost:8123
> User-Agent: curl/7.81.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Transfer-Encoding: chunked
<
This is /200                                                         # <------
* Connection #0 to host localhost left intact
@fredrikekre fredrikekre added bug client About our HTTP client labels Apr 10, 2024
@quinnj
Copy link
Member

quinnj commented Apr 13, 2024

Ok, so you're saying the issue is that in the 2nd case, both the redirect body and the redirected body are both written to the response_stream? Yeah, I agree we don't want that behavior.

I've put up #1168 for now to see how CI looks; not sure if it may unintentionally break something else.

@fredrikekre
Copy link
Member Author

Yea sorry if the description wasn't so clear. Basically, only the last body should be written.

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

No branches or pull requests

2 participants