fix(aws-lambda) strip Connection header if client is using HTTP/2 #4032
Conversation
We currently access Lambda over HTTP/1.1, and it returns a Connection header as such. The Connection header is not used in HTTP/2 (it is superseded by other protocol features) and including it violates RFC 7540 8.1.2.2. Per the same "an intermediary transforming an HTTP/1.x message to HTTP/2 will need to remove any header fields nominated by the Connection header field, along with the Connection header field itself."
|
Previously #3481, which did not like my attempt to rebase. The current version of the plugin no longer uses At present we lack an HTTP/2 client library in the test suite, so this does not include new tests. |
|
Thanks @rainest. Before I check myself, have you researched (i.e. |
|
As far as I know, this should only apply when lua-resty-http is used to fetch content rather than simply proxying the request--it'd be very apparent if it applied to regular proxying as no proxied HTTP/2 requests would work. We use lua-resty-http in the Lambda, Galileo, OIDC, Azure Functions, Zipkin, and Forward Proxy. Of those, I think only Azure and Forward Proxy are likely affected by the same issue, as the others are using lua-resty-http to send data to an external service, and don't affect the response returned to clients. Edit: Forward Proxy probably isn't affected as it uses lua-resty-http's proxy_response function, which includes its own mechanism to ignore the upstream Connection header. |
|
@rainest Thanks for tracking them down. It indeed looks like Azure would be guilty of this as well (https://github.com/Kong/kong-plugin-azure-functions/blob/master/kong/plugins/azure-functions/handler.lua#L118). Will you be able to submit a fix for it as well? |
|
No, bug fixes are to be submitted against the |
Remove additional headers mentioned by RFC 7540 8.1.2.2. Lambda does not currently emit these in my tests, and the RFC indicates it's not strictly necessary to strip them unless they're nominated by Connection, but their presence will break some HTTP/2 implementations (libcurl and Chrome at least, possibly others), so removing to future-proof against Lambda adding them in the future.
| headers["Proxy-Connection"] = nil | ||
| headers["Upgrade"] = nil | ||
| headers["Transfer-Encoding"] = nil | ||
| end |
There was a problem hiding this comment.
Note for future reference: https://tools.ietf.org/html/rfc7540#section-8.1.2.2
thibaultcha
added
the
pr/changes requested
|
|
rainest
force-pushed
the
fix/lambda-http2
branch
from
97a6361 to
c73d4cd
Compare
Summary
Strip Connection headers from AWS Lambda responses if client is using HTTP per RFC 7540
Issues resolved
Fix #2991