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

https connections dropped from certain client libs #8020

Closed
jcsp opened this issue Jan 4, 2023 · 2 comments
Closed

https connections dropped from certain client libs #8020

jcsp opened this issue Jan 4, 2023 · 2 comments
Labels
area/admin-api kind/bug Something isn't working sev/medium Bugs that do not meet criteria for high or critical, but are more severe than low.

Comments

@jcsp
Copy link
Contributor

jcsp commented Jan 4, 2023

It has been observed that between v22.2.x and v22.3.x, requests to our HTTP endpoints (admin api, pandaproxy) made with python requests continue to work, but when made with urllib.request they fail.

One difference between these libs is that requests defaults to Connection: keep-alive, while urllib.urlopen defaults to Connection: close. They also send different Accept and Accept-Encoding headers by default.

If we compare the version of seastar used in v22.2.x with the one in v22.3.x...

git diff bdf4c58630f06aaffa02c98edc1192a2d8db2fe8..vectorizedio/v22.3.x src/http/

There are changes to handling of Connection header.

The upstream commits are

commit dd9577d7e03e33da6337542b2493445ce774df1f
Author: Jianyong Chen <baluschch@gmail.com>
Date:   Wed Aug 10 04:47:22 2022 +0800

    httpd: move the logic of keepalive to a separate method.
    
    For better readability.
    
    Signed-off-by: Jianyong Chen <baluschch@gmail.com>
    Message-Id: <20220809204722.18439-2-baluschch@gmail.com>

commit c14f9315dc9a1a1f21f2ff32499b7d3fee0ecf55
Author: Jianyong Chen <baluschch@gmail.com>
Date:   Wed Aug 10 04:47:20 2022 +0800

    httpd: compare the Connection header value in a case-insensitive manner.
    
    As per RFC 9110(section 7.6.1), connection options are case-insensitive.
    
    Signed-off-by: Jianyong Chen <baluschch@gmail.com>
    Message-Id: <20220809204722.18439-1-baluschch@gmail.com>
@jcsp jcsp added kind/bug Something isn't working sev/medium Bugs that do not meet criteria for high or critical, but are more severe than low. area/admin-api labels Jan 4, 2023
jcsp added a commit to jcsp/redpanda that referenced this issue Jan 4, 2023
@jcsp
Copy link
Contributor Author

jcsp commented Jan 4, 2023

See:

I think we are hitting a long standing seastar bug where "Connection: Close" never worked for TLS-wrapped http requests. When we seen close, the read fiber closes the input stream, but TLS sockets close both sides when either is closed, so the response then cannot be sent.

Non-TLS connections do not have the same limitation, which is why this issue only comes up when using TLS.

@jcsp
Copy link
Contributor Author

jcsp commented Jan 4, 2023

Reverting the fixes to Connection header parsing works in practice + is what we will do for v22.3.x, but maybe we can fix this more cleanly upstream for v23.1.x: scylladb/seastar#1391

@jcsp jcsp closed this as completed Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admin-api kind/bug Something isn't working sev/medium Bugs that do not meet criteria for high or critical, but are more severe than low.
Projects
None yet
Development

No branches or pull requests

1 participant