athom
athom

Reputation: 1608

Possible bug in Spring HttpHeaders

I found what I believe could be a bug in the Spring classes HttpHeaders and ReadOnlyHttpHeaders. I want to confirm this before raising a Jira defect with Spring. Here is a snippet of the code I use to create an empty HttpHeaders object:

HttpHeaders myHeaders = HttpHeaders.writableHttpHeaders(HttpHeaders.EMPTY);

I then add headers to my new object using:

myHeaders.add(HttpHeaders.ACCEPT_ENCODING, "gzip")

After this HttpHeaders.EMPTY is not empty anymore

HttpHeaders.EMPTY.size() == 1

The javadoc for HttpHeaders.EMPTY states:

/**
* The empty {@code HttpHeaders} instance (immutable).
*/
public static final HttpHeaders EMPTY

The problem here is that when 'HttpHeaders.EMPTY' is used elsewhere, it introduces unexpected headers.

Consider the following unit test:

@Test
public void testUpdateEmptyHeaders() {
    assertEquals(0, HttpHeaders.EMPTY.size()); // **Success**
    HttpHeaders myHeaders = HttpHeaders.writableHttpHeaders(HttpHeaders.EMPTY);
    myHeaders.add(HttpHeaders.ACCEPT_ENCODING, "gzip");
    assertEquals(0, HttpHeaders.EMPTY.size()); // **Assert Fails**
}

@Test
// This test will fail if run after the test above, but will be successful if run by itself
public void testEmptyHeaders() {
    assertEquals(0, HttpHeaders.EMPTY.size()); 
}

Here is the result of the unit tests:

