You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by mo...@apache.org on 2021/02/25 22:45:06 UTC
[knox] branch master updated: KNOX-2538 - Make sure SET-COOKIE
attributes are ordered properly (#403)
This is an automated email from the ASF dual-hosted git repository.
more pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new 8159136 KNOX-2538 - Make sure SET-COOKIE attributes are ordered properly (#403)
8159136 is described below
commit 81591369501b82880987d09e5704d685d97aaf4a
Author: Sandeep Moré <mo...@gmail.com>
AuthorDate: Thu Feb 25 17:44:58 2021 -0500
KNOX-2538 - Make sure SET-COOKIE attributes are ordered properly (#403)
---
.../knox/gateway/dispatch/DefaultDispatch.java | 23 +++++++++++++-----
.../gateway/dispatch/ConfigurableDispatchTest.java | 27 ++++++++++++++++++++--
2 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java
index 4afb61e..bdc0596 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java
@@ -17,6 +17,8 @@
*/
package org.apache.knox.gateway.dispatch;
+import static java.util.stream.Collectors.toCollection;
+
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
@@ -58,7 +60,7 @@ import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
-import java.util.stream.Collectors;
+import java.util.LinkedHashSet;
public class DefaultDispatch extends AbstractGatewayDispatch {
protected static final String SET_COOKIE = "SET-COOKIE";
@@ -382,15 +384,24 @@ public class DefaultDispatch extends AbstractGatewayDispatch {
return ""; // we should exclude all -> there should not be any value added with this header
} else {
final String separator = SET_COOKIE.equalsIgnoreCase(headerNameToCheck) ? "; " : " ";
- Set<String> headerValuesToCheck;
+ /**
+ * special attention needs to be given to make sure we maintain
+ * the attribute order else bad things can happen.
+ * 1. String.split() always maintains the order in generated array
+ * 2. LinkedHashSet is an ordered set
+ * 3. *.stream().map() maintains the order *iff* the collection type is ordered
+ * 4. *.collect() needs to be ordered as well to make sure the generated set is ordered
+ */
+ LinkedHashSet<String> headerValuesToCheck;
if(headerToCheck.getName().equalsIgnoreCase(SET_COOKIE)) {
- headerValuesToCheck = new HashSet<>(Arrays.asList(headerToCheck.getValue().trim().split(";")));
+ /* make sure we maintain the order */
+ headerValuesToCheck = new LinkedHashSet<>(Arrays.asList(headerToCheck.getValue().trim().split(";")));
/* trim */
- headerValuesToCheck = headerValuesToCheck.stream().map(String::trim).collect(Collectors.toSet());
+ headerValuesToCheck = headerValuesToCheck.stream().map(String::trim).collect(toCollection(LinkedHashSet::new));
} else {
- headerValuesToCheck = new HashSet<>(Arrays.asList(headerToCheck.getValue().trim().split("\\s+")));
+ headerValuesToCheck = new LinkedHashSet<>(Arrays.asList(headerToCheck.getValue().trim().split("\\s+")));
}
- headerValuesToCheck = headerValuesToCheck.stream().map(h -> h.replaceAll(separator.trim(), "")).collect(Collectors.toSet());
+ headerValuesToCheck = headerValuesToCheck.stream().map(h -> h.replaceAll(separator.trim(), "")).collect(toCollection(LinkedHashSet::new));
headerValuesToCheck.removeIf(h -> excludedHeaderValues.stream().anyMatch(e -> h.contains(e)));
return headerValuesToCheck.isEmpty() ? "" : String.join(separator, headerValuesToCheck);
}
diff --git a/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/ConfigurableDispatchTest.java b/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/ConfigurableDispatchTest.java
index 7c341a6..954f229 100644
--- a/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/ConfigurableDispatchTest.java
+++ b/gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/ConfigurableDispatchTest.java
@@ -280,6 +280,28 @@ public class ConfigurableDispatchTest {
}
/**
+ * Make sure that SET-COOKIE attributes order is maintained
+ */
+ @Test
+ public void testSetCookieHeaderOrder() {
+ final String SET_COOKIE_VALUE = "JSESSIONID=ba760126-414f-406d-baa1-99e14eb47656; zx=yz; SameSite=none; Secure; Path=/; HttpOnly; a=b; c=d";
+ final String EXPECTED_SET_COOKIE_VALUE = "JSESSIONID=ba760126-414f-406d-baa1-99e14eb47656; zx=yz; SameSite=none; Path=/; HttpOnly; a=b; c=d";
+ final Header[] headers = new Header[] { new BasicHeader(SET_COOKIE, SET_COOKIE_VALUE) };
+ final HttpResponse inboundResponse = EasyMock.createNiceMock(HttpResponse.class);
+ EasyMock.expect(inboundResponse.getAllHeaders()).andReturn(headers).anyTimes();
+ EasyMock.replay(inboundResponse);
+
+ final ConfigurableDispatch dispatch = new ConfigurableDispatch();
+ final String setCookieExludeHeaders = SET_COOKIE + ": Secure";
+ dispatch.setResponseExcludeHeaders(setCookieExludeHeaders);
+
+ final HttpServletResponse outboundResponse = new MockHttpServletResponse();
+ dispatch.copyResponseHeaderFields(outboundResponse, inboundResponse);
+
+ assertThat(outboundResponse.getHeader(SET_COOKIE), is(EXPECTED_SET_COOKIE_VALUE));
+ }
+
+ /**
* When exclude list is defined and set-cookie is not in the list
* make sure auth cookies are blocked.
* @throws Exception
@@ -310,8 +332,9 @@ public class ConfigurableDispatchTest {
*/
@Test
public void testAllowSetCookieHeaderDirectivesDefault() throws Exception {
+ final String SET_COOKIE_VALUE = "RANGERADMINSESSIONID=5C0C1805BD3B43BA8E9FC04A63586505; Path=/; Secure; HttpOnly";
final Header[] headers = new Header[] {
- new BasicHeader(SET_COOKIE, "RANGERADMINSESSIONID=5C0C1805BD3B43BA8E9FC04A63586505; Path=/; Secure; HttpOnly")};
+ new BasicHeader(SET_COOKIE, SET_COOKIE_VALUE)};
final HttpResponse inboundResponse = EasyMock.createNiceMock(HttpResponse.class);
EasyMock.expect(inboundResponse.getAllHeaders()).andReturn(headers).anyTimes();
EasyMock.replay(inboundResponse);
@@ -325,7 +348,7 @@ public class ConfigurableDispatchTest {
dispatch.copyResponseHeaderFields(outboundResponse, inboundResponse);
assertThat(outboundResponse.getHeaderNames().size(), is(1));
- assertThat(outboundResponse.getHeader(SET_COOKIE), is("Secure; Path=/; HttpOnly; RANGERADMINSESSIONID=5C0C1805BD3B43BA8E9FC04A63586505"));
+ assertThat(outboundResponse.getHeader(SET_COOKIE), is(SET_COOKIE_VALUE));
}
/**