This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push: new 6c72f6ee5c Refactor storage of trailer fields to use MimeHeaders 6c72f6ee5c is described below commit 6c72f6ee5cceee0fc52fe5909d23321e62e02181 Author: Mark Thomas <ma...@apache.org> AuthorDate: Wed Apr 24 19:47:33 2024 +0100 Refactor storage of trailer fields to use MimeHeaders --- java/org/apache/catalina/connector/Request.java | 4 ++-- java/org/apache/coyote/Request.java | 15 +++++++++++++-- .../coyote/http11/filters/ChunkedInputFilter.java | 6 +++--- java/org/apache/coyote/http2/Stream.java | 2 +- java/org/apache/tomcat/util/buf/StringUtils.java | 5 +++++ java/org/apache/tomcat/util/http/MimeHeaders.java | 19 +++++++++++++++++++ webapps/docs/changelog.xml | 8 ++++++++ 7 files changed, 51 insertions(+), 8 deletions(-) diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java index c28a9bf216..19fb94f889 100644 --- a/java/org/apache/catalina/connector/Request.java +++ b/java/org/apache/catalina/connector/Request.java @@ -1870,8 +1870,8 @@ public class Request implements HttpServletRequest { if (!isTrailerFieldsReady()) { throw new IllegalStateException(sm.getString("coyoteRequest.trailersNotReady")); } - Map<String,String> result = new HashMap<>(coyoteRequest.getTrailerFields()); - return result; + // No need for a defensive copy since a new Map is returned for every call. + return coyoteRequest.getTrailerFields(); } diff --git a/java/org/apache/coyote/Request.java b/java/org/apache/coyote/Request.java index e6f1ea8ce2..72eb5171ff 100644 --- a/java/org/apache/coyote/Request.java +++ b/java/org/apache/coyote/Request.java @@ -94,7 +94,7 @@ public final class Request { private final MessageBytes localAddrMB = MessageBytes.newInstance(); private final MimeHeaders headers = new MimeHeaders(); - private final Map<String, String> trailerFields = new HashMap<>(); + private final MimeHeaders trailerFields = new MimeHeaders(); /** * Path parameters @@ -280,6 +280,11 @@ public final class Request { public Map<String, String> getTrailerFields() { + return trailerFields.toMap(); + } + + + public MimeHeaders getMimeTrailerFields() { return trailerFields; } @@ -721,7 +726,13 @@ public final class Request { characterEncoding = null; expectation = false; headers.recycle(); - trailerFields.clear(); + trailerFields.recycle(); + /* + * Trailer fields are limited in size by bytes. The following call ensures that any request with a large number + * of small trailer fields doesn't result in a long lasting, large array of headers inside the MimeHeader + * instance. + */ + trailerFields.setLimit(MimeHeaders.DEFAULT_HEADER_SIZE); serverNameMB.recycle(); serverPort = -1; localAddrMB.recycle(); diff --git a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java index c0b74a185f..83bccbd161 100644 --- a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java +++ b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.Locale; -import java.util.Map; import java.util.Set; import org.apache.coyote.ActionCode; @@ -32,6 +31,7 @@ import org.apache.coyote.http11.Constants; import org.apache.coyote.http11.InputFilter; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.HexUtils; +import org.apache.tomcat.util.http.MimeHeaders; import org.apache.tomcat.util.http.parser.HttpParser; import org.apache.tomcat.util.net.ApplicationBufferHandler; import org.apache.tomcat.util.res.StringManager; @@ -467,7 +467,7 @@ public class ChunkedInputFilter implements InputFilter, ApplicationBufferHandler * implementation wasn't viewed as practical. */ - Map<String,String> headers = request.getTrailerFields(); + MimeHeaders headers = request.getMimeTrailerFields(); byte chr = 0; @@ -629,7 +629,7 @@ public class ChunkedInputFilter implements InputFilter, ApplicationBufferHandler String value = new String(trailingHeaders.getBytes(), colonPos, lastSignificantChar - colonPos, StandardCharsets.ISO_8859_1); - headers.put(headerName, value); + headers.addValue(headerName).setString(value); } return true; diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java index c363082159..6d705e20b1 100644 --- a/java/org/apache/coyote/http2/Stream.java +++ b/java/org/apache/coyote/http2/Stream.java @@ -460,7 +460,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter { if (headerState == HEADER_STATE_TRAILER) { // HTTP/2 headers are already always lower case - coyoteRequest.getTrailerFields().put(name, value); + coyoteRequest.getMimeTrailerFields().addValue(name).setString(value); } else { coyoteRequest.getMimeHeaders().addValue(name).setString(value); } diff --git a/java/org/apache/tomcat/util/buf/StringUtils.java b/java/org/apache/tomcat/util/buf/StringUtils.java index f339021ce0..cdc52c6e05 100644 --- a/java/org/apache/tomcat/util/buf/StringUtils.java +++ b/java/org/apache/tomcat/util/buf/StringUtils.java @@ -34,6 +34,11 @@ public final class StringUtils { } + public static String join(String a, String b) { + return join(new String[] { a, b }); + } + + public static String join(String[] array) { if (array == null) { return EMPTY_STRING; diff --git a/java/org/apache/tomcat/util/http/MimeHeaders.java b/java/org/apache/tomcat/util/http/MimeHeaders.java index 070a709e82..48ec279a09 100644 --- a/java/org/apache/tomcat/util/http/MimeHeaders.java +++ b/java/org/apache/tomcat/util/http/MimeHeaders.java @@ -19,9 +19,13 @@ package org.apache.tomcat.util.http; import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.Collections; import java.util.Enumeration; +import java.util.HashMap; +import java.util.Map; import org.apache.tomcat.util.buf.MessageBytes; +import org.apache.tomcat.util.buf.StringUtils; import org.apache.tomcat.util.res.StringManager; /** @@ -148,6 +152,21 @@ public class MimeHeaders { } + public Map<String,String> toMap() { + if (count == 0) { + return Collections.emptyMap(); + } + Map<String,String> result = new HashMap<>(); + + for (int i = 0; i < count; i++) { + String name = headers[i].getName().toStringType(); + String value = headers[i].getValue().toStringType(); + result.merge(name, value, StringUtils::join); + } + return result; + } + + public void duplicate(MimeHeaders source) throws IOException { for (int i = 0; i < source.size(); i++) { MimeHeaderField mhf = createHeader(); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 975279bfd2..223d89c3c9 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -163,6 +163,14 @@ <bug>68934</bug>: Add debug logging in the latch object when exceeding <code>maxConnections</code>. (remm) </fix> + <fix> + Refactor trailer field handling to use a <code>MimeHeaders</code> + instance to store trailer fields. (markt) + </fix> + <fix> + Ensure that multiple instances of the same trailer field are handled + correctly. (markt) + </fix> </changelog> </subsection> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org