Fixed a potential null pointer in security utils and simplified header processing to continue addressing the feedback in #263

Author: sapessi

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

@@ -70,6 +70,9 @@ public static boolean isValidHost(String host, String apiId, String region) {
      * @return A copy of the original string without CRLF characters
      */
     public static String crlf(String s) {
+        if (s == null) {
+            return null;
+        }
         return s.replaceAll("[\r\n]", "");
     }
 

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

@@ -13,18 +13,13 @@
 package com.amazonaws.serverless.proxy.internal.servlet;
 
 import com.amazonaws.serverless.proxy.RequestReader;
-import com.amazonaws.serverless.proxy.internal.LambdaContainerHandler;
 import com.amazonaws.serverless.proxy.internal.SecurityUtils;
 import com.amazonaws.serverless.proxy.model.AwsProxyRequestContext;
 import com.amazonaws.serverless.proxy.model.ContainerConfig;
 import com.amazonaws.serverless.proxy.model.MultiValuedTreeMap;
 import com.amazonaws.services.lambda.runtime.Context;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
-import org.apache.http.HeaderElement;
 import org.apache.http.message.BasicHeaderValueParser;
-import org.apache.http.message.ParserCursor;
-import org.apache.http.util.CharArrayBuffer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -376,31 +371,44 @@ protected List<HeaderValue> parseHeaderValue(String headerValue, String valueSep
             newValue.setRawValue(v);
 
             for (String q : curValue.split(qualifierSeparator)) {
-                // contains key/value pairs and it's not a base64-encoded value.
-                if (q.contains(HEADER_KEY_VALUE_SEPARATOR) && !q.trim().endsWith("==")) {
-                    String[] kv = q.split(HEADER_KEY_VALUE_SEPARATOR);
-                    String key = kv[0].trim();
-                    String val = null;
-                    if (kv.length > 1) {
-                        val = kv[1].trim();
+
+                String[] kv = q.split(HEADER_KEY_VALUE_SEPARATOR, 2);
+                String key = null;
+                String val = null;
+                // no separator, set the value only
+                if (kv.length == 1) {
+                    val = q.trim();
+                }
+                // we have a separator
+                if (kv.length == 2) {
+                    // if the length of the value is 0 we assume that we are looking at a
+                    // base64 encoded value with padding so we just set the value. This is because
+                    // we assume that empty values in a key/value pair will contain at least a white space
+                    if (kv[1].length() == 0) {
+                        val = q.trim();
                     }
-                    // TODO: Should we concatenate the rest of the values?
-                    if (newValue.getValue() == null) {
-                        newValue.setKey(key);
-                        newValue.setValue(val);
-                    } else {
-                        // special case for quality q=
-                        if ("q".equals(key)) {
-                            curPreference = Float.parseFloat(val);
-                        } else {
-                            newValue.addAttribute(key, val);
-                        }
+                    // this was a base64 string with an additional = for padding, set the value only
+                    if ("=".equals(kv[1].trim())) {
+                        val = q.trim();
+                    } else { // it's a proper key/value set both
+                        key = kv[0].trim();
+                        val = ("".equals(kv[1].trim()) ? null : kv[1].trim());
                     }
+                }
+
+                if (newValue.getValue() == null) {
+                    newValue.setKey(key);
+                    newValue.setValue(val);
                 } else {
-                    newValue.setValue(q.trim());
+                    // special case for quality q=
+                    if ("q".equals(key)) {
+                        curPreference = Float.parseFloat(val);
+                    } else {
+                        newValue.addAttribute(key, val);
+                    }
                 }
-                newValue.setPriority(curPreference);
             }
+            newValue.setPriority(curPreference);
             values.add(newValue);
         }
 
@@ -428,7 +436,6 @@ protected String decodeRequestPath(String requestPath, ContainerConfig config) {
 
     }
 
-
     /**
      * Class that represents a header value.
      */

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

@@ -10,6 +10,7 @@
 import org.junit.Test;
 
 import javax.servlet.ServletException;
+import javax.servlet.http.Cookie;
 import javax.ws.rs.core.HttpHeaders;
 
 import static org.junit.Assert.*;
@@ -85,10 +86,48 @@ public void headers_parseHeaderValue_encodedContentWithEquals() {
         String value = Base64.getUrlEncoder().encodeToString("a".getBytes());
 
         List<AwsHttpServletRequest.HeaderValue> result = context.parseHeaderValue(value);
-
+        assertTrue(result.size() > 0);
         assertEquals("YQ==", result.get(0).getValue());
     }
 
+    @Test
+    public void headers_parseHeaderValue_base64EncodedCookieValue() {
+        String value = Base64.getUrlEncoder().encodeToString("a".getBytes());
+        String cookieValue = "jwt=" + value + "; secondValue=second";
+        AwsProxyRequest req = new AwsProxyRequestBuilder("/test", "GET").header(HttpHeaders.COOKIE, cookieValue).build();
+        AwsHttpServletRequest context = new AwsProxyHttpServletRequest(req,null,null);
+
+        Cookie[] cookies = context.getCookies();
+
+        assertEquals(2, cookies.length);
+        assertEquals("jwt", cookies[0].getName());
+        assertEquals(value, cookies[0].getValue());
+    }
+
+    @Test
+    public void headers_parseHeaderValue_cookieWithSeparatorInValue() {
+        String cookieValue = "jwt==test; secondValue=second";
+        AwsProxyRequest req = new AwsProxyRequestBuilder("/test", "GET").header(HttpHeaders.COOKIE, cookieValue).build();
+        AwsHttpServletRequest context = new AwsProxyHttpServletRequest(req,null,null);
+
+        Cookie[] cookies = context.getCookies();
+
+        assertEquals(2, cookies.length);
+        assertEquals("jwt", cookies[0].getName());
+        assertEquals("=test", cookies[0].getValue());
+    }
+
+    @Test
+    public void headers_parseHeaderValue_headerWithPaddingButNotBase64Encoded() {
+        AwsHttpServletRequest context = new AwsProxyHttpServletRequest(null,null,null);
+
+        List<AwsHttpServletRequest.HeaderValue> result = context.parseHeaderValue("hello=");
+        assertTrue(result.size() > 0);
+        assertEquals("hello", result.get(0).getKey());
+        System.out.println("\"" + result.get(0).getValue() + "\"");
+        assertNull(result.get(0).getValue());
+    }
+
     @Test
     public void queryString_generateQueryString_validQuery() {
         AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(queryString, mockContext, null, config);