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:09:34 UTC
[tomcat] branch 10.0.x 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 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.0.x by this push:
new c2500fd Fix BZ 65505. HTTP header removal must not change order of other headers
c2500fd is described below
commit c2500fd06d20cdad03f51a53a3f102e1675b4517
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 65f1702..9cee208 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -143,6 +143,10 @@
the expected effect. <code>NONE</code> was incorrectly treated as a file
path. Patch provided by Mikael Sterner. (markt)
</fix>
+ <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