Skip to content

Commit ab8310b

Browse files
committed
Fix HeadersAdapters implementations
This commit harmonizes the `HeadersAdapter` implementations across all supported servers with regards to the `get(Object key)` contract; some server implementations are not sticking to a `Map`-like contract and return empty `List` instead of `null` when a header is not present. This also fixes the `size()` implementations to reflect the number of header keys, as some implementations consider multiple values for the same header as different entries. Issue: SPR-17396
1 parent fdaceeb commit ab8310b

File tree

4 files changed

+103
-4
lines changed

4 files changed

+103
-4
lines changed

spring-web/src/main/java/org/springframework/http/server/reactive/JettyHeadersAdapter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public boolean containsValue(Object value) {
114114
@Nullable
115115
@Override
116116
public List<String> get(Object key) {
117-
if (key instanceof String) {
117+
if (containsKey(key)) {
118118
return this.headers.getValuesList((String) key);
119119
}
120120
return null;

spring-web/src/main/java/org/springframework/http/server/reactive/NettyHeadersAdapter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public Map<String, String> toSingleValueMap() {
9191

9292
@Override
9393
public int size() {
94-
return this.headers.size();
94+
return this.headers.names().size();
9595
}
9696

9797
@Override
@@ -114,7 +114,7 @@ public boolean containsValue(Object value) {
114114
@Override
115115
@Nullable
116116
public List<String> get(Object key) {
117-
if (key instanceof String) {
117+
if (containsKey(key)) {
118118
return this.headers.getAll((String) key);
119119
}
120120
return null;

spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHeadersAdapter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public boolean containsValue(Object value) {
128128
@Override
129129
@Nullable
130130
public List<String> get(Object key) {
131-
if (key instanceof String) {
131+
if (containsKey(key)) {
132132
return Collections.list(this.headers.values((String) key));
133133
}
134134
return null;
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Copyright 2002-2018 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.http.server.reactive;
18+
19+
import java.util.Locale;
20+
21+
import io.netty.handler.codec.http.DefaultHttpHeaders;
22+
import io.undertow.util.HeaderMap;
23+
import org.apache.tomcat.util.http.MimeHeaders;
24+
import org.eclipse.jetty.http.HttpFields;
25+
import org.junit.Test;
26+
import org.junit.runner.RunWith;
27+
import org.junit.runners.Parameterized;
28+
29+
import org.springframework.util.CollectionUtils;
30+
import org.springframework.util.LinkedCaseInsensitiveMap;
31+
import org.springframework.util.MultiValueMap;
32+
33+
import static org.junit.Assert.assertEquals;
34+
import static org.junit.Assert.assertNull;
35+
import static org.junit.Assert.assertTrue;
36+
37+
/**
38+
* Unit tests for {@code HeadersAdapters} {@code MultiValueMap} implementations.
39+
*
40+
* @author Brian Clozel
41+
*/
42+
@RunWith(Parameterized.class)
43+
public class HeadersAdaptersTests {
44+
45+
@Parameterized.Parameter(0)
46+
public MultiValueMap<String, String> headers;
47+
48+
@Parameterized.Parameters(name = "headers [{0}]")
49+
public static Object[][] arguments() {
50+
return new Object[][] {
51+
{CollectionUtils.toMultiValueMap(
52+
new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH))},
53+
{new NettyHeadersAdapter(new DefaultHttpHeaders())},
54+
{new TomcatHeadersAdapter(new MimeHeaders())},
55+
{new UndertowHeadersAdapter(new HeaderMap())},
56+
{new JettyHeadersAdapter(new HttpFields())}
57+
};
58+
}
59+
60+
@Test
61+
public void getWithUnknownHeaderShouldReturnNull() {
62+
assertNull(this.headers.get("Unknown"));
63+
}
64+
65+
@Test
66+
public void getFirstWithUnknownHeaderShouldReturnNull() {
67+
assertNull(this.headers.getFirst("Unknown"));
68+
}
69+
70+
@Test
71+
public void sizeWithMultipleValuesForHeaderShouldCountHeaders() {
72+
this.headers.add("TestHeader", "first");
73+
this.headers.add("TestHeader", "second");
74+
assertEquals(1, this.headers.size());
75+
}
76+
77+
@Test
78+
public void keySetShouldNotDuplicateHeaderNames() {
79+
this.headers.add("TestHeader", "first");
80+
this.headers.add("OtherHeader", "test");
81+
this.headers.add("TestHeader", "second");
82+
assertEquals(2, this.headers.keySet().size());
83+
}
84+
85+
@Test
86+
public void containsKeyShouldBeCaseInsensitive() {
87+
this.headers.add("TestHeader", "first");
88+
assertTrue(this.headers.containsKey("testheader"));
89+
}
90+
91+
@Test
92+
public void addShouldKeepOrdering() {
93+
this.headers.add("TestHeader", "first");
94+
this.headers.add("TestHeader", "second");
95+
assertEquals("first", this.headers.getFirst("TestHeader"));
96+
assertEquals("first", this.headers.get("TestHeader").get(0));
97+
}
98+
99+
}

0 commit comments

Comments
 (0)