Skip to content

Commit 58b3af9

Browse files
jhoellerjhoeller
committed
Nullability fine-tuning and related polishing
Issue: SPR-17250
1 parent 5f2d47a commit 58b3af9

File tree

7 files changed

+77
-59
lines changed

7 files changed

+77
-59
lines changed

‎spring-web/src/main/java/org/springframework/http/ReadOnlyHttpHeaders.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
* {@code HttpHeaders} object that can only be read, not written to.
3232
*
3333
* @author Brian Clozel
34-
* @since 5.1
34+
* @since 5.1.1
3535
*/
3636
class ReadOnlyHttpHeaders extends HttpHeaders {
3737

@@ -59,10 +59,7 @@ public MediaType getContentType() {
5959
@Override
6060
public List<String> get(Object key) {
6161
List<String> values = this.headers.get(key);
62-
if (values != null) {
63-
return Collections.unmodifiableList(values);
64-
}
65-
return values;
62+
return (values != null ? Collections.unmodifiableList(values) : null);
6663
}
6764

6865
@Override

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

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,18 @@
3636
* {@code MultiValueMap} implementation for wrapping Jetty HTTP headers.
3737
*
3838
* @author Brian Clozel
39-
* @since 5.1
39+
* @since 5.1.1
4040
*/
4141
class JettyHeadersAdapter implements MultiValueMap<String, String> {
4242

4343
private final HttpFields headers;
4444

45+
4546
JettyHeadersAdapter(HttpFields headers) {
4647
this.headers = headers;
4748
}
4849

50+
4951
@Override
5052
public String getFirst(String key) {
5153
return this.headers.get(key);
@@ -95,24 +97,18 @@ public int size() {
9597

9698
@Override
9799
public boolean isEmpty() {
98-
return this.headers.size() == 0;
100+
return (this.headers.size() == 0);
99101
}
100102

101103
@Override
102104
public boolean containsKey(Object key) {
103-
if (key instanceof String) {
104-
return this.headers.containsKey((String) key);
105-
}
106-
return false;
105+
return (key instanceof String && this.headers.containsKey((String) key));
107106
}
108107

109108
@Override
110109
public boolean containsValue(Object value) {
111-
if (value instanceof String) {
112-
return this.headers.stream()
113-
.anyMatch(field -> field.contains((String) value));
114-
}
115-
return false;
110+
return (value instanceof String &&
111+
this.headers.stream().anyMatch(field -> field.contains((String) value)));
116112
}
117113

118114
@Nullable
@@ -144,8 +140,8 @@ public List<String> remove(Object key) {
144140
}
145141

146142
@Override
147-
public void putAll(Map<? extends String, ? extends List<String>> m) {
148-
m.forEach(this::put);
143+
public void putAll(Map<? extends String, ? extends List<String>> map) {
144+
map.forEach(this::put);
149145
}
150146

151147
@Override
@@ -179,6 +175,7 @@ public int size() {
179175
};
180176
}
181177

178+
182179
private class EntryIterator implements Iterator<Entry<String, List<String>>> {
183180

184181
private Enumeration<String> names = headers.getFieldNames();
@@ -194,6 +191,7 @@ public Entry<String, List<String>> next() {
194191
}
195192
}
196193

194+
197195
private class HeaderEntry implements Entry<String, List<String>> {
198196

199197
private final String key;
@@ -204,7 +202,7 @@ private class HeaderEntry implements Entry<String, List<String>> {
204202

205203
@Override
206204
public String getKey() {
207-
return this.key.toString();
205+
return this.key;
208206
}
209207

210208
@Override
@@ -219,4 +217,5 @@ public List<String> setValue(List<String> value) {
219217
return previousValues;
220218
}
221219
}
220+
222221
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@
3434
import org.springframework.core.io.buffer.DataBufferFactory;
3535
import org.springframework.http.HttpHeaders;
3636
import org.springframework.http.MediaType;
37+
import org.springframework.util.Assert;
3738

3839
/**
3940
* {@link ServletHttpHandlerAdapter} extension that uses Jetty APIs for writing
4041
* to the response with {@link ByteBuffer}.
4142
*
4243
* @author Violeta Georgieva
44+
* @author Brian Clozel
4345
* @since 5.0
4446
* @see org.springframework.web.server.adapter.AbstractReactiveWebInitializer
4547
*/
@@ -53,6 +55,8 @@ public JettyHttpHandlerAdapter(HttpHandler httpHandler) {
5355
@Override
5456
protected ServletServerHttpRequest createRequest(HttpServletRequest request, AsyncContext context)
5557
throws IOException, URISyntaxException {
58+
59+
Assert.notNull(getServletPath(), "Servlet path is not initialized");
5660
return new JettyServerHttpRequest(request, context, getServletPath(), getDataBufferFactory(), getBufferSize());
5761
}
5862

@@ -64,6 +68,7 @@ protected ServletServerHttpResponse createResponse(HttpServletResponse response,
6468
response, context, getDataBufferFactory(), getBufferSize(), request);
6569
}
6670

71+
6772
private static final class JettyServerHttpRequest extends ServletServerHttpRequest {
6873

6974
JettyServerHttpRequest(HttpServletRequest request, AsyncContext asyncContext,

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,18 @@
3434
* {@code MultiValueMap} implementation for wrapping Netty HTTP headers.
3535
*
3636
* @author Brian Clozel
37-
* @since 5.1
37+
* @since 5.1.1
3838
*/
3939
class NettyHeadersAdapter implements MultiValueMap<String, String> {
4040

4141
private final HttpHeaders headers;
4242

43+
4344
NettyHeadersAdapter(HttpHeaders headers) {
4445
this.headers = headers;
4546
}
4647

48+
4749
@Override
4850
@Nullable
4951
public String getFirst(String key) {
@@ -99,14 +101,14 @@ public boolean isEmpty() {
99101

100102
@Override
101103
public boolean containsKey(Object key) {
102-
return (key instanceof String) && this.headers.contains((String) key);
104+
return (key instanceof String && this.headers.contains((String) key));
103105
}
104106

105107
@Override
106108
public boolean containsValue(Object value) {
107-
return (value instanceof String) &&
109+
return (value instanceof String &&
108110
this.headers.entries().stream()
109-
.anyMatch(entry -> value != null && value.equals(entry.getValue()));
111+
.anyMatch(entry -> value.equals(entry.getValue())));
110112
}
111113

112114
@Override
@@ -138,8 +140,8 @@ public List<String> remove(Object key) {
138140
}
139141

140142
@Override
141-
public void putAll(Map<? extends String, ? extends List<String>> m) {
142-
m.forEach(this.headers::add);
143+
public void putAll(Map<? extends String, ? extends List<String>> map) {
144+
map.forEach(this.headers::add);
143145
}
144146

145147
@Override
@@ -173,6 +175,7 @@ public int size() {
173175
};
174176
}
175177

178+
176179
private class EntryIterator implements Iterator<Entry<String, List<String>>> {
177180

178181
private Iterator<String> names = headers.names().iterator();
@@ -188,6 +191,7 @@ public Entry<String, List<String>> next() {
188191
}
189192
}
190193

194+
191195
private class HeaderEntry implements Entry<String, List<String>> {
192196

193197
private final String key;

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,23 +38,25 @@
3838
* {@code MultiValueMap} implementation for wrapping Tomcat HTTP headers.
3939
*
4040
* @author Brian Clozel
41-
* @since 5.1
41+
* @since 5.1.1
4242
*/
4343
class TomcatHeadersAdapter implements MultiValueMap<String, String> {
4444

4545
private final MimeHeaders headers;
4646

47+
4748
TomcatHeadersAdapter(MimeHeaders headers) {
4849
this.headers = headers;
4950
}
5051

52+
5153
@Override
5254
public String getFirst(String key) {
5355
return this.headers.getHeader(key);
5456
}
5557

5658
@Override
57-
public void add(String key, String value) {
59+
public void add(String key, @Nullable String value) {
5860
this.headers.addValue(key).setString(value);
5961
}
6062

@@ -69,7 +71,7 @@ public void addAll(MultiValueMap<String, String> values) {
6971
}
7072

7173
@Override
72-
public void set(String key, String value) {
74+
public void set(String key, @Nullable String value) {
7375
this.headers.setValue(key).setString(value);
7476
}
7577

@@ -98,13 +100,13 @@ public int size() {
98100

99101
@Override
100102
public boolean isEmpty() {
101-
return this.headers.size() == 0;
103+
return (this.headers.size() == 0);
102104
}
103105

104106
@Override
105107
public boolean containsKey(Object key) {
106108
if (key instanceof String) {
107-
return this.headers.findHeader((String) key, 0) != -1;
109+
return (this.headers.findHeader((String) key, 0) != -1);
108110
}
109111
return false;
110112
}
@@ -152,8 +154,8 @@ public List<String> remove(Object key) {
152154
}
153155

154156
@Override
155-
public void putAll(Map<? extends String, ? extends List<String>> m) {
156-
m.forEach(this::put);
157+
public void putAll(Map<? extends String, ? extends List<String>> map) {
158+
map.forEach(this::put);
157159
}
158160

159161
@Override
@@ -191,6 +193,7 @@ public int size() {
191193
};
192194
}
193195

196+
194197
private class EntryIterator implements Iterator<Entry<String, List<String>>> {
195198

196199
private Enumeration<String> names = headers.names();
@@ -206,11 +209,12 @@ public Entry<String, List<String>> next() {
206209
}
207210
}
208211

212+
209213
private final class HeaderEntry implements Entry<String, List<String>> {
210214

211215
private final String key;
212216

213-
private HeaderEntry(String key) {
217+
HeaderEntry(String key) {
214218
this.key = key;
215219
}
216220

@@ -234,4 +238,5 @@ public List<String> setValue(List<String> value) {
234238
return previous;
235239
}
236240
}
241+
237242
}

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

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
*
4949
* @author Violeta Georgieva
5050
* @author Brian Clozel
51-
* @author Brian Clozel
5251
* @since 5.0
5352
* @see org.springframework.web.server.adapter.AbstractReactiveWebInitializer
5453
*/
@@ -64,7 +63,7 @@ public TomcatHttpHandlerAdapter(HttpHandler httpHandler) {
6463
protected ServletServerHttpRequest createRequest(HttpServletRequest request, AsyncContext asyncContext)
6564
throws IOException, URISyntaxException {
6665

67-
Assert.notNull(getServletPath(), "servletPath is not initialized.");
66+
Assert.notNull(getServletPath(), "Servlet path is not initialized");
6867
return new TomcatServerHttpRequest(
6968
request, asyncContext, getServletPath(), getDataBufferFactory(), getBufferSize());
7069
}
@@ -77,16 +76,20 @@ protected ServletServerHttpResponse createResponse(HttpServletResponse response,
7776
response, asyncContext, getDataBufferFactory(), getBufferSize(), request);
7877
}
7978

79+
8080
private static final class TomcatServerHttpRequest extends ServletServerHttpRequest {
8181

82-
private static final Field COYOTE_REQUEST_FIELD = ReflectionUtils.findField(RequestFacade.class, "request");
82+
private static final Field COYOTE_REQUEST_FIELD;
8383

8484
private final int bufferSize;
8585

8686
private final DataBufferFactory factory;
8787

8888
static {
89-
ReflectionUtils.makeAccessible(COYOTE_REQUEST_FIELD);
89+
Field field = ReflectionUtils.findField(RequestFacade.class, "request");
90+
Assert.state(field != null, "Incompatible Tomcat implementation");
91+
ReflectionUtils.makeAccessible(field);
92+
COYOTE_REQUEST_FIELD = field;
9093
}
9194

9295
TomcatServerHttpRequest(HttpServletRequest request, AsyncContext context,
@@ -99,8 +102,10 @@ private static final class TomcatServerHttpRequest extends ServletServerHttpRequ
99102
}
100103

101104
private static HttpHeaders createTomcatHttpHeaders(HttpServletRequest request) {
102-
Request tomcatRequest = ((org.apache.catalina.connector.Request) ReflectionUtils
103-
.getField(COYOTE_REQUEST_FIELD, request)).getCoyoteRequest();
105+
org.apache.catalina.connector.Request connectorRequest = (org.apache.catalina.connector.Request)
106+
ReflectionUtils.getField(COYOTE_REQUEST_FIELD, request);
107+
Assert.state(connectorRequest != null, "No Tomcat connector request");
108+
Request tomcatRequest = connectorRequest.getCoyoteRequest();
104109
TomcatHeadersAdapter headers = new TomcatHeadersAdapter(tomcatRequest.getMimeHeaders());
105110
return new HttpHeaders(headers);
106111
}
@@ -112,11 +117,9 @@ protected DataBuffer readFromInputStream() throws IOException {
112117
DataBuffer dataBuffer = this.factory.allocateBuffer(capacity);
113118
try {
114119
ByteBuffer byteBuffer = dataBuffer.asByteBuffer(0, capacity);
115-
116120
ServletRequest request = getNativeRequest();
117121
int read = ((CoyoteInputStream) request.getInputStream()).read(byteBuffer);
118122
logBytesRead(read);
119-
120123
if (read > 0) {
121124
dataBuffer.writePosition(read);
122125
release = false;
@@ -140,10 +143,13 @@ else if (read == -1) {
140143

141144
private static final class TomcatServerHttpResponse extends ServletServerHttpResponse {
142145

143-
private static final Field COYOTE_RESPONSE_FIELD = ReflectionUtils.findField(ResponseFacade.class, "response");
146+
private static final Field COYOTE_RESPONSE_FIELD;
144147

145148
static {
146-
ReflectionUtils.makeAccessible(COYOTE_RESPONSE_FIELD);
149+
Field field = ReflectionUtils.findField(ResponseFacade.class, "response");
150+
Assert.state(field != null, "Incompatible Tomcat implementation");
151+
ReflectionUtils.makeAccessible(field);
152+
COYOTE_RESPONSE_FIELD = field;
147153
}
148154

149155
TomcatServerHttpResponse(HttpServletResponse response, AsyncContext context,
@@ -153,8 +159,10 @@ private static final class TomcatServerHttpResponse extends ServletServerHttpRes
153159
}
154160

155161
private static HttpHeaders createTomcatHttpHeaders(HttpServletResponse response) {
156-
Response tomcatResponse = ((org.apache.catalina.connector.Response) ReflectionUtils
157-
.getField(COYOTE_RESPONSE_FIELD, response)).getCoyoteResponse();
162+
org.apache.catalina.connector.Response connectorResponse = (org.apache.catalina.connector.Response)
163+
ReflectionUtils.getField(COYOTE_RESPONSE_FIELD, response);
164+
Assert.state(connectorResponse != null, "No Tomcat connector response");
165+
Response tomcatResponse = connectorResponse.getCoyoteResponse();
158166
TomcatHeadersAdapter headers = new TomcatHeadersAdapter(tomcatResponse.getMimeHeaders());
159167
return new HttpHeaders(headers);
160168
}

0 commit comments

Comments
 (0)