Merge pull request #119 from awslabs/spring

Fixing forward and include filter issues with Spring - this closes all issues discovered while investigating #105.

Author: sapessi

aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java

@@ -101,7 +101,7 @@ public abstract class AwsHttpServletRequest implements HttpServletRequest {
 
     @Override
     public String getRequestedSessionId() {
-        throw new UnsupportedOperationException();
+        return null;
     }
 
 

aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsLambdaServletContainerHandler.java

@@ -22,12 +22,16 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.servlet.DispatcherType;
 import javax.servlet.FilterChain;
 import javax.servlet.Servlet;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
+import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpServletResponseWrapper;
+
 import java.io.IOException;
 
 /**
@@ -85,7 +89,7 @@ protected AwsLambdaServletContainerHandler(RequestReader<RequestType, ContainerR
     public void forward(ContainerRequestType servletRequest, ContainerResponseType servletResponse)
             throws ServletException, IOException {
         try {
-            handleRequest(servletRequest, servletResponse, lambdaContext);
+            handleRequest(servletRequest, (ContainerResponseType)getServletResponse(servletResponse), lambdaContext);
         } catch (Exception e) {
             log.error("Could not forward request", e);
             throw new ServletException(e);
@@ -103,13 +107,28 @@ public void forward(ContainerRequestType servletRequest, ContainerResponseType s
     public void include(ContainerRequestType servletRequest, ContainerResponseType servletResponse)
             throws ServletException, IOException {
         try {
-           handleRequest(servletRequest, servletResponse, lambdaContext);
+           handleRequest(servletRequest, (ContainerResponseType)getServletResponse(servletResponse), lambdaContext);
         } catch (Exception e) {
             log.error("Could not include request", e);
             throw new ServletException(e);
         }
     }
 
+    private HttpServletResponse getServletResponse(ContainerResponseType resp) {
+        if (HttpServletResponseWrapper.class.isAssignableFrom(resp.getClass())) {
+            ServletResponse servletResp = ((HttpServletResponseWrapper)resp).getResponse();
+            assert servletResp instanceof HttpServletResponse : servletResp.getClass();
+            return (HttpServletResponse)servletResp;
+        }
+
+        if (HttpServletResponse.class.isAssignableFrom(resp.getClass())) {
+            return resp;
+        }
+
+
+        throw new UnsupportedOperationException("Response type of " + resp.getClass().getName() + " is not supported");
+    }
+
 
     /**
      * You can use the <code>onStartup</code> to intercept the ServletContext as the Spring application is
@@ -189,6 +208,11 @@ protected FilterChain getFilterChain(ContainerRequestType req, Servlet servlet)
     protected void doFilter(ContainerRequestType request, ContainerResponseType response, Servlet servlet) throws IOException, ServletException {
         FilterChain chain = getFilterChain(request, servlet);
         chain.doFilter(request, response);
+
+        // if for some reason the response wasn't flushed yet, we force it here.
+        if (request.getDispatcherType() != DispatcherType.FORWARD && request.getDispatcherType() != DispatcherType.INCLUDE && !response.isCommitted()) {
+            response.flushBuffer();
+        }
     }
 
     //-------------------------------------------------------------

aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyRequestDispatcher.java

@@ -1,6 +1,8 @@
 package com.amazonaws.serverless.proxy.internal.servlet;
 
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 import javax.servlet.DispatcherType;
 import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletException;
@@ -61,6 +63,7 @@ public void forward(ServletRequest servletRequest, ServletResponse servletRespon
         ((AwsProxyHttpServletRequest) servletRequest).setDispatcherType(DispatcherType.FORWARD);
         ((AwsProxyHttpServletRequest) servletRequest).getAwsProxyRequest().setPath(dispatchPath);
 
+        assert servletResponse instanceof HttpServletResponse : servletResponse.getClass();
         lambdaContainerHandler.forward((HttpServletRequest)servletRequest, (HttpServletResponse)servletResponse);
     }
 
@@ -80,6 +83,7 @@ public void include(ServletRequest servletRequest, ServletResponse servletRespon
         ((AwsProxyHttpServletRequest) servletRequest).setDispatcherType(DispatcherType.INCLUDE);
         ((AwsProxyHttpServletRequest) servletRequest).getAwsProxyRequest().setPath(dispatchPath);
 
+        assert servletResponse instanceof HttpServletResponse : servletResponse.getClass();
         lambdaContainerHandler.include((HttpServletRequest)servletRequest, (HttpServletResponse)servletResponse);
     }
 }

aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainHolder.java

@@ -17,7 +17,6 @@
 import org.slf4j.LoggerFactory;
 
 import javax.servlet.*;
-import javax.servlet.http.HttpServletRequest;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -86,11 +85,6 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
             log.debug("Executed {}: filter {}-{}", servletRequest.getDispatcherType(),
                       currentFilter, holder.getFilterName());
         }
-
-        // if for some reason the response wasn't flushed yet, we force it here.
-        if (!servletResponse.isCommitted()) {
-            servletResponse.flushBuffer();
-        }
     }
 
 

aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponseTest.java

@@ -17,6 +17,9 @@
 
 
 public class AwsHttpServletResponseTest {
+    // we use this int to compare the cookie expiration time in the tests. The date we generate to compare to
+    // may be slight off compared to the date generated during the request processing
+    private static final int COOKIE_GRACE_COMPARE_MILLIS = 2000;
     private static final String COOKIE_NAME = "session_id";
     private static final String COOKIE_VALUE = "123";
     private static final String COOKIE_PATH = "/api";
@@ -121,8 +124,9 @@ public void cookie_addCookie_positiveMaxAgeExpiresDate() {
         Calendar expiration = getExpires(cookieHeader);
         System.out.println("Cookie date: " + dateFormat.format(expiration.getTime()));
         System.out.println("Test date: " + dateFormat.format(testExpiration.getTime()));
-        // we need to compare strings because the millis time will be off
-        assertEquals(dateFormat.format(testExpiration.getTime()), dateFormat.format(expiration.getTime()));
+
+        long dateDiff = testExpiration.getTimeInMillis() - expiration.getTimeInMillis();
+        assertTrue(Math.abs(dateDiff) < COOKIE_GRACE_COMPARE_MILLIS);
     }
 
     @Test

aws-serverless-java-container-jersey/src/main/java/com/amazonaws/serverless/proxy/jersey/JerseyServletResponseWriter.java

@@ -60,6 +60,7 @@ class JerseyServletResponseWriter
      * @param resp The current ServletResponse from the container
      */
     public JerseyServletResponseWriter(ServletResponse resp, CountDownLatch latch) {
+        assert resp instanceof HttpServletResponse;
         servletResponse = (HttpServletResponse)resp;
         jerseyLatch = latch;
     }

aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringBootAppTest.java

@@ -9,10 +9,13 @@
 import com.amazonaws.serverless.proxy.spring.springbootapp.LambdaHandler;
 import com.amazonaws.serverless.proxy.spring.springbootapp.TestController;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.Map;
 
 import static org.junit.Assert.*;
 
@@ -30,6 +33,29 @@ public void testMethod_springSecurity_doesNotThrowException() {
         validateSingleValueModel(resp, TestController.TEST_VALUE);
     }
 
+    @Test
+    public void defaultError_requestForward_springBootForwardsToDefaultErrorPage() {
+        AwsProxyRequest req = new AwsProxyRequestBuilder("/test2", "GET").build();
+        AwsProxyResponse resp = handler.handleRequest(req, context);
+        assertNotNull(resp);
+        assertEquals(404, resp.getStatusCode());
+        assertNotNull(resp.getHeaders());
+        assertTrue(resp.getHeaders().containsKey("Content-Type"));
+        assertEquals("application/json;charset=UTF-8", resp.getHeaders().get("Content-Type"));
+        try {
+            JsonNode errorData = mapper.readTree(resp.getBody());
+            assertNotNull(errorData.findValue("status"));
+            assertEquals(404, errorData.findValue("status").asInt());
+            assertNotNull(errorData.findValue("message"));
+            assertEquals("No message available", errorData.findValue("message").asText());
+
+        } catch (IOException e) {
+            e.printStackTrace();
+            fail();
+        }
+
+    }
+
     private void validateSingleValueModel(AwsProxyResponse output, String value) {
         try {
             SingleValueModel response = mapper.readValue(output.getBody(), SingleValueModel.class);