-
Notifications
You must be signed in to change notification settings - Fork 38.8k
Description
Version: 6.2.6
Problem
Leaky reference in HttpHeaders breaks assumptions of immutability surrounding HttpEntity.EMPTY.
Description
When passing an existing instance of HttpHeaders to the HttpHeaders constructor, a reference to the original is maintained internally, and this is updated when setting headers on the new instance.
When creating a new HttpHeaders from HttpEntity.EMPTY.getHeaders(), the original headers are updated when updating the new ones, and the original HttpEntity is no longer "empty".
Use case
It is desirable to pass an existing instance of HttpEntity to some method, for the purposes of centralising logic to add an Authorization header for requests during integration testing. The HttpEntity is constructed naively by the test, with the parameters that it defines, then delegates to the method to have a valid auth header added.
Imagine a GET request to the endpoint /some-path.
The test may call a method like
<T> ResponseEntity<T> getAuthenticatedForEntity(String path, Class<T> responseType) {
return restTemplate
.exchange(
path,
HttpMethod.GET,
authenticated(HttpEntity.EMPTY),
responseType
);
}
where the authenticated method will take an existing HttpEntity and add a token to the headers. There may be other cases where the HttpEntity is defined by the test itself and the test calls the authenticated method, so it becomes desirable to base the abstraction, ultimately, around HttpEntity.
Proposal
Deprecate the constant HttpEntity.EMPTY, as the semantics are flawed. There cannot be a single instance, due to the way that HttpHeaders works.
Either the default constructor on HttpEntity should be public, or a new static method HttpEntity.empty() should return a new, empty instance.
Example Repo
https://github.com/sean-hernon/empty-http-entity-issue/
Only a simplified version of the authenticated method described above is included, for brevity.
Workaround
Instead of HttpEntity.EMPTY, use new HttpEntity(){}.