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

Consistent handling of null header values in HttpHeaders [SPR-17588] #22120

Closed
spring-issuemaster opened this Issue Dec 11, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Dec 11, 2018

Juergen Hoeller opened SPR-17588 and commented

The specific header setter methods in HttpHeaders use MultiValueMap.set/add for populating the underlying data structure. Unfortunately, null values are mostly being stored as null entries in single-entry lists for a header, or sometimes being skipped altogether. While the end result (e.g. in a populated server response) usually ignores null header values anyway and all getFirst access returns null just like it would in case of a non-existing header as well, some user-level accessor methods (e.g. HttpHeaders.getCacheControl() return a bogus text representation in case of a null value. Let's streamline this for 5.1.4 as a continuation of the HttpHeaders refactoring in the 5.1.x line.


Affects: 5.1.3

Reference URL: eclipse/jetty.project#1116

Issue Links:

  • #21783 Improve WebFlux performance for header management
  • #17816 SimpleClientHttpRequestFactory: headers with null values should be sent as empty Strings
  • #22103 Allow java.time types for setting the Last-Modified header

Referenced from: commits 5bbbc82

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Dec 12, 2018

Juergen Hoeller commented

The 5.0-introduced null-capable setters delegate to the new setOrRemove method now, removing the header entirely if the value is null. Since 4.3 did not support null values in most cases to begin with, this shouldn't cause much difference in practice and simply deliver the expected result in a streamlined fashion. However, there are few subtleties like containsKey calls returning false now after a null-passing call, or the cacheControl builder methods now removing any previous header if an empty CacheControl is being given (instead of just skipping the call). On the other hand, the plain set operation (as inherited from MultiValueMap) still stores null values if given, to be turned into empty header values later on (as desired by certain cases, e.g. #17816).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment