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

Issue #6544 - Fixing broken jetty.gzip.excludedMimeTypeList property support #6550

Merged
merged 6 commits into from Jul 30, 2021

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Jul 27, 2021

  • Adding GzipHandler tests
  • Adding Gzip module tests
  • Updating jetty-gzip.xml for
    includedMimeTypesList and
    excludedMimeTypesList behavior
  • Adding GzipHandler support for
    setIncludedMimeTypesList(String) and
    setExcludedMimeTypesList(String

Signed-off-by: Joakim Erdfelt joakim.erdfelt@gmail.com

…y support

+ Adding GzipHandler tests
+ Adding Gzip module tests
+ Updating jetty-gzip.xml for
  includedMimeTypesList and
  excludedMimeTypesList behavior
+ Adding GzipHandler support for
  setIncludedMimeTypesList(String) and
  setExcludedMimeTypesList(String

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime added the Bug For general bugs on Jetty side label Jul 27, 2021
@joakime joakime requested review from gregw and sbordet July 27, 2021 16:37
@joakime joakime self-assigned this Jul 27, 2021
@joakime joakime added this to In progress in Jetty 10.0.7/11.0.7 FROZEN via automation Jul 27, 2021
@joakime
Copy link
Contributor Author

joakime commented Jul 27, 2021

Note: this PR does not need to be backported to 9.4.x, as 9.4.x does not support a property to include/exclude by mimetype.

gregw
gregw previously approved these changes Jul 30, 2021
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

I don't love it, but it follows a pattern set by other methods in the class.

@@ -18,8 +18,8 @@
<Set name="dispatcherTypes" property="jetty.gzip.dispatcherTypes"/>
<Set name="includedMethodList" property="jetty.gzip.includedMethodList"/>
<Set name="excludedMethodList" property="jetty.gzip.excludedMethodList"/>
<Set name="includedMimeTypes" property="jetty.gzip.includedMimeTypeList"/>
<Set name="excludedMimeTypes" property="jetty.gzip.excludedMimeTypeList"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

If these methods did not exist, then the changes from #6398 should have detected that???

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah they did exist, they just had the wrong signature.

<Set name="includedMimeTypes" property="jetty.gzip.includedMimeTypeList"/>
<Set name="excludedMimeTypes" property="jetty.gzip.excludedMimeTypeList"/>
<Set name="includedMimeTypesList" property="jetty.gzip.includedMimeTypeList"/>
<Set name="excludedMimeTypesList" property="jetty.gzip.excludedMimeTypeList"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have preferred to split the string in the XML, but we can't if we maintain the property attribute ability to not set if not set.

Jetty 10.0.7/11.0.7 FROZEN automation moved this from In progress to Reviewer approved Jul 30, 2021
Jetty 10.0.7/11.0.7 FROZEN automation moved this from Reviewer approved to Review in progress Jul 30, 2021
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime requested a review from sbordet July 30, 2021 12:33
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Jetty 10.0.7/11.0.7 FROZEN automation moved this from Review in progress to Reviewer approved Jul 30, 2021
@joakime joakime merged commit 409a2fc into jetty-10.0.x Jul 30, 2021
Jetty 10.0.7/11.0.7 FROZEN automation moved this from Reviewer approved to Done Jul 30, 2021
@joakime joakime deleted the jetty-10.0.x-6544-gziphandler-excludedMimeTypes branch August 2, 2021 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Using jetty.gzip.excludedMimeTypeList property results in an error
3 participants