Skip to content

[Bug] NettyHeadersAdapter duplicates values when header case is mixed #33931

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

Closed
mtheos opened this issue Nov 21, 2024 · 3 comments
Closed

[Bug] NettyHeadersAdapter duplicates values when header case is mixed #33931

mtheos opened this issue Nov 21, 2024 · 3 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: duplicate A duplicate of another issue

Comments

@mtheos
Copy link

mtheos commented Nov 21, 2024

Hello Spring maintainers,

I've found a bug in the NettyHeadersAdapter that causes it to duplicate header values when the string case is mixed up between entries.

This arises because the adapter iterates through the underlying entries using a case-insensitive method to get the values.

DefaultHttpHeaders headers;
for (var n : headers.names()) {       <-- case sensitive
  for (var v : headers.getAll(n)) {   <-- case insensitive
    System.out.println(n + "=" + v);
  }
}

I've reproduced this on 6.2.0
Reproducer:

var headers = new DefaultHttpHeaders();
var adapter = new Netty5HeadersAdapter(headers);  // Also with Netty4
adapter.add("foo-bar", "one");
adapter.add("Foo-Bar", "two");
adapter.add("fOO-bar", "three");
for (var e : adapter.entrySet()) {
   System.out.println(e.getKey() + "=" + e.getValue());
}

/*
Output:
foo-bar=[one, two, three]
Foo-Bar=[one, two, three]
fOO-bar=[one, two, three]
*/
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 21, 2024
@bclozel
Copy link
Member

bclozel commented Nov 21, 2024

@mtheos have you seen this part of the javadoc?

* <p>Note that {@code HttpHeaders} instances created by the default constructor

@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) labels Nov 21, 2024
@mtheos
Copy link
Author

mtheos commented Nov 21, 2024

Hi @bclozel, I haven't; thanks for pointing it out. I can't say I'm a big fan of that API 😓 but if you know about the behaviour and don't plan to change it, I can work around it.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 21, 2024
@bclozel
Copy link
Member

bclozel commented Nov 21, 2024

We're not big fans of this either and we plan to revisit this arrangement altogether in #33913.

In the past, this wasn't an issue as we were copying all "native" (as in the underlying server implementation) headers into our own multivalue map, and then back. We found out that this was one of the main source of framework overhead and adapting to native headers improved throughput and allocation performance significantly.

Most server implementations do not model headers as a multivalue map, but rather as a collection of name/values pairs. They're only considering the case insensitive behavior for a few dedicated methods. Iterating over headers does not take this into account.

So we're stuck between making the performance way worse for all, or reconsidering this aspect in the future. For now, following the advice in the javadoc is your best bet and we'll figure out a way to improve the API in the future.

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Nov 21, 2024
@bclozel bclozel added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants