You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "Long9725 (via GitHub)" <gi...@apache.org> on 2025/11/01 02:14:58 UTC
[PR] Add configurable compression [tomcat]
Long9725 opened a new pull request, #914:
URL: https://github.com/apache/tomcat/pull/914
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Add configurable compression [tomcat]
Posted by "Long9725 (via GitHub)" <gi...@apache.org>.
Long9725 commented on PR #914:
URL: https://github.com/apache/tomcat/pull/914#issuecomment-4187121059
Apologies for the delayed response — I missed this review earlier. Thank you for the thoughtful feedback.
I agree that we should look beyond gzip and move toward a more generic compression approach. Supporting additional algorithms such as Zstandard was also one of the motivations behind this work. My intention was to approach this incrementally in two stages:
1. Improving gzip configuration support as a small and predictable step
2. Moving toward a more generic design to support multiple compression algorithms as a larger follow-up change
I split it this way because the first step felt relatively contained, while the second seemed likely to require broader architectural discussion and potentially significant changes depending on the preferred direction. Given your feedback, would you prefer this PR to remain focused on the incremental improvements, with the broader compression design handled separately, or should this PR evolve toward a more generic compression architecture?
Separately, regarding CompressionConfig — one reason I made changes there is that it is also used by the Spring Boot Tomcat integration. While investigating how to make these options configurable from Spring Boot, I found that Tomcat first needs to expose the relevant configuration properties before they can be surfaced on the Spring side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Add configurable compression [tomcat]
Posted by "rmaucher (via GitHub)" <gi...@apache.org>.
rmaucher commented on PR #914:
URL: https://github.com/apache/tomcat/pull/914#issuecomment-4197955718
Personally I think this is way overengineered. The only real benefit is to be able to avoid picking up any feature requests in that area, since "this is pluggable".
Background: I came up with these filters a long time ago. I thought at the time that maybe more would appear or something, but this did not materialize (that was the most likely outcome to be honest). Nobody said back then this was too complicated and I should simply fully hardcode the two actually useful ones, but there was always the excuse of "what if".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Add configurable compression [tomcat]
Posted by "Long9725 (via GitHub)" <gi...@apache.org>.
Long9725 commented on PR #914:
URL: https://github.com/apache/tomcat/pull/914#issuecomment-4191598136
I considered splitting this work into two steps, but that would force two released versions and require downstream modules to bump versions twice. To avoid that churn, I applied your feedback and implemented the general approach in one pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Add configurable compression [tomcat]
Posted by "Long9725 (via GitHub)" <gi...@apache.org>.
Long9725 commented on PR #914:
URL: https://github.com/apache/tomcat/pull/914#issuecomment-4289513779
> Personally I think this is way overengineered. The only real benefit is to be able to avoid picking up any feature requests in that area, since "this is pluggable".
>
> Background: I came up with these filters a long time ago. I thought at the time that maybe more would appear or something, but this did not materialize (that was the most likely outcome to be honest). Nobody said back then this was too complicated and I should simply fully hardcode the two actually useful ones, but there was always the excuse of "what if".
Thanks for the feedback.
I agree that this approach introduces abstraction before there is a concrete second implementation. My intention was to make it easier to support additional compression algorithms in the future, but I understand that this may be premature at this stage.
My main goal is to support multiple compression algorithms (such as Zstandard or Brotli), rather than introducing a pluggable structure upfront.
For additional codecs like Zstandard, I would prefer to explore them in a separate PR, starting with a straightforward codec-specific implementation. If multiple implementations emerge, we can then revisit whether a more generic structure is warranted.
If supporting additional codecs like Zstandard is not something Tomcat is aiming for, that would also be helpful to understand.
Please let me know if this direction aligns better with the preferred approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Add configurable compression [tomcat]
Posted by "Long9725 (via GitHub)" <gi...@apache.org>.
Long9725 commented on PR #914:
URL: https://github.com/apache/tomcat/pull/914#issuecomment-4191619075
What I changed
## Pluggable factories
Introduce a list of OutputFilterFactory with default auto-registration of gzip; add `GzipOutputFilterFactory` with level and bufferSize tunables.
- Implementation note: Http11OutputBuffer allocates activeFilters = new OutputFilter[filterLibrary.length + 1] to allow one dynamic filter (e.g., negotiated compression) in addition to all library filters.
## Compression negotiation
Add quality-aware selection for TE and Accept-Encoding (highest q wins; wildcard supported; q=0 excluded; ties resolved by server priority/index; case-insensitive).
- Processors: Integrate negotiation in HTTP/1.1 and HTTP/2; install negotiated filter dynamically; set headers correctly (Transfer-Encoding for TE, Content-Encoding for Accept-Encoding); mark content length unknown when compressing.
- EncodingNegotiator (new): Extract negotiation into a public utility with clear rules and Javadoc; CompressionConfig now delegates directly to it.
## ETC
- Tests: Add TestEncodingNegotiator for rule-focused unit tests; keep end-to-end negotiation/headers in
TestCompressionConfig; remove duplicated negotiation tests; fix Accept-Encoding typo and prefer getHeader() for
TE assertions.
- Docs: Update webapps/docs/config/http.xml to document factory-based gzip config and noCompressionEncodings; fix small typos and wrap long lines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Add configurable compression [tomcat]
Posted by "Long9725 (via GitHub)" <gi...@apache.org>.
Long9725 commented on PR #914:
URL: https://github.com/apache/tomcat/pull/914#issuecomment-4289511661
Thanks for the feedback.
I agree that this approach introduces abstraction before there is a concrete second implementation. My intention was to make it easier to support additional compression algorithms in the future, but I understand that this may be premature at this stage.
My main goal is to support multiple compression algorithms (such as Zstandard or Brotli), rather than introducing a pluggable structure upfront.
For additional codecs like Zstandard, I would prefer to explore them in a separate PR, starting with a straightforward codec-specific implementation. If multiple implementations emerge, we can then revisit whether a more generic structure is warranted.
If supporting additional codecs like Zstandard is not something Tomcat is aiming for, that would also be helpful to understand.
Please let me know if this direction aligns better with the preferred approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org