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

Fixes #166 [router] Return all response headers unmodified #167

Merged

Conversation

romanwozniak
Copy link
Contributor

@romanwozniak romanwozniak commented Feb 14, 2022

Context:

Issue: #166

Summary:

  • Knative adds the Accept-Encoding: gzip header to each proxied request in all versions before 0.21.x (this behaviour was changed in v0.21: Avoid adding gzip-encoding to requests implicitly knative/serving#10691)
  • Becasue if this, even if a client has sent a request without asking to compress the response, the requests to all internal components (such as routes, enricher, ensembler) will be made with Accept-Encoding: gzip, and if any of these components supports compression – the response will be gzipped
  • Before sending the response back to the client, Knative looks if the request is compressed (response headers contain Content-Encoding: gzip) and decodes it accordingly
  • But Turing router doesn't propagate headers from the ensembler, hence Content-Encoding: gzip is missing in the response that the router sends back to Knative's queue-proxy. And subsequently, Knative sends the response back to the client unmodified (i.e. compressed)

Solution:

In order to avoid such an unexpected behavior as returning a compressed response when the client doesn't request it, Turing router needs to let queue-proxy know if the response is compressed (i.e. propagate Content-Encoding header), so queue-proxy can handle it accordingly and decode the response before sending it to the client.
Currently, Turing router strips all of the response headers except Content-Type. This PR fixes this by propagating all of the responses back to the queue-proxy (and subsequently – the client).

@romanwozniak romanwozniak added the type: bug Something isn't working label Feb 14, 2022
@romanwozniak romanwozniak requested a review from a team February 14, 2022 07:42
@romanwozniak romanwozniak self-assigned this Feb 14, 2022
@romanwozniak romanwozniak linked an issue Feb 14, 2022 that may be closed by this pull request
@romanwozniak romanwozniak changed the title Fixes #166: Return all response headers unmodified Fixes #166 [router] Return all response headers unmodified Feb 14, 2022
Copy link
Collaborator

@krithika369 krithika369 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One (or 3) line PRs are the best. Thanks for the detailed explanation on the root cause. LGTM.

@romanwozniak romanwozniak merged commit 51d407e into caraml-dev:main Feb 15, 2022
@romanwozniak romanwozniak deleted the issue_166_pass_response_headers branch February 15, 2022 01:54
@ankurs
Copy link

ankurs commented Feb 15, 2022

@krithika369 @romanwozniak
I think I probably missed something, This PR is not going to prevent #166
clients will continue to receive compressed responses even when they do not request for it right ?

@romanwozniak
Copy link
Contributor Author

romanwozniak commented Feb 15, 2022

@krithika369 @romanwozniak I think I probably missed something, This PR is not going to prevent #166 clients will continue to receive compressed responses even when they do not request for it right ?

Hey @ankurs, no, that's incorrect. Client will only receive compressed responses when they request for it by passing Accept-Encoding: gzip. The reason is that Turing routers are deployed as Knative services, and each request to Turing router is proxied through Knative's queue-proxy which controls the autoscaling behaviour and collects metrics. In the older versions of Knative, this queue-proxy adds Accept-Encoding: gzip header to each subsequent request (no matter if it was requested by the client) and decodes the received response after it (if the client doesn't support compression) or returns it unmodified (if client supports compression).

So if the client sends the request to turing-router with no Accep-Encoding: gzip, this router's queue-proxy will add such header automatically, but will decode any compressed responses from the downstream services, before sending it back to the client.

In addition to what is already described in this PR's description, you can also check knative/serving#10691 to understand the issue better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in handling Accept-Encoding
3 participants