// testUpdateEmptyHeaders
08:39:28.450 [main] DEBUG org.springframework.test.context.support.AbstractDirtiesContextTestExecutionListener - After test method: context [DefaultTestContext@2e222612, testMethod = testUpdateEmptyHeaders@AuditContextTest, testException = java.lang.AssertionError: expected:<0> but was:<1>

java.lang.AssertionError: 
Expected :0
Actual   :1

// testEmptyHeaders
08:39:28.482 [main] DEBUG org.springframework.test.context.support.AbstractDirtiesContextTestExecutionListener - After test method: context [DefaultTestContext@2e222612, testMethod = testEmptyHeaders@AuditContextTest, testException = java.lang.AssertionError: expected:<0> but was:<1>

java.lang.AssertionError: 
Expected :0
Actual   :1

I feel that this is a bug since HttpHeaders.EMPTY should be immutable. I have also been able to fix this by making two changes in Spring HttpHeaders.java and ReadOnlyHttpHeaders.java

Upvotes: 4

Views: 2271

Answers (2)

athom
athom

Reputation: 1608

I have reported this bug and it is fixed in Spring 5.1.4 as per https://jira.spring.io/browse/SPR-17633

Upvotes: 2

Ryuzaki L
Ryuzaki L

Reputation: 40098

Yup you are right this might be bug for spring framework HttpHeaders

public static final HttpHeaders EMPTY

HttpHeaders.EMPTY This will return empty HttpHeaders instance (immutable). (and it is singleton)

Case :1 Let's take a look at HttpHeaders.Empty, which returns immutable object

    HttpHeaders head = HttpHeaders.EMPTY;
    
    System.out.println(System.identityHashCode(head));  //1338668845
    
    System.out.println(head.size());                    //0
    
    HttpHeaders myHeaders = HttpHeaders.writableHttpHeaders(HttpHeaders.EMPTY); 
    
    System.out.println(System.identityHashCode(myHeaders));  //159413332
    
    myHeaders.add(HttpHeaders.ACCEPT_ENCODING, "gzip"); 
    
    head = HttpHeaders.EMPTY; 
    System.out.println(System.identityHashCode(head));       //1338668845
    System.out.println(head.size());                    //1
    System.out.println(head);                            //{Accept-Encoding=[gzip]}
    
    HttpHeaders head1 = HttpHeaders.EMPTY;
    System.out.println(head1);                          //{Accept-Encoding=[gzip]}
    System.out.println(System.identityHashCode(head1));   //1338668845

Conclusions :

1 : HttpHeaders.EMPTY is always returning the singleton object

2: The problem is when HttpHeaders.EMPTY is passed to writableHttpHeaders method internally the returned object is having relation with HttpHeaders.EMPTY singleton object, look at case 2

Case :2 return object fromwritableHttpHeaders reflects to HttpHeaders.EMPTY singleton object (internally and indirectly)

    HttpHeaders head = HttpHeaders.EMPTY;           
    
    System.out.println(System.identityHashCode(head));  //1338668845
    
    System.out.println(head.size());                //0
    
    HttpHeaders myHeaders = HttpHeaders.writableHttpHeaders(HttpHeaders.EMPTY); 
    
    System.out.println(System.identityHashCode(myHeaders));  //159413332
    
    myHeaders.add(HttpHeaders.ACCEPT_ENCODING, "gzip"); 
    
    myHeaders.add("hello", "value");
    
    head = HttpHeaders.EMPTY; 
    System.out.println(System.identityHashCode(head));  //1338668845
    System.out.println(head.size());              //2
    System.out.println(head);                     //{Accept-Encoding=[gzip], hello=[value]}
    
    HttpHeaders head1 = HttpHeaders.EMPTY;
    System.out.println(head1);                    //{Accept-Encoding=[gzip], hello=[value]}
    System.out.println(System.identityHashCode(head1));   //1338668845
    
    myHeaders.remove("hello");
    
    System.out.println(System.identityHashCode(head));     //1338668845
    System.out.println(head.size());                 //1
    System.out.println(head);                       //{Accept-Encoding=[gzip]}
    
    System.out.println(head1);                        //{Accept-Encoding=[gzip]}
    System.out.println(System.identityHashCode(head1));    //1338668845
    

Conclusion :

1 : add and remove operations performed on myHeaders object are reflecting to HttpHeaders.EMPTY object

Case : 3 Suppose if we pass empty instance of the HttpHeaders object to writableHttpHeaders using constructor then there is no issue everything works pretty clear

    HttpHeaders head = HttpHeaders.EMPTY;
    
    System.out.println(System.identityHashCode(head));       //1338668845
    
    System.out.println(head.size());                     //0
    
    HttpHeaders myHeaders = HttpHeaders.writableHttpHeaders(new HttpHeaders()); 
    
    System.out.println(System.identityHashCode(myHeaders));       //1323165413
    
    myHeaders.add(HttpHeaders.ACCEPT_ENCODING, "gzip"); 
    
    myHeaders.add("hello", "value");
    
    head = HttpHeaders.EMPTY; 
    System.out.println(System.identityHashCode(head));           //1338668845
    System.out.println(head.size());                    //0
    System.out.println(head);                           //{}
    
    HttpHeaders head1 = HttpHeaders.EMPTY;
    System.out.println(head1);                           //{}
    System.out.println(System.identityHashCode(head1));   //1338668845

Case : 4 Even though indirectly immutable HttPHeaders.EMPTY can be modified, but still it throws an error if you try modifying directly

    HttpHeaders head = HttpHeaders.EMPTY;
    
    System.out.println(System.identityHashCode(head));
    
    System.out.println(head.size());
    
    HttpHeaders myHeaders = HttpHeaders.writableHttpHeaders(HttpHeaders.EMPTY); 
    
    System.out.println(System.identityHashCode(myHeaders));
    
    myHeaders.add(HttpHeaders.ACCEPT_ENCODING, "gzip"); 
    
    head = HttpHeaders.EMPTY; 
    System.out.println(System.identityHashCode(head));
    System.out.println(head.size());
    System.out.println(head);
    
    head.add("hello", "value");

Output :

1338668845
0
159413332
1338668845
1
{Accept-Encoding=[gzip]}
Exception in thread "main" java.lang.UnsupportedOperationException
at org.springframework.http.ReadOnlyHttpHeaders.add(ReadOnlyHttpHeaders.java:67)
at com.demo.NestedJsonParse.main(NestedJsonParse.java:40)

Final Conclusion : Yes it is bug you can raise a bug for spring projects spring-bug, immutable objects cannot change the state

Upvotes: 2

Related Questions