You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/08/24 12:07:29 UTC
[tomcat] branch main updated: Fix BZ 65505. HTTP header removal
must not change order of other headers
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 3bdd802 Fix BZ 65505. HTTP header removal must not change order of other headers
3bdd802 is described below
commit 3bdd80293fb2b88a8b3df50692a93047261bd082
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Aug 24 13:06:59 2021 +0100
Fix BZ 65505. HTTP header removal must not change order of other headers
If there are headers with multiple values, we must retain relative order
of those values. It is easier to retain order for all header values.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65505
https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.2
---
java/org/apache/tomcat/util/http/MimeHeaders.java | 18 +++++++++--
.../apache/tomcat/util/http/TestResponseUtil.java | 36 ++++++++++++++++++++++
webapps/docs/changelog.xml | 4 +++
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/tomcat/util/http/MimeHeaders.java b/java/org/apache/tomcat/util/http/MimeHeaders.java
index 113412c..2396c45 100644
--- a/java/org/apache/tomcat/util/http/MimeHeaders.java
+++ b/java/org/apache/tomcat/util/http/MimeHeaders.java
@@ -392,15 +392,27 @@ public class MimeHeaders {
}
/**
- * reset and swap with last header
+ * Reset, move to the end and then reduce count by 1.
* @param idx the index of the header to remove.
*/
public void removeHeader(int idx) {
- MimeHeaderField mh = headers[idx];
+ // Implementation note. This method must not change the order of the
+ // remaining headers because, if there are multiple header values for
+ // the same name, the order of those headers is significant. It is
+ // simpler to retain order for all values than try to determine if there
+ // are multiple header values for the same name.
+ // Clear the header to remove
+ MimeHeaderField mh = headers[idx];
mh.recycle();
- headers[idx] = headers[count - 1];
+
+ // Move the remaining headers
+ System.arraycopy(headers, idx + 1, headers, idx, count - idx -1);
+
+ // Place the removed header at the end
headers[count - 1] = mh;
+
+ // Reduce the count
count--;
}
diff --git a/test/org/apache/tomcat/util/http/TestResponseUtil.java b/test/org/apache/tomcat/util/http/TestResponseUtil.java
index 4c760e0..d988a20 100644
--- a/test/org/apache/tomcat/util/http/TestResponseUtil.java
+++ b/test/org/apache/tomcat/util/http/TestResponseUtil.java
@@ -214,4 +214,40 @@ public class TestResponseUtil {
}
Assert.assertEquals(expected, result);
}
+
+
+ /*
+ * https://bz.apache.org/bugzilla/show_bug.cgi?id=65505
+ */
+ @Test
+ public void testAddVaryHeaderOrder() {
+ MimeHeaders responseHeaders = new MimeHeaders();
+ responseHeaders.addValue("Vary").setString("Origin");
+ responseHeaders.addValue("Vary").setString("Access-Control-Request-Method");
+ responseHeaders.addValue("Vary").setString("Access-Control-Request-Headers");
+ responseHeaders.addValue("Access-Control-Allow-Origin").setString("https://xxxx");
+ responseHeaders.addValue("Access-Control-Allow-Credentials").setString("true");
+ responseHeaders.addValue("Set-Cookie").setString("rememberMe=deleteMe; Path=/; Max-Age=0; Expires=Tue, 17-Aug-2021 11:19:04 GMT; SameSite=lax");
+ responseHeaders.addValue("Set-Cookie").setString("rememberMe=rememberMeData; Path=/; Max-Age=1296000; Expires=Thu, 02-Sep-2021 11:19:04 GMT; HttpOnly; SameSite=lax");
+
+ String cookiesBefore = getHeaderValues(responseHeaders, "Set-Cookie");
+
+ ResponseUtil.addVaryFieldName(responseHeaders, "accept-encoding");
+
+ String cookiesAfter = getHeaderValues(responseHeaders, "Set-Cookie");
+
+ Assert.assertEquals(cookiesBefore, cookiesAfter);
+ }
+
+
+ private String getHeaderValues(MimeHeaders headers, String headerName) {
+ StringBuilder sb = new StringBuilder();
+ for (int i = 0; i < headers.size(); i++) {
+ if (headers.getName(i).equals(headerName)) {
+ sb.append(headers.getValue(i));
+ sb.append('\n');
+ }
+ }
+ return sb.toString();
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3c644e6..65eb5f7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -158,6 +158,10 @@
Refactor the JSSE/OpenSSL integration to avoid the use of
<code>finalize()</code>. (markt)
</scode>
+ <fix>
+ <bug>65505</bug>: When an HTTP header value is removed, ensure that the
+ order of the remaining header values is unchanged. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="WebSocket">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org