Re: (tomcat) branch main updated: Refactor storage of trailer fields to use MimeHeaders

2024-04-29 Thread Christopher Schultz

Mark,

On 4/24/24 14:47, ma...@apache.org wrote:

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 f087decbc9 Refactor storage of trailer fields to use MimeHeaders
f087decbc9 is described below

commit f087decbc938eff084b7be92298457736fe783c2
Author: Mark Thomas 
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 390ca9daa1..6bf0f0a940 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -1763,8 +1763,8 @@ public class Request implements HttpServletRequest {
  if (!isTrailerFieldsReady()) {
  throw new 
IllegalStateException(sm.getString("coyoteRequest.trailersNotReady"));
  }
-Map 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 680aec6a7b..bf948b09a6 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -110,7 +110,7 @@ public final class Request {
  private final MessageBytes localAddrMB = MessageBytes.newInstance();
  
  private final MimeHeaders headers = new MimeHeaders();

-private final Map trailerFields = new HashMap<>();
+private final MimeHeaders trailerFields = new MimeHeaders();
  
  /**

   * Path parameters
@@ -293,6 +293,11 @@ public final class Request {
  
  
  public Map getTrailerFields() {

+return trailerFields.toMap();
+}


Should getTrailerFields call getMimeTrailerFields instead of using 
this.trailerFields directly? I'm not sure how much we really care about 
subclasses...


-chris

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



(tomcat) branch main updated: Refactor storage of trailer fields to use MimeHeaders

2024-04-24 Thread markt
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 f087decbc9 Refactor storage of trailer fields to use MimeHeaders
f087decbc9 is described below

commit f087decbc938eff084b7be92298457736fe783c2
Author: Mark Thomas 
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 390ca9daa1..6bf0f0a940 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -1763,8 +1763,8 @@ public class Request implements HttpServletRequest {
 if (!isTrailerFieldsReady()) {
 throw new 
IllegalStateException(sm.getString("coyoteRequest.trailersNotReady"));
 }
-Map 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 680aec6a7b..bf948b09a6 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -110,7 +110,7 @@ public final class Request {
 private final MessageBytes localAddrMB = MessageBytes.newInstance();
 
 private final MimeHeaders headers = new MimeHeaders();
-private final Map trailerFields = new HashMap<>();
+private final MimeHeaders trailerFields = new MimeHeaders();
 
 /**
  * Path parameters
@@ -293,6 +293,11 @@ public final class Request {
 
 
 public Map getTrailerFields() {
+return trailerFields.toMap();
+}
+
+
+public MimeHeaders getMimeTrailerFields() {
 return trailerFields;
 }
 
@@ -782,7 +787,13 @@ public final class Request {
 charsetHolder = CharsetHolder.EMPTY;
 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 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