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

GzipHandler Vary head should be configurable #5171

Closed
joakime opened this issue Aug 19, 2020 · 2 comments · Fixed by #5196
Closed

GzipHandler Vary head should be configurable #5171

joakime opened this issue Aug 19, 2020 · 2 comments · Fixed by #5196
Assignees
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement

Comments

@joakime
Copy link
Contributor

joakime commented Aug 19, 2020

Jetty version
9.4.31

Description
The current implementation of the GzipHandler produces a Vary response header.
It only has 2 possible values depending on User-Agent configuration.

  • Vary: Accept-Encoding, User-Agent (if there are User-Agent configurations present in the GzipHandler)
  • Vary: Accept-Encoding (if there are no User-Agent configurations present in the GzipHandler)

There has been an expressed interest in being able to configure the Vary header.

At minimum, the ability to turn off the User-Agent portion, even if the User-Agent configurations are present on the GzipHandler.

But it might make sense to just make that field entirely configurable with any value.

Perhaps even allowing a null value to mean don't produce the header at all.

@joakime joakime added Enhancement Sponsored This issue affects a user with a commercial support agreement labels Aug 19, 2020
@gregw
Copy link
Contributor

gregw commented Aug 24, 2020

I also wonder if it is time to just drop the default configuration to exclude MSIE6. It is beyond wasteful to make literally billions of checks for user agent when that is only necessary to help an ancient broken browser.

I'm not sure that just lying about the user-agent pattern is the correct thing to do?

@gregw
Copy link
Contributor

gregw commented Aug 24, 2020

Alternately, in jetty-10, we could just entirely remove the User-Agent handling from GzipHandler. Instead we could have an optional rewrite and/or customiser that removes Accept-Encoding from MSIE6.

We already have MsieSslRule, so in 10, let's replace that with MsieRule that does both the accept encoding fiddle and the ssl keep alive fiddle. Perhaps we should then have a msie module that depends on rewrite and mixes in this rule.

@gregw gregw self-assigned this Aug 24, 2020
gregw added a commit that referenced this issue Aug 24, 2020
+ Remove User-Agent handling from GzipHandler
+ Allow Vary header to be set
+ Create rewrite MsieRule to remove Accept-Encoding from IE<=6

Signed-off-by: Greg Wilkins <[email protected]>
@gregw gregw linked a pull request Aug 24, 2020 that will close this issue
gregw added a commit that referenced this issue Aug 26, 2020
 + improved comments

Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Aug 26, 2020
* Issue #5171 Simplify GzipHandler user-agent handling

+ Remove User-Agent handling from GzipHandler
+ Allow Vary header to be set
+ Create rewrite MsieRule to remove Accept-Encoding from IE<=6

Signed-off-by: Greg Wilkins <[email protected]>

* + Full implementation of HttpFields ensure
 + use for Vary field

* + fixed checkstyle

* + fixed test for merged header

* + fixed javadoc

* Issue #5171 Simplify GzipHandler user-agent handling

 + improved comments

Signed-off-by: Greg Wilkins <[email protected]>

* rename and testing after review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants