Skip to content

Commit 0761ee9

Browse files
committed
Workaround for WFLY-3474 NullPointerException
Prior to this commit, the ServletResponseHttpHeaders.get method would throw an NPE when used under Wildfly 8.0.0.Final and 8.1.0.Final. This can be traced to WFLY-3474, which throws an NPE when calling HttpServletResponse.getHeaders("foo") and that header has not been defined prior to that. This would cause NPE being thrown by AbstractSockJsService when checking for CORS HTTP headers in the server HTTP response. This commit surrounds that method call in AbstractSockJsService and guards against this issue. Issue: SPR-11919 (cherry picked from commit 24cdefb)
1 parent 9054f4f commit 0761ee9

File tree

2 files changed

+42
-17
lines changed

2 files changed

+42
-17
lines changed

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import org.springframework.web.socket.WebSocketHandler;
4545
import org.springframework.web.socket.sockjs.SockJsException;
4646
import org.springframework.web.socket.sockjs.SockJsService;
47-
import org.springframework.web.util.UriComponentsBuilder;
4847

4948
/**
5049
* An abstract base class for {@link SockJsService} implementations that provides SockJS
@@ -102,16 +101,20 @@ public String getName() {
102101
}
103102

104103
/**
105-
* Transports which don't support cross-domain communication natively (e.g.
106-
* "eventsource", "htmlfile") rely on serving a simple page (using the
107-
* "foreign" domain) from an invisible iframe. Code run from this iframe
108-
* doesn't need to worry about cross-domain issues since it is running from
109-
* a domain local to the SockJS server. The iframe does need to load the
110-
* SockJS javascript client library and this option allows configuring that url.
111-
* For more details see the reference documentation.
112-
*
104+
* Transports with no native cross-domain communication (e.g. "eventsource",
105+
* "htmlfile") must get a simple page from the "foreign" domain in an invisible
106+
* iframe so that code in the iframe can run from a domain local to the SockJS
107+
* server. Since the iframe needs to load the SockJS javascript client library,
108+
* this property allows specifying where to load it from.
113109
* <p>By default this is set to point to
114110
* "https://d1fxtkz8shb9d2.cloudfront.net/sockjs-0.3.4.min.js".
111+
* However, it can also be set to point to a URL served by the application.
112+
* <p>Note that it's possible to specify a relative URL in which case the URL
113+
* must be relative to the iframe URL. For example assuming a SockJS endpoint
114+
* mapped to "/sockjs", and resulting iframe URL "/sockjs/iframe.html", then the
115+
* the relative URL must start with "../../" to traverse up to the location
116+
* above the SockJS mapping. In case of a prefix-based Servlet mapping one more
117+
* traversal may be needed.
115118
*/
116119
public void setSockJsClientLibraryUrl(String clientLibraryUrl) {
117120
this.clientLibraryUrl = clientLibraryUrl;
@@ -148,16 +151,13 @@ public int getStreamBytesLimit() {
148151
* clients with a "cookie_needed" boolean property that indicates whether the use of a
149152
* JSESSIONID cookie is required for the application to function correctly, e.g. for
150153
* load balancing or in Java Servlet containers for the use of an HTTP session.
151-
*
152154
* <p>This is especially important for IE 8,9 that support XDomainRequest -- a modified
153155
* AJAX/XHR -- that can do requests across domains but does not send any cookies. In
154156
* those cases, the SockJS client prefers the "iframe-htmlfile" transport over
155157
* "xdr-streaming" in order to be able to send cookies.
156-
*
157158
* <p>The SockJS protocol also expects a SockJS service to echo back the JSESSIONID
158159
* cookie when this property is set to true. However, when running in a Servlet
159160
* container this is not necessary since the container takes care of it.
160-
*
161161
* <p>The default value is "true" to maximize the chance for applications to work
162162
* correctly in IE 8,9 with support for cookies (and the JSESSIONID cookie in
163163
* particular). However, an application can choose to set this to "false" if
@@ -358,14 +358,18 @@ protected abstract void handleTransportRequest(ServerHttpRequest request, Server
358358

359359

360360
protected void addCorsHeaders(ServerHttpRequest request, ServerHttpResponse response, HttpMethod... httpMethods) {
361-
362361
HttpHeaders requestHeaders = request.getHeaders();
363362
HttpHeaders responseHeaders = response.getHeaders();
364363

365-
// Perhaps a CORS Filter has already added this?
366-
if (!CollectionUtils.isEmpty(responseHeaders.get("Access-Control-Allow-Origin"))) {
367-
logger.debug("Skip adding CORS headers, response already contains \"Access-Control-Allow-Origin\"");
368-
return;
364+
try {
365+
// Perhaps a CORS Filter has already added this?
366+
if (!CollectionUtils.isEmpty(responseHeaders.get("Access-Control-Allow-Origin"))) {
367+
logger.debug("Skip adding CORS headers, response already contains \"Access-Control-Allow-Origin\"");
368+
return;
369+
}
370+
}
371+
catch (NullPointerException npe) {
372+
// See SPR-11919 and https://issues.jboss.org/browse/WFLY-3474
369373
}
370374

371375
String origin = requestHeaders.getFirst("origin");
@@ -434,6 +438,7 @@ else if (HttpMethod.OPTIONS.equals(request.getMethod())) {
434438
}
435439
};
436440

441+
437442
private final SockJsRequestHandler iframeHandler = new SockJsRequestHandler() {
438443

439444
private static final String IFRAME_CONTENT =

spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,18 @@
2323
import org.springframework.http.HttpStatus;
2424
import org.springframework.http.server.ServerHttpRequest;
2525
import org.springframework.http.server.ServerHttpResponse;
26+
import org.springframework.http.server.ServletServerHttpResponse;
2627
import org.springframework.scheduling.TaskScheduler;
2728
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
2829
import org.springframework.web.socket.AbstractHttpRequestTests;
2930
import org.springframework.web.socket.WebSocketHandler;
3031
import org.springframework.web.socket.sockjs.SockJsException;
3132

3233
import static org.junit.Assert.*;
34+
import static org.mockito.Mockito.*;
35+
36+
import javax.servlet.ServletOutputStream;
37+
import javax.servlet.http.HttpServletResponse;
3338

3439
/**
3540
* Test fixture for {@link AbstractSockJsService}.
@@ -106,6 +111,21 @@ public void handleInfoGetCorsFilter() throws Exception {
106111
assertEquals("foobar:123", this.servletResponse.getHeader("Access-Control-Allow-Origin"));
107112
}
108113

114+
// SPR-11919
115+
116+
@Test
117+
public void handleInfoGetWildflyNPE() throws Exception {
118+
HttpServletResponse mockResponse = mock(HttpServletResponse.class);
119+
ServletOutputStream ous = mock(ServletOutputStream.class);
120+
when(mockResponse.getHeaders("Access-Control-Allow-Origin")).thenThrow(NullPointerException.class);
121+
when(mockResponse.getOutputStream()).thenReturn(ous);
122+
this.response = new ServletServerHttpResponse(mockResponse);
123+
124+
handleRequest("GET", "/echo/info", HttpStatus.OK);
125+
126+
verify(mockResponse, times(1)).getOutputStream();
127+
}
128+
109129
@Test
110130
public void handleInfoOptions() throws Exception {
111131

0 commit comments

Comments
 (0)