Re: Refactoring heads up
On Fri, Apr 26, 2024 at 7:20 PM Mark Thomas wrote: > > On 24/04/2024 17:52, Mark Thomas wrote: > > > > > My plan is to commit these changes to 11.0.x with the low risk parts > > (e.g. new methods) back-ported. Then, once we can see what is left, we > > can decide how quickly/slowly we want to back-port the complete fix to > > 10.1.x and 9.0.x (the issue was reported against 10.1.x). > > All is looking good so far. > > The complete refactoring has been applied to 11.0.x > > 10.1.x and 9.0.x have the new header parser and are using it for the > ChunkedInputFilter. > > The question is how long do we want to wait before back-porting the > standard HTTP header parsing? Essentially this means back-porting this > commit: > > https://github.com/apache/tomcat/commit/e5acf2cf0f745350c85d81532826d92b1882469a > > Thoughts? Nice overall. You can wait a bit just in case ... I was not aware that non blocking was not working there. Rémy > I'm thinking wait at least one release cycle before back-porting just in > case of regressions given that this affects every request. > > Mark > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Refactoring heads up
Mark, On 4/26/24 13:17, Mark Thomas wrote: On 24/04/2024 17:52, Mark Thomas wrote: My plan is to commit these changes to 11.0.x with the low risk parts (e.g. new methods) back-ported. Then, once we can see what is left, we can decide how quickly/slowly we want to back-port the complete fix to 10.1.x and 9.0.x (the issue was reported against 10.1.x). All is looking good so far. The complete refactoring has been applied to 11.0.x 10.1.x and 9.0.x have the new header parser and are using it for the ChunkedInputFilter. The question is how long do we want to wait before back-porting the standard HTTP header parsing? Essentially this means back-porting this commit: https://github.com/apache/tomcat/commit/e5acf2cf0f745350c85d81532826d92b1882469a Thoughts? I'm thinking wait at least one release cycle before back-porting just in case of regressions given that this affects every request. +1 for waiting until next cycle to back-port. I don't think we have to wait any longer than that. -chris - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Refactoring heads up
On 24/04/2024 17:52, Mark Thomas wrote: My plan is to commit these changes to 11.0.x with the low risk parts (e.g. new methods) back-ported. Then, once we can see what is left, we can decide how quickly/slowly we want to back-port the complete fix to 10.1.x and 9.0.x (the issue was reported against 10.1.x). All is looking good so far. The complete refactoring has been applied to 11.0.x 10.1.x and 9.0.x have the new header parser and are using it for the ChunkedInputFilter. The question is how long do we want to wait before back-porting the standard HTTP header parsing? Essentially this means back-porting this commit: https://github.com/apache/tomcat/commit/e5acf2cf0f745350c85d81532826d92b1882469a Thoughts? I'm thinking wait at least one release cycle before back-porting just in case of regressions given that this affects every request. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Refactoring heads up
Hi all, The Spring folks have pinged me on an issue reported to them. The short version is that Tomcat doesn't support non-blocking reads of chunked request bodies. While we have nearly all of the pieces we need to fix this, the commit is still going to be quite large involving quite a lot of changes to the ChunkedInputFilter. This will require some changes to Tomcat's internal API for ChunkedInputFilter - a number of protected fields and methods will be removed or made private. I have this working locally except for trailer fields. Closely related is the parsing of trailer fields. We already have a code comment saying this is very similar to HTTP fields (headers) but a common implementation isn't provided because fields use blocking or non-blocking IO but trailer fields only use blocking IO. Given we need to support non-blocking IO there is now a much stronger case for pulling the field (header) parsing code out to a separate class and reusing it for trailer fields. I'm starting to work on this now and hope to complete the work this week. Given these changes are going to impact pretty much every request I wanted to provide a heads up that these changes were on the way. My plan is to commit these changes to 11.0.x with the low risk parts (e.g. new methods) back-ported. Then, once we can see what is left, we can decide how quickly/slowly we want to back-port the complete fix to 10.1.x and 9.0.x (the issue was reported against 10.1.x). Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Refactoring heads up
Hi, As you might have worked out from my commits, I'm working on a fix for bug 60276 [1] to add gzip compression support to HTTP/2. As usual, I want to reduce duplication / copy/paste so I am looking to re-use the GzipOutputFilter from HTTP/1.1. As I have been working on this it has become apparent that there is a set of existing classes that are potentially common to HTTP/1.1 and HTTP/2 and that the HTTP/2 compression implementation will introduce a few new classes / interfaces that are HTTP specific but not HTTP version specific. I have therefore been thinking about the following refactoring: Plan A: - Create a new package org.apache.coyote.http - Move org.apache.coyote.http11.filters.* to org.apache.coyote.http.filters.* - Move org.apache.coyote.http11.(In|Out)putFilter to org.apache.coyote.http - Put the new HTTP version neutral classes in org.apache.coyote.http My current response compression solution means HTTP/2 only needs the GzipOutputFilter. I think there is scope to use some of the other filters as well. Thoughts? Is this too much refactoring (and potential disruption) for too little benefit? Plan B is too leave the existing classes where they are and put the new classes common to HTTP/1.1 and HTTP/2 in org.apache.coyote.http11 (since the existing classes common to both are under that package). I do like the idea of having the HTTP version neutral classes in a separate package. That said, the chunking filters aren't version neutral. They only apply to HTTP/1.1. While we could keep them in their existing package splitting the filters across multiple packages really does seem wrong. As I have been typing this e-mail I think I have convinced myself that plan B is the better option. I'll look at this again tomorrow with a fresh pair of eyes but I'd appreciate some community feedback on the two options above (or a suggestion of an alternative approach). Thanks, Mark [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=60276 - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
