arturobernalg commented on PR #1027: URL: https://github.com/apache/tomcat/pull/1027#issuecomment-6719226575
Thanks, this is a useful limit and it lines up with the HTTP/2 attribute. I tried it on a real Tomcat. With the default, 100 trailer fields are accepted and 101 are rejected, `-1` removes the limit and `maxTrailerCount="5"` rejects 6. It also counts trailer fields that are not in `allowedTrailerHeaders`, which is what we want since they still have to be parsed. Two small things. The description says the request fails with a 400, but when the servlet reads the body it gets a 500 and the connection is closed. That is the same as the existing `maxTrailerSize` limit does on main, so I think the description should say that rather than 400. The new tests also do not compile against current main, because `Context.addServletMappingDecoded()` no longer exists. `addServletMapping()` is what the other tests in that class use now. With that change all the http11 tests pass for me. I would keep the default of 100 for consistency with HTTP/2, but it is a behaviour change. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
