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

getHeaderNames should return header name once also when request has it in different case #11811

Closed
findepi opened this issue May 19, 2024 · 3 comments · Fixed by #11823 or #11846
Closed
Assignees
Labels
Bug For general bugs on Jetty side Sponsored This issue affects a user with a commercial support agreement

Comments

@findepi
Copy link

findepi commented May 19, 2024

Jetty version(s)
12.0.9

Jetty Environment
ee10

Java version/vendor (use: java -version)
Java 22

OS type/version
MacOS

Description
request.getHeaderNames is typically used to iterate over header names in the request and then request.getHeaders is used to retrieve values:

// example code supposed tor resemble typical use of getHeaderNames()
Enumeration<String> names = servletRequest.getHeaderNames();
while (names.hasMoreElements()) {
    String name = names.nextElement();
    doSomething(name, servletRequest.getHeaders(name))
}

When request contains both Foo-Bar and foo-bar headers, getHeaderNames returns both String values, so foo-bar header values are processed twice.

How to reproduce?

curl -H 'Foo-Bar: one' -H 'foo-bar: two'  http://localhost:8080/endpoint
@findepi findepi added the Bug For general bugs on Jetty side label May 19, 2024
@findepi
Copy link
Author

findepi commented May 19, 2024

This code is potentially wrong because of this: https://github.com/jakartaee/servlet/blob/26a58cb1a6f148b56e274bf0f8030176d20cfeb6/api/src/main/java/jakarta/servlet/http/HttpServlet.java#L565
(it's look like it doesn't handle duplicated headers well, even if they are in same case)

@sbordet sbordet added the Sponsored This issue affects a user with a commercial support agreement label May 20, 2024
@gregw gregw self-assigned this May 21, 2024
@gregw
Copy link
Contributor

gregw commented May 21, 2024

I can confirm a reproduction... investigating....

gregw added a commit that referenced this issue May 21, 2024
Fix #11811 insensitive header name set by:
 + Using a EnumSet and TreeSet to ensure no duplicates in the set
 + Using an ArrayList to preserve the ordering (not necessary, but useful).
gregw added a commit that referenced this issue May 22, 2024
* Fix #11811 insensitive header name set

Fix #11811 insensitive header name set by:
 + Using a EnumSet and TreeSet to ensure no duplicates in the set
 + Using an ArrayList to preserve the ordering (not necessary, but useful).

* updates from review
@gregw
Copy link
Contributor

gregw commented May 27, 2024

Reopened to add some comments to HttpFields regarding how it is implemented as a list and the reasons behind that (see #3682)

gregw added a commit that referenced this issue May 27, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue May 27, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue May 27, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue May 27, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue May 27, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue May 27, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue May 27, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue May 27, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue May 28, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue Jun 6, 2024
Fix #11811 with javadoc and an optional random access implementation.
gregw added a commit that referenced this issue Jun 18, 2024
…elds via EnumMap not worth it. (#11846)

Fix #11811 with javadoc and  benchmark
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 Sponsored This issue affects a user with a commercial support agreement
Projects
Status: ✅ Done
3 participants