From 9f61d504466c8f2796c580b25c2b8f2b62953a17 Mon Sep 17 00:00:00 2001 From: kibeom lee <70303094+2012160085@users.noreply.github.com> Date: Sun, 22 Sep 2024 15:10:34 +0900 Subject: [PATCH 1/7] Add unit test to verify cookie behavior with Max-Age=0 --- .../servlet/AwsHttpServletResponseTest.java | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponseTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponseTest.java index 6c951a4f1..88d2a5e74 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponseTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponseTest.java @@ -17,6 +17,7 @@ import java.text.SimpleDateFormat; import java.time.Instant; import java.util.Calendar; +import java.util.Locale; import java.util.TimeZone; import java.util.concurrent.CountDownLatch; import java.util.regex.Matcher; @@ -36,7 +37,7 @@ public class AwsHttpServletResponseTest { private static final int MAX_AGE_VALUE = 300; private static final Pattern MAX_AGE_PATTERN = Pattern.compile("Max-Age=(-?[0-9]+)"); - private static final Pattern EXPIRES_PATTERN = Pattern.compile("Expires=(.*)$"); + private static final Pattern EXPIRES_PATTERN = Pattern.compile("Expires=([^;]+)"); private static final String CONTENT_TYPE_WITH_CHARSET = "application/json; charset=UTF-8"; private static final String JAVASCRIPT_CONTENT_TYPE_WITH_CHARSET = "application/javascript; charset=UTF-8"; @@ -144,6 +145,23 @@ void cookie_addCookieWithoutMaxAge_expectNoExpires() { assertFalse(cookieHeader.contains("Expires")); } + @Test + void cookie_addCookieWithMaxAgeZero_expectExpiresInThePast() { + AwsHttpServletResponse resp = new AwsHttpServletResponse(null, null); + Cookie zeroMaxAgeCookie = new Cookie(COOKIE_NAME, COOKIE_VALUE); + zeroMaxAgeCookie.setMaxAge(0); + + resp.addCookie(zeroMaxAgeCookie); + String cookieHeader = resp.getHeader(HttpHeaders.SET_COOKIE); + + Calendar cal = getExpires(cookieHeader); + long currentTimeMillis = System.currentTimeMillis(); + + assertNotNull(cookieHeader); + assertTrue(cal.getTimeInMillis() < currentTimeMillis); + assertTrue(cookieHeader.contains(COOKIE_NAME + "=" + COOKIE_VALUE)); + } + @Test void responseHeaders_getAwsResponseHeaders_expectLatestHeader() { AwsHttpServletResponse resp = new AwsHttpServletResponse(null, null); @@ -352,7 +370,7 @@ private Calendar getExpires(String header) { assertTrue(ageMatcher.find()); assertTrue(ageMatcher.groupCount() >= 1); String expiresString = ageMatcher.group(1); - SimpleDateFormat sdf = new SimpleDateFormat(AwsHttpServletResponse.HEADER_DATE_PATTERN); + SimpleDateFormat sdf = new SimpleDateFormat(AwsHttpServletResponse.HEADER_DATE_PATTERN, Locale.US); Calendar cal = Calendar.getInstance(); try { cal.setTime(sdf.parse(expiresString)); From 3a720953f276b76207f099e665386c146683ec71 Mon Sep 17 00:00:00 2001 From: kibeom lee <70303094+2012160085@users.noreply.github.com> Date: Sun, 22 Sep 2024 17:15:35 +0900 Subject: [PATCH 2/7] Add Servlet request tests to validate RFC 6265 cookie compliance --- .../servlet/AwsHttpServletRequestTest.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java index 2bc433041..358d673a1 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java @@ -26,6 +26,12 @@ public class AwsHttpServletRequestTest { .header(HttpHeaders.CONTENT_TYPE, "application/xml; charset=utf-8").build(); private static final AwsProxyRequest validCookieRequest = new AwsProxyRequestBuilder("/cookie", "GET") .header(HttpHeaders.COOKIE, "yummy_cookie=choco; tasty_cookie=strawberry").build(); + private static final AwsProxyRequest controlCharCookieRequest = new AwsProxyRequestBuilder("/cookie", "GET") + .header(HttpHeaders.COOKIE, "name=\u0007\u0009; tasty_cookie=strawberry").build(); + private static final AwsProxyRequest unicodeCookieRequest = new AwsProxyRequestBuilder("/cookie", "GET") + .header(HttpHeaders.COOKIE, "yummy_cookie=chøcø; tasty_cookie=strawberry").build(); + private static final AwsProxyRequest invalidNameCookieRequest = new AwsProxyRequestBuilder("/cookie", "GET") + .header(HttpHeaders.COOKIE, "yummy@cookie=choco; tasty_cookie=strawberry").build(); private static final AwsProxyRequest complexAcceptHeader = new AwsProxyRequestBuilder("/accept", "GET") .header(HttpHeaders.ACCEPT, "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8").build(); private static final AwsProxyRequest queryString = new AwsProxyRequestBuilder("/test", "GET") @@ -75,6 +81,39 @@ void headers_parseHeaderValue_validMultipleCookie() { assertEquals("strawberry", values.get(1).getValue()); } + @Test + void headers_parseHeaderValue_controlCharCookie() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(controlCharCookieRequest, mockContext, null, config); + Cookie[] cookies = request.getCookies(); + + // parse only valid cookies + assertEquals(1, cookies.length); + assertEquals("tasty_cookie", cookies[0].getName()); + assertEquals("strawberry", cookies[0].getValue()); + } + + @Test + void headers_parseHeaderValue_unicodeCookie() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(unicodeCookieRequest, mockContext, null, config); + Cookie[] cookies = request.getCookies(); + + // parse only valid cookies + assertEquals(1, cookies.length); + assertEquals("tasty_cookie", cookies[0].getName()); + assertEquals("strawberry", cookies[0].getValue()); + } + + @Test + void headers_parseHeaderValue_invalidNameCookie() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(invalidNameCookieRequest, mockContext, null, config); + Cookie[] cookies = request.getCookies(); + + // parse only valid cookies + assertEquals(1, cookies.length); + assertEquals("tasty_cookie", cookies[0].getName()); + assertEquals("strawberry", cookies[0].getValue()); + } + @Test void headers_parseHeaderValue_complexAccept() { AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(complexAcceptHeader, mockContext, null, config); From c04fd1387cf4b20207abd43225ea06ffc8973277 Mon Sep 17 00:00:00 2001 From: kibeom lee <70303094+2012160085@users.noreply.github.com> Date: Sun, 22 Sep 2024 19:57:39 +0900 Subject: [PATCH 3/7] feat: Introduce CookieProcessor interface and refactor cookie handling - Created a `CookieProcessor` interface along with its implementation `AwsCookieProcessor` to encapsulate cookie parsing and formatting logic. - Modified `AwsHttpServletResponse` and `AwsHttpServletRequest` to use the `CookieProcessor` for all cookie-related operations. --- .../internal/servlet/AwsCookieProcessor.java | 275 ++++++++++++++++++ .../AwsHttpApiV2ProxyHttpServletRequest.java | 17 +- .../servlet/AwsHttpServletRequest.java | 15 +- .../servlet/AwsHttpServletResponse.java | 35 +-- .../internal/servlet/CookieProcessor.java | 23 ++ 5 files changed, 317 insertions(+), 48 deletions(-) create mode 100644 aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java create mode 100644 aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/CookieProcessor.java diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java new file mode 100644 index 000000000..2a8496082 --- /dev/null +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java @@ -0,0 +1,275 @@ +package com.amazonaws.serverless.proxy.internal.servlet; + +import com.amazonaws.serverless.proxy.internal.SecurityUtils; +import jakarta.servlet.http.Cookie; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.text.DateFormat; +import java.text.FieldPosition; +import java.text.SimpleDateFormat; +import java.util.*; + +/** + * Implementation of the CookieProcessor interface that provides cookie parsing and generation functionality. + */ +public class AwsCookieProcessor implements CookieProcessor { + + // Cookie attribute constants + static final String COOKIE_COMMENT_ATTR = "Comment"; + static final String COOKIE_DOMAIN_ATTR = "Domain"; + static final String COOKIE_MAX_AGE_ATTR = "Max-Age"; + static final String COOKIE_PATH_ATTR = "Path"; + static final String COOKIE_SECURE_ATTR = "Secure"; + static final String COOKIE_HTTP_ONLY_ATTR = "HttpOnly"; + static final String COOKIE_SAME_SITE_ATTR = "SameSite"; + static final String COOKIE_PARTITIONED_ATTR = "Partitioned"; + static final String EMPTY_STRING = ""; + + // BitSet to store valid token characters as defined in RFC 2616 + static final BitSet tokenValid = createTokenValidSet(); + + // BitSet to validate domain characters + static final BitSet domainValid = createDomainValidSet(); + + static final String COOKIE_DATE_PATTERN = "EEE, dd MMM yyyy HH:mm:ss z"; + + // ThreadLocal to ensure thread-safe creation of DateFormat instances for each thread + static final ThreadLocal COOKIE_DATE_FORMAT = ThreadLocal.withInitial(() -> { + DateFormat df = new SimpleDateFormat(COOKIE_DATE_PATTERN, Locale.US); + df.setTimeZone(TimeZone.getTimeZone("GMT")); + return df; + }); + + static final String ANCIENT_DATE = COOKIE_DATE_FORMAT.get().format(new Date(10000)); + + static BitSet createTokenValidSet() { + BitSet tokenSet = new BitSet(128); + for (char c = '0'; c <= '9'; c++) tokenSet.set(c); + for (char c = 'a'; c <= 'z'; c++) tokenSet.set(c); + for (char c = 'A'; c <= 'Z'; c++) tokenSet.set(c); + for (char c : "!#$%&'*+-.^_`|~".toCharArray()) tokenSet.set(c); + return tokenSet; + } + + static BitSet createDomainValidSet() { + BitSet domainValid = new BitSet(128); + for (char c = '0'; c <= '9'; c++) domainValid.set(c); + for (char c = 'a'; c <= 'z'; c++) domainValid.set(c); + for (char c = 'A'; c <= 'Z'; c++) domainValid.set(c); + domainValid.set('.'); + domainValid.set('-'); + return domainValid; + } + + private final Logger log = LoggerFactory.getLogger(AwsCookieProcessor.class); + + @Override + public Cookie[] parseCookieHeader(String cookieHeader) { + // Return an empty array if the input is null or empty after trimming + if (cookieHeader == null || cookieHeader.trim().isEmpty()) { + return new Cookie[0]; + } + + // Parse cookie header and convert to Cookie array + return Arrays.stream(cookieHeader.split("\\s*;\\s*")) + .map(this::parseCookiePair) + .filter(Objects::nonNull) // Filter out invalid pairs + .toArray(Cookie[]::new); + } + + /** + * Parse a single cookie pair (name=value). + * + * @param cookiePair The cookie pair string. + * @return A valid Cookie object or null if the pair is invalid. + */ + private Cookie parseCookiePair(String cookiePair) { + String[] kv = cookiePair.split("=", 2); + + if (kv.length != 2) { + log.warn("Ignoring invalid cookie: {}", cookiePair); + return null; // Skip malformed cookie pairs + } + + String cookieName = kv[0]; + String cookieValue = kv[1]; + + // Validate name and value + if (!isToken(cookieName)){ + log.warn("Ignoring cookie with invalid name: {}={}", cookieName, cookieValue); + return null; // Skip invalid cookie names + } + + if (!isValidCookieValue(cookieValue)) { + log.warn("Ignoring cookie with invalid value: {}={}", cookieName, cookieValue); + return null; // Skip invalid cookie values + } + + // Return a new Cookie object after security processing + return new Cookie(SecurityUtils.crlf(cookieName), SecurityUtils.crlf(cookieValue)); + } + + @Override + public String generateHeader(Cookie cookie) { + StringBuffer header = new StringBuffer(); + header.append(cookie.getName()).append('='); + + String value = cookie.getValue(); + if (value != null && value.length() > 0) { + validateCookieValue(value); + header.append(value); + } + + int maxAge = cookie.getMaxAge(); + if (maxAge > -1) { + header.append("; Expires="); + if (maxAge == 0) { + header.append(ANCIENT_DATE); + } else { + COOKIE_DATE_FORMAT.get().format( + new Date(System.currentTimeMillis() + maxAge * 1000L), header, new FieldPosition(0)); + header.append("; Max-Age=").append(maxAge); + } + } + + String domain = cookie.getDomain(); + if (domain != null && !domain.isEmpty()) { + validateDomain(domain); + header.append("; Domain=").append(domain); + } + + String path = cookie.getPath(); + if (path != null && !path.isEmpty()) { + validatePath(path); + header.append("; Path=").append(path); + } + + if (cookie.getSecure()) { + header.append("; Secure"); + } + + if (cookie.isHttpOnly()) { + header.append("; HttpOnly"); + } + + String sameSite = cookie.getAttribute(COOKIE_SAME_SITE_ATTR); + if (sameSite != null) { + header.append("; SameSite=").append(sameSite); + } + + String partitioned = cookie.getAttribute(COOKIE_PARTITIONED_ATTR); + if (EMPTY_STRING.equals(partitioned)) { + header.append("; Partitioned"); + } + + addAdditionalAttributes(cookie, header); + + return header.toString(); + } + + private void addAdditionalAttributes(Cookie cookie, StringBuffer header) { + for (Map.Entry entry : cookie.getAttributes().entrySet()) { + switch (entry.getKey()) { + case COOKIE_COMMENT_ATTR: + case COOKIE_DOMAIN_ATTR: + case COOKIE_MAX_AGE_ATTR: + case COOKIE_PATH_ATTR: + case COOKIE_SECURE_ATTR: + case COOKIE_HTTP_ONLY_ATTR: + case COOKIE_SAME_SITE_ATTR: + case COOKIE_PARTITIONED_ATTR: + // Already handled attributes are ignored + break; + default: + validateAttribute(entry.getKey(), entry.getValue()); + header.append("; ").append(entry.getKey()); + if (!EMPTY_STRING.equals(entry.getValue())) { + header.append('=').append(entry.getValue()); + } + break; + } + } + } + + private void validateCookieValue(String value) { + if (!isValidCookieValue(value)) { + throw new IllegalArgumentException("Invalid cookie value: " + value); + } + } + + private void validateDomain(String domain) { + if (!isValidDomain(domain)) { + throw new IllegalArgumentException("Invalid cookie domain: " + domain); + } + } + + private void validatePath(String path) { + for (char ch : path.toCharArray()) { + if (ch < 0x20 || ch > 0x7E || ch == ';') { + throw new IllegalArgumentException("Invalid cookie path: " + path); + } + } + } + + private void validateAttribute(String name, String value) { + if (!isToken(name)) { + throw new IllegalArgumentException("Invalid cookie attribute name: " + name); + } + + for (char ch : value.toCharArray()) { + if (ch < 0x20 || ch > 0x7E || ch == ';') { + throw new IllegalArgumentException("Invalid cookie attribute value: " + ch); + } + } + } + + private boolean isValidCookieValue(String value) { + int start = 0; + int end = value.length(); + boolean quoted = end > 1 && value.charAt(0) == '"' && value.charAt(end - 1) == '"'; + + char[] chars = value.toCharArray(); + for (int i = start; i < end; i++) { + if (quoted && (i == start || i == end - 1)) { + continue; + } + char c = chars[i]; + if (!isValidCookieChar(c)) return false; + } + return true; + } + + private boolean isValidDomain(String domain) { + if (domain.isEmpty()) { + return false; + } + int prev = -1; + for (char c : domain.toCharArray()) { + if (!domainValid.get(c) || isInvalidLabelStartOrEnd(prev, c)) { + return false; + } + prev = c; + } + return prev != '.' && prev != '-'; + } + + private boolean isInvalidLabelStartOrEnd(int prev, char current) { + return (prev == '.' || prev == -1) && (current == '.' || current == '-') || + (prev == '-' && current == '.'); + } + + private boolean isToken(String s) { + if (s.isEmpty()) return false; + for (char c : s.toCharArray()) { + if (!tokenValid.get(c)) { + return false; + } + } + return true; + } + + private boolean isValidCookieChar(char c) { + return !(c < 0x21 || c > 0x7E || c == 0x22 || c == 0x2c || c == 0x3b || c == 0x5c || c == 0x7f); + } +} diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java index 6fdb31f08..537e10759 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java @@ -37,7 +37,6 @@ import java.time.ZonedDateTime; import java.time.format.DateTimeParseException; import java.util.*; -import java.util.stream.Collectors; import java.util.stream.Stream; public class AwsHttpApiV2ProxyHttpServletRequest extends AwsHttpServletRequest { @@ -81,26 +80,14 @@ public Cookie[] getCookies() { if (headers == null || !headers.containsKey(HttpHeaders.COOKIE)) { rhc = new Cookie[0]; } else { - rhc = parseCookieHeaderValue(headers.getFirst(HttpHeaders.COOKIE)); + rhc = getCookieProcessor().parseCookieHeader(headers.getFirst(HttpHeaders.COOKIE)); } Cookie[] rc; if (request.getCookies() == null) { rc = new Cookie[0]; } else { - rc = request.getCookies().stream() - .map(c -> { - int i = c.indexOf('='); - if (i == -1) { - return null; - } else { - String k = SecurityUtils.crlf(c.substring(0, i)).trim(); - String v = SecurityUtils.crlf(c.substring(i+1)); - return new Cookie(k, v); - } - }) - .filter(c -> c != null) - .toArray(Cookie[]::new); + rc = getCookieProcessor().parseCookieHeader(String.join("; ", request.getCookies())); } return Stream.concat(Arrays.stream(rhc), Arrays.stream(rc)).toArray(Cookie[]::new); diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java index b76fd216e..ea8ef4a1a 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java @@ -90,6 +90,7 @@ public abstract class AwsHttpServletRequest implements HttpServletRequest { private String queryString; private Map> multipartFormParameters; private Map> urlEncodedFormParameters; + private CookieProcessor cookieProcessor; protected AwsHttpServletResponse response; protected AwsLambdaServletContainerHandler containerHandler; @@ -295,12 +296,7 @@ public void setServletContext(ServletContext context) { * @return An array of Cookie objects from the header */ protected Cookie[] parseCookieHeaderValue(String headerValue) { - List parsedHeaders = this.parseHeaderValue(headerValue, ";", ","); - - return parsedHeaders.stream() - .filter(e -> e.getKey() != null) - .map(e -> new Cookie(SecurityUtils.crlf(e.getKey()), SecurityUtils.crlf(e.getValue()))) - .toArray(Cookie[]::new); + return getCookieProcessor().parseCookieHeader(headerValue); } @@ -512,6 +508,13 @@ protected Map> getFormUrlEncodedParametersMap() { return urlEncodedFormParameters; } + protected CookieProcessor getCookieProcessor(){ + if (cookieProcessor == null) { + cookieProcessor = new AwsCookieProcessor(); + } + return cookieProcessor; + } + @Override public Collection getParts() throws IOException, ServletException { diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponse.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponse.java index f82d062a7..86a72ead6 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponse.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponse.java @@ -70,6 +70,7 @@ public class AwsHttpServletResponse private CountDownLatch writersCountDownLatch; private HttpServletRequest request; private boolean isCommitted = false; + private CookieProcessor cookieProcessor; private Logger log = LoggerFactory.getLogger(AwsHttpServletResponse.class); @@ -102,33 +103,7 @@ public void addCookie(Cookie cookie) { if (request != null && request.getDispatcherType() == DispatcherType.INCLUDE && isCommitted()) { throw new IllegalStateException("Cannot add Cookies for include request when response is committed"); } - String cookieData = cookie.getName() + "=" + cookie.getValue(); - if (cookie.getPath() != null) { - cookieData += "; Path=" + cookie.getPath(); - } - if (cookie.getSecure()) { - cookieData += "; Secure"; - } - if (cookie.isHttpOnly()) { - cookieData += "; HttpOnly"; - } - if (cookie.getDomain() != null && !"".equals(cookie.getDomain().trim())) { - cookieData += "; Domain=" + cookie.getDomain(); - } - - if (cookie.getMaxAge() > 0) { - cookieData += "; Max-Age=" + cookie.getMaxAge(); - - // we always set the timezone to GMT - TimeZone gmtTimeZone = TimeZone.getTimeZone(COOKIE_DEFAULT_TIME_ZONE); - Calendar currentTimestamp = Calendar.getInstance(gmtTimeZone); - currentTimestamp.add(Calendar.SECOND, cookie.getMaxAge()); - SimpleDateFormat cookieDateFormatter = new SimpleDateFormat(HEADER_DATE_PATTERN); - cookieDateFormatter.setTimeZone(gmtTimeZone); - cookieData += "; Expires=" + cookieDateFormatter.format(currentTimestamp.getTime()); - } - - setHeader(HttpHeaders.SET_COOKIE, cookieData, false); + setHeader(HttpHeaders.SET_COOKIE, getCookieProcessor().generateHeader(cookie), false); } @@ -500,6 +475,12 @@ AwsProxyRequest getAwsProxyRequest() { return (AwsProxyRequest)request.getAttribute(API_GATEWAY_EVENT_PROPERTY); } + CookieProcessor getCookieProcessor(){ + if (cookieProcessor == null) { + cookieProcessor = new AwsCookieProcessor(); + } + return cookieProcessor; + } //------------------------------------------------------------- // Methods - Private diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/CookieProcessor.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/CookieProcessor.java new file mode 100644 index 000000000..c59dc806a --- /dev/null +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/CookieProcessor.java @@ -0,0 +1,23 @@ +package com.amazonaws.serverless.proxy.internal.servlet; + +import jakarta.servlet.http.Cookie; + +public interface CookieProcessor { + /** + * Parse the provided cookie header value into an array of Cookie objects. + * + * @param cookieHeader The cookie header value string to parse, e.g., "SID=31d4d96e407aad42; lang=en-US" + * @return An array of Cookie objects parsed from the cookie header value + */ + Cookie[] parseCookieHeader(String cookieHeader); + + /** + * Generate the Set-Cookie HTTP header value for the given Cookie. + * + * @param cookie The cookie for which the header will be generated + * @return The header value in a form that can be added directly to the response + */ + String generateHeader(Cookie cookie); + + +} From 22bf190bf7172adc362b9f99280f17f1df2f6fbb Mon Sep 17 00:00:00 2001 From: kibeom lee <70303094+2012160085@users.noreply.github.com> Date: Mon, 23 Sep 2024 09:29:06 +0900 Subject: [PATCH 4/7] refactor: Use `java.time` API for thread-safe cookie expiration handling - Replaced the usage of `SimpleDateFormat` with `DateTimeFormatter` to ensure thread-safe date formatting. --- .../internal/servlet/AwsCookieProcessor.java | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java index 2a8496082..0293a3c61 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java @@ -4,10 +4,9 @@ import jakarta.servlet.http.Cookie; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - -import java.text.DateFormat; -import java.text.FieldPosition; -import java.text.SimpleDateFormat; +import java.time.Instant; +import java.time.ZoneId; +import java.time.format.DateTimeFormatter; import java.util.*; /** @@ -32,16 +31,9 @@ public class AwsCookieProcessor implements CookieProcessor { // BitSet to validate domain characters static final BitSet domainValid = createDomainValidSet(); - static final String COOKIE_DATE_PATTERN = "EEE, dd MMM yyyy HH:mm:ss z"; - - // ThreadLocal to ensure thread-safe creation of DateFormat instances for each thread - static final ThreadLocal COOKIE_DATE_FORMAT = ThreadLocal.withInitial(() -> { - DateFormat df = new SimpleDateFormat(COOKIE_DATE_PATTERN, Locale.US); - df.setTimeZone(TimeZone.getTimeZone("GMT")); - return df; - }); + static final DateTimeFormatter COOKIE_DATE_FORMATTER = DateTimeFormatter.RFC_1123_DATE_TIME.withZone(ZoneId.of("GMT")); - static final String ANCIENT_DATE = COOKIE_DATE_FORMAT.get().format(new Date(10000)); + static final String ANCIENT_DATE = COOKIE_DATE_FORMATTER.format(Instant.ofEpochMilli(10000)); static BitSet createTokenValidSet() { BitSet tokenSet = new BitSet(128); @@ -127,8 +119,8 @@ public String generateHeader(Cookie cookie) { if (maxAge == 0) { header.append(ANCIENT_DATE); } else { - COOKIE_DATE_FORMAT.get().format( - new Date(System.currentTimeMillis() + maxAge * 1000L), header, new FieldPosition(0)); + Instant expiresAt = Instant.now().plusSeconds(maxAge); + header.append(COOKIE_DATE_FORMATTER.format(expiresAt)); header.append("; Max-Age=").append(maxAge); } } From 4ab26c702f33528acd89957ade751524b396b88c Mon Sep 17 00:00:00 2001 From: kibeom lee <70303094+2012160085@users.noreply.github.com> Date: Mon, 23 Sep 2024 09:31:34 +0900 Subject: [PATCH 5/7] refactor: Remove redundant check in cookie character validation --- .../serverless/proxy/internal/servlet/AwsCookieProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java index 0293a3c61..7be78b03e 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java @@ -262,6 +262,6 @@ private boolean isToken(String s) { } private boolean isValidCookieChar(char c) { - return !(c < 0x21 || c > 0x7E || c == 0x22 || c == 0x2c || c == 0x3b || c == 0x5c || c == 0x7f); + return !(c < 0x21 || c > 0x7E || c == 0x22 || c == 0x2c || c == 0x3b || c == 0x5c); } } From 1770ccac825810476865ae4445cdb808fe667b9c Mon Sep 17 00:00:00 2001 From: kibeom lee <70303094+2012160085@users.noreply.github.com> Date: Sat, 28 Sep 2024 03:33:17 +0900 Subject: [PATCH 6/7] refactor: added helper methods to simplify cookie attribute appending. --- .../internal/servlet/AwsCookieProcessor.java | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java index 7be78b03e..133d07edb 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java @@ -17,6 +17,7 @@ public class AwsCookieProcessor implements CookieProcessor { // Cookie attribute constants static final String COOKIE_COMMENT_ATTR = "Comment"; static final String COOKIE_DOMAIN_ATTR = "Domain"; + static final String COOKIE_EXPIRES_ATTR = "Expires"; static final String COOKIE_MAX_AGE_ATTR = "Max-Age"; static final String COOKIE_PATH_ATTR = "Path"; static final String COOKIE_SECURE_ATTR = "Secure"; @@ -114,45 +115,42 @@ public String generateHeader(Cookie cookie) { } int maxAge = cookie.getMaxAge(); - if (maxAge > -1) { - header.append("; Expires="); - if (maxAge == 0) { - header.append(ANCIENT_DATE); - } else { - Instant expiresAt = Instant.now().plusSeconds(maxAge); - header.append(COOKIE_DATE_FORMATTER.format(expiresAt)); - header.append("; Max-Age=").append(maxAge); - } + if (maxAge == 0) { + appendAttribute(header, COOKIE_EXPIRES_ATTR, ANCIENT_DATE); + } else if (maxAge > 0){ + Instant expiresAt = Instant.now().plusSeconds(maxAge); + appendAttribute(header, COOKIE_EXPIRES_ATTR, COOKIE_DATE_FORMATTER.format(expiresAt)); + appendAttribute(header, COOKIE_MAX_AGE_ATTR, String.valueOf(maxAge)); } String domain = cookie.getDomain(); if (domain != null && !domain.isEmpty()) { validateDomain(domain); - header.append("; Domain=").append(domain); + appendAttribute(header, COOKIE_DOMAIN_ATTR, domain); } String path = cookie.getPath(); if (path != null && !path.isEmpty()) { validatePath(path); - header.append("; Path=").append(path); + appendAttribute(header, COOKIE_PATH_ATTR, path); } if (cookie.getSecure()) { - header.append("; Secure"); + appendAttributeWithoutValue(header, COOKIE_SECURE_ATTR); } if (cookie.isHttpOnly()) { - header.append("; HttpOnly"); + appendAttributeWithoutValue(header, COOKIE_HTTP_ONLY_ATTR); } String sameSite = cookie.getAttribute(COOKIE_SAME_SITE_ATTR); if (sameSite != null) { - header.append("; SameSite=").append(sameSite); + appendAttribute(header, COOKIE_SAME_SITE_ATTR, sameSite); } String partitioned = cookie.getAttribute(COOKIE_PARTITIONED_ATTR); if (EMPTY_STRING.equals(partitioned)) { - header.append("; Partitioned"); + appendAttributeWithoutValue(header, COOKIE_PARTITIONED_ATTR); } addAdditionalAttributes(cookie, header); @@ -160,7 +158,18 @@ public String generateHeader(Cookie cookie) { return header.toString(); } - private void addAdditionalAttributes(Cookie cookie, StringBuffer header) { + private void appendAttribute(StringBuilder header, String name, String value) { + header.append("; ").append(name); + if (!EMPTY_STRING.equals(value)) { + header.append('=').append(value); + } + } + + private void appendAttributeWithoutValue(StringBuilder header, String name) { + header.append("; ").append(name); + } + + private void addAdditionalAttributes(Cookie cookie, StringBuilder header) { for (Map.Entry entry : cookie.getAttributes().entrySet()) { switch (entry.getKey()) { case COOKIE_COMMENT_ATTR: @@ -175,10 +184,7 @@ private void addAdditionalAttributes(Cookie cookie, StringBuffer header) { break; default: validateAttribute(entry.getKey(), entry.getValue()); - header.append("; ").append(entry.getKey()); - if (!EMPTY_STRING.equals(entry.getValue())) { - header.append('=').append(entry.getValue()); - } + appendAttribute(header, entry.getKey(), entry.getValue()); break; } } From dfcc006fe63cb7ff231625c359d02e628ac5d0f9 Mon Sep 17 00:00:00 2001 From: kibeom lee <70303094+2012160085@users.noreply.github.com> Date: Sat, 28 Sep 2024 03:33:33 +0900 Subject: [PATCH 7/7] refactor: replaced StringBuffer with StringBuilder. --- .../serverless/proxy/internal/servlet/AwsCookieProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java index 133d07edb..36ade344c 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsCookieProcessor.java @@ -105,7 +105,7 @@ private Cookie parseCookiePair(String cookiePair) { @Override public String generateHeader(Cookie cookie) { - StringBuffer header = new StringBuffer(); + StringBuilder header = new StringBuilder(); header.append(cookie.getName()).append('='); String value = cookie.getValue();