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 #12397 - Add more default excludes to GzipHandler. #12414

Merged
merged 2 commits into from
Oct 23, 2024

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Oct 22, 2024

  • Exclude more compressed mime-types
  • Exclude more compressed path extensions
  • Add missing compressed file types to mime.properties

Fixes: #12397

+ Exclude more compressed mime-types
+ Exclude more compressed path extensions
+ Add missing compressed file types to mime.properties
@joakime joakime added the Bug For general bugs on Jetty side label Oct 22, 2024
@joakime joakime requested review from gregw and janbartel October 22, 2024 14:08
@joakime joakime self-assigned this Oct 22, 2024
@joakime
Copy link
Contributor Author

joakime commented Oct 22, 2024

@gregw with this change, I see yet another thing that MimeTypes should probably do (flag if a mime-type represents compressed content that shouldn't be recompressed).

What do you think about refactoring the MimeTypes configuration files a great deal?

We could use json, and our jetty-util-ajax lib ...

[
  "application/gzip" : {
    "extensions": [ ".gz", ".gzip" ],
    "assumedCharset": "",
    "inferredCharset": ""
    "ignoreCharset": "true"
    "compressedType": "true"
  },
  "application/json" : {
    "extensions": [ ".json" ],
    "assumedCharset": "utf-8",
    "inferredCharset": ""
    "ignoreCharset": "false"
    "compressedType": "false"
  }
]

The idea is that this becomes our preferred configuration file, but we still allow the use of the older mime.properties and encoding.properties as an alternate deprecated configuration technique.

_mimeTypes.exclude("application/compress");
_paths.exclude("*.z");
Copy link
Contributor

Choose a reason for hiding this comment

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

That seems a very broad brush stroke. Also I don't see this mimetype listed on https://www.iana.org/assignments/media-types/media-types.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was taken from our mime.properties file.

Copy link
Contributor

Choose a reason for hiding this comment

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

yikes

@janbartel janbartel self-requested a review October 22, 2024 22:42
@joakime joakime merged commit 92302d6 into jetty-12.0.x Oct 23, 2024
10 checks passed
@joakime joakime deleted the fix/12.0.x/gziphandler-tgz branch October 23, 2024 02:53
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
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

.tgz files are double-gzipped
2 participants