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

Improve iteration methods in native headers to MultiValueMap adapters #33823

Closed
simonbasle opened this issue Oct 30, 2024 · 2 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@simonbasle
Copy link
Contributor

In 6.1, HttpHeaders can be constructed from a native container/server's HTTP headers implementation by wrapping these various implementations in dedicated MultiValueMap<String, String> adapters. This is critical for performance in terms of throughput and memory pressure, as this allows for efficient storage of headers by avoiding copies when not necessary.

However, the iteration methods of MultiValueMap don't align entirely well with native implementations. For instance, some of these implementation only ignore the casing of a given header name when accessing it, not storing it. As such, when queried for the list of header names they include case variants.

This can lead to subtle bugs where an attempt to iterate over the entries of such an HttpHeaders produces duplicate keys AND duplicate values : as the pairing of names and values is not native, it is reconstructed by iterating over the names provided by the underlying structure and calling get(name) on each.

For example, considering the native headers map below contains TestHeader=first and TestHEADER=second:

HttpHeaders original = new HttpHeaders(someNativeHeaderMap);

HttpHeaders headers = new HttpHeaders();
for (Entry<String, List<String>> h : original.entrySet()) {
    headers.addAll(h.getKey(), h.getValue());
}

will result in an instance which contains TestHeader=[first,second,firstSecond], TestHEADER=[first,second,first,second].

The goal of this issue is to maintain current performance status quo as much as possible while introducing an alternative to entrySet that guarantees case-insensitive iteration. The javadoc should expose this limitation and offer guidance towards using the new method.

@simonbasle simonbasle added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 30, 2024
@simonbasle simonbasle self-assigned this Oct 30, 2024
@simonbasle simonbasle added this to the 6.1.15 milestone Nov 6, 2024
@simonbasle simonbasle added type: enhancement A general enhancement in: web Issues in web modules (web, webmvc, webflux, websocket) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 6, 2024
@simonbasle
Copy link
Contributor Author

The new method is headerSet(), instance method in HttpHeaders.

simonbasle added a commit that referenced this issue Nov 7, 2024
@simonbasle
Copy link
Contributor Author

Note that in the snippet above, headers.put(...) instead of headers.addAll(...) would be a relevant workaround.

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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant