Problem

  1. When I use HttpHeaders, its constructors confuse me, because instance initialized from original header has same hashmap of original. For example, test code below will be failed. Calling a method of copy instance may affect original instance.
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpHeaders;

public class HeaderTest {

    @Test
    void test() {
        var original = new HttpHeaders();
        var copy = new HttpHeaders(original);

        copy.add("key", "value");

        Assertions.assertFalse(original.containsKey("key"));
    }
}
  1. The javadoc says that the constructor I mentioned is for internal use in the spring fremework, but users can access it. https://github.com/spring-projects/spring-framework/blob/c28fd91a10eaeae4c090f4fb9faa89f2f9d34bda/spring-web/src/main/java/org/springframework/http/HttpHeaders.java#L426-L444

Suggetion

So I suggest this confusional constructor is package-private not to be accessed from external the library or support immutableness of its hashmap field.

Comment From: sbrannen

Hi @ryoheinagao,

Thanks for opening your first issue for the Spring Framework.

What is your concrete use case for creating an instance of HttpHeaders from an existing instance of HttpHeaders and not wanting modifications to write through to the original instance?

Comment From: ryoheinagao

@sbrannen Thank you for your comment.

For example, most of header's value are constant but some value(secret token or user's location info) are vary in each request. And constant pairs are common among multiple implementation classes or methods. In this case, I want to create an instance from constant header, and then I want to add these temporary key-value. The pseudo code is below.

In the case of sharing key-value between implementation class or there are many constant value, I think instantiation from existing HttpHeaders is effective.

import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;

public class SomeRepository {

    private static final HttpHeaders STATIC_HEADER = new HttpHeaders();

    static {
        STATIC_HEADER.add(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE);
        // please assume this header has many key-value pairs and is shared among methods or classes
    }

    String findSomePrivateValue(String userToken,){
        var header = new HttpHeaders(STATIC_HEADER);
        header.add("X-Secret", userToken);
        // rest operations etc...
    }

    String findUserAround(String userLocation){
        var header = new HttpHeaders(STATIC_HEADER);
        header.add("X-Location", userLocation);
        // rest operations etc...
    }
}

Comment From: ryoheinagao

What do you think my suggestion?

Comment From: bclozel

Thanks for the suggestion, but the behavior of that constructor is well documented and well defined. Unlike collection-related constructors (like HashMap or ArrayList), this is not about copying entries to a new collection, but creating a new HttpHeaders instance backed by an existing map. Making that method package private will break existing integrations it's made for (not all live in the same package).

I'm declining this issue as a result.