Updates dispatcher to not use sendError when non error code http status.

Rather it uses setStatus and writes content like a normal response. Using sendError causes the servlet container to include it’s own content indicating the request is an error, which throws off the desired response.
geoserver · Jan 2, 2017 · 2c7b7fe · 2c7b7fe
1 parent e780412
commit 2c7b7fe
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 10 deletions.
diff --git a/src/ows/src/main/java/org/geoserver/ows/Dispatcher.java b/src/ows/src/main/java/org/geoserver/ows/Dispatcher.java
@@ -1634,14 +1634,25 @@ void exception(Throwable t, Service service, Request request) {
             } else {
                 logger.log(Level.FINER, "", t);
             }
-
+
+            boolean isError = ece.getErrorCode() >= 400;
+            HttpServletResponse rsp = request.getHttpResponse();
             try {
-                if(ece.getMessage() != null) {
-                    request.getHttpResponse().sendError(ece.getErrorCode(),ece.getMessage());
-                } else {
-                    request.getHttpResponse().sendError(ece.getErrorCode());
+                if (isError) {
+                    if (ece.getMessage() != null) {
+                        rsp.sendError(ece.getErrorCode(), ece.getMessage());
+                    }
+                    else {
+                        rsp.sendError(ece.getErrorCode());
+                    }
+                }
+                else {
+                    rsp.setStatus(ece.getErrorCode());
+                    if (ece.getMessage() != null) {
+                        rsp.getOutputStream().print(ece.getMessage());
+                    }
                 }
-                if (ece.getErrorCode() < 400) {
+                if (!isError) {
                     // gwc returns an HttpErrorCodeException for 304s
                     // we don't want to flag these as errors for upstream filters, ie the monitoring extension
                     t = null;

diff --git a/src/ows/src/test/java/org/geoserver/ows/DispatcherTest.java b/src/ows/src/test/java/org/geoserver/ows/DispatcherTest.java
@@ -342,14 +342,18 @@ public int available(){
     }
 
     public void testHttpErrorCodeException() throws Exception {
-    	assertHttpErrorCode("httpErrorCodeException");
+    	assertHttpErrorCode("httpErrorCodeException", HttpServletResponse.SC_NO_CONTENT);
     }
 
     public void testWrappedHttpErrorCodeException() throws Exception {
-        assertHttpErrorCode("wrappedHttpErrorCodeException");
+        assertHttpErrorCode("wrappedHttpErrorCodeException", HttpServletResponse.SC_NO_CONTENT);
     }
 
-    private void assertHttpErrorCode(String requestType) throws Exception {
+    public void testBadRequestHttpErrorCodeException() throws Exception {
+        assertHttpErrorCode("badRequestHttpErrorCodeException", HttpServletResponse.SC_BAD_REQUEST);
+    }
+
+    private void assertHttpErrorCode(String requestType, int expectedCode) throws Exception {
         URL url = getClass().getResource("applicationContext.xml");
 
         FileSystemXmlApplicationContext context = new FileSystemXmlApplicationContext(url.toString());
@@ -389,7 +393,9 @@ public void setCharacterEncoding(String encoding) {
         request.setQueryString("service=hello&request=hello&message=HelloWorld");
 
         dispatcher.handleRequest(request, response);
-        assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatusCode());
+        assertEquals(expectedCode, response.getStatusCode());
+
+        assertEquals(expectedCode >= 400, response.isError());
 	}
 
     /**

diff --git a/src/ows/src/test/java/org/geoserver/ows/HelloWorld.java b/src/ows/src/test/java/org/geoserver/ows/HelloWorld.java
@@ -27,4 +27,8 @@ public void wrappedHttpErrorCodeException() {
         	throw new ServiceException("Wrapping code error", e);
         }
     }
+
+    public void badRequestHttpErrorCodeException() {
+        throw new HttpErrorCodeException( HttpServletResponse.SC_BAD_REQUEST );
+    }
 }
diff --git a/src/ows/src/test/java/org/geoserver/ows/applicationContext.xml b/src/ows/src/test/java/org/geoserver/ows/applicationContext.xml
@@ -38,6 +38,7 @@
 		     <value>hello</value>
 		     <value>httpErrorCodeException</value>
 		     <value>wrappedHttpErrorCodeException</value>
+		     <value>badRequestHttpErrorCodeException</value>
 		   </list>
 		</constructor-arg>
 	</bean>

diff --git a/src/ows/src/test/java/org/geoserver/test/CodeExpectingHttpServletResponse.java b/src/ows/src/test/java/org/geoserver/test/CodeExpectingHttpServletResponse.java
@@ -16,6 +16,7 @@
  */
 public class CodeExpectingHttpServletResponse extends HttpServletResponseWrapper{
     private int myErrorCode;
+    private boolean error;
 
     public CodeExpectingHttpServletResponse (HttpServletResponse req){
         super(req);
@@ -33,11 +34,13 @@ public void setStatus(int sc, String sm){
     }
 
     public void sendError(int sc) throws IOException {
+        error = true;
         myErrorCode = sc;
         super.sendError(sc);
     }
 
     public void sendError(int sc, String sm) throws IOException {
+        error = true;
         myErrorCode = sc;
         super.sendError(sc, sm);
     }
@@ -49,6 +52,10 @@ public int getErrorCode(){
     public int getStatusCode(){
         return myErrorCode;
     }
+
+    public boolean isError() {
+        return error;
+    }
 }