ChristopherSchultz commented on PR #893: URL: https://github.com/apache/tomcat/pull/893#issuecomment-3287022507
I'm also -1 for this approach. A more efficient equalsIgnoreCase could certainly be implemented, though there a bunch of gotchas with regard to i18n which make offloading to String.toLowerCase attractive. Creating two new String objects every time a comparison is done is certainly wasteful. I believe even String.toLowerCase applies all those complicated rules. Looking at MimeHeaders and MessageBytes, I don't see where String.toLowerCase is being called. Should I be looking elsewhere? The only place I see forcing lower-case is in Http11InputBuffer, and that's done once (during parsing) using as efficient an algorithm I can think of. Back to the original request: preserving the original letter-casing of the HTTP headers... I fail to see the use-case. I'm happy to hear what yours @maxfortun actually is, but your original post is very vague. The only way to preserve the original case of the headers _and_ improve comparison speed would be to store the string in two different formats: the `MessageBytes` (the current format) as well as another String value which contains the "original casing". Strings are not reusable, while `MessageBytes` instances are. So, supporting original-casing will cause a lot of GC churn. :( It also means we need to link the header-name MessageBytes with a String which makes everything a _little_ bigger. This is doable, but if we really like the idea of reusable MessageBytes objects, then coupling those with non-reusable String objects sounds like a deal-breaker to me. I agree that removing the (single) to-lower-case operation will achieve the goals of this PR / feature request. But I really want to understand the use-case. I haven't seen a modern web server that _doesn't_ force the header names to lower case. I also think that, should we continue to force everything to lower case, we should take advantage of the minor speedup we'll get from performing .equals instead of .equalsIgnoreCase. I tend to think that doing lower case is the right move, here. I'm sorry @maxfortun but you'll need to provide a much better justification for supporting case-preservation if you want us to consider this. -- 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]
