HttpHeaders constructor should not be public or should not mutate original HttpHeaders

[ryoheinagao]

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"));
    }
}

2. The javadoc says that the constructor I mentioned is for internal use in the spring fremework, but users can access it.
   

   spring-framework/spring-web/src/main/java/org/springframework/http/HttpHeaders.java

   Lines 426 to 444 in c28fd91

   	/**
   	* Construct a new, empty instance of the {@code HttpHeaders} object.
   	* <p>This is the common constructor, using a case-insensitive map structure.
   	*/
   	public HttpHeaders() {
   	this(CollectionUtils.toMultiValueMap(new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH)));
   	}
   	/**
   	* Construct a new {@code HttpHeaders} instance backed by an existing map.
   	* <p>This constructor is available as an optimization for adapting to existing
   	* headers map structures, primarily for internal use within the framework.
   	* @param headers the headers map (expected to operate with case-insensitive keys)
   	* @since 5.1
   	*/
   	public HttpHeaders(MultiValueMap<String, String> headers) {
   	Assert.notNull(headers, "MultiValueMap must not be null");
   	this.headers = headers;
   	}

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.

[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?

[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...
    }
}

[ryoheinagao]

What do you think my suggestion?

[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.