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 2023/01/05 14:29:24 UTC
[tomcat] branch 9.0.x updated (d4f17e770a -> 98b70be69a)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from d4f17e770a Fix BZ 66370 Change default for GET_CLASSLOADER_USE_PRIVILEGED
new 10a1a6d46d Refactor MessageBytes make conversions consistent with most recent set
new 98b70be69a Fix BZ 66196 - Log and drop invalid HTTP/1.1 response headers
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/catalina/core/ApplicationContext.java | 1 +
java/org/apache/catalina/mapper/Mapper.java | 1 +
.../catalina/valves/rewrite/RewriteValve.java | 18 +-
java/org/apache/coyote/http11/Http11Processor.java | 15 +-
.../apache/coyote/http11/LocalStrings.properties | 1 +
.../apache/tomcat/util/buf/LocalStrings.properties | 2 +
java/org/apache/tomcat/util/buf/MessageBytes.java | 147 ++++++++++-----
.../coyote/http11/TestHttp11OutputBuffer.java | 81 ++++++++
.../apache/tomcat/util/buf/TestMessageBytes.java | 67 +++++++
.../util/buf/TestMessageBytesConversion.java | 207 +++++++++++++++++++++
webapps/docs/changelog.xml | 5 +
11 files changed, 482 insertions(+), 63 deletions(-)
create mode 100644 test/org/apache/tomcat/util/buf/TestMessageBytesConversion.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/02: Fix BZ 66196 - Log and drop invalid HTTP/1.1 response headers
Posted by ma...@apache.org.
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
commit 98b70be69aca89e2397da3fc26a75adc88514a17
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Sep 2 11:24:06 2022 +0100
Fix BZ 66196 - Log and drop invalid HTTP/1.1 response headers
https://bz.apache.org/bugzilla/show_bug.cgi?id=66196
This also optimizes the common case of String -> Byte conversion in
MessageByte when ISO-8859-1 is used.
---
java/org/apache/coyote/http11/Http11Processor.java | 15 +++-
.../apache/coyote/http11/LocalStrings.properties | 1 +
.../apache/tomcat/util/buf/LocalStrings.properties | 2 +
java/org/apache/tomcat/util/buf/MessageBytes.java | 80 ++++++++++++++++-----
.../coyote/http11/TestHttp11OutputBuffer.java | 81 ++++++++++++++++++++++
.../apache/tomcat/util/buf/TestMessageBytes.java | 67 ++++++++++++++++++
.../util/buf/TestMessageBytesConversion.java | 2 +-
webapps/docs/changelog.xml | 5 ++
8 files changed, 235 insertions(+), 18 deletions(-)
diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java
index 3ed86daeb2..4e288fff79 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -1061,7 +1061,20 @@ public class Http11Processor extends AbstractProcessor {
int size = headers.size();
for (int i = 0; i < size; i++) {
- outputBuffer.sendHeader(headers.getName(i), headers.getValue(i));
+ try {
+ outputBuffer.sendHeader(headers.getName(i), headers.getValue(i));
+ } catch (IllegalArgumentException iae) {
+ // Log the problematic header
+ log.warn(sm.getString("http11processor.response.invalidHeader",
+ headers.getName(i), headers.getValue(i)), iae);
+ // Remove the problematic header
+ headers.removeHeader(i);
+ size--;
+ // Header buffer is corrupted. Reset it and start again.
+ outputBuffer.resetHeaderBuffer();
+ i = 0;
+ outputBuffer.sendStatus();
+ }
}
outputBuffer.endHeaders();
} catch (Throwable t) {
diff --git a/java/org/apache/coyote/http11/LocalStrings.properties b/java/org/apache/coyote/http11/LocalStrings.properties
index d880c717af..3e49897b51 100644
--- a/java/org/apache/coyote/http11/LocalStrings.properties
+++ b/java/org/apache/coyote/http11/LocalStrings.properties
@@ -35,6 +35,7 @@ http11processor.request.nonNumericContentLength=The request contained a content-
http11processor.request.prepare=Error preparing request
http11processor.request.process=Error processing request
http11processor.response.finish=Error finishing response
+http11processor.response.invalidHeader=The HTTP response header [{0}] with value [{1}] has been removed from the response because it is invalid
http11processor.sendfile.error=Error sending data using sendfile. May be caused by invalid request attributes for start/end points
http11processor.socket.info=Exception getting socket information
diff --git a/java/org/apache/tomcat/util/buf/LocalStrings.properties b/java/org/apache/tomcat/util/buf/LocalStrings.properties
index cd883e6f57..b4d5a4eccb 100644
--- a/java/org/apache/tomcat/util/buf/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/buf/LocalStrings.properties
@@ -27,6 +27,8 @@ encodedSolidusHandling.invalid=The value [{0}] is not recognised
hexUtils.fromHex.nonHex=The input must consist only of hex digits
hexUtils.fromHex.oddDigits=The input must consist of an even number of hex digits
+messageBytes.illegalCharacter=The Unicode character [{0}] at code point [{1}] cannot be encoded as it is outside the permitted range of 0 to 255
+
uDecoder.eof=End of file (EOF)
uDecoder.noSlash=The encoded slash character is not allowed
uDecoder.urlDecode.conversionError=Failed to decode [{0}] using character set [{1}]
diff --git a/java/org/apache/tomcat/util/buf/MessageBytes.java b/java/org/apache/tomcat/util/buf/MessageBytes.java
index d1d675fd02..50ae3596dc 100644
--- a/java/org/apache/tomcat/util/buf/MessageBytes.java
+++ b/java/org/apache/tomcat/util/buf/MessageBytes.java
@@ -19,9 +19,12 @@ package org.apache.tomcat.util.buf;
import java.io.IOException;
import java.io.Serializable;
import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
import java.nio.charset.Charset;
import java.util.Locale;
+import org.apache.tomcat.util.res.StringManager;
+
/**
* This class is used to represent a subarray of bytes in an HTTP message.
* It represents all request/response elements. The byte/char conversions are
@@ -35,8 +38,11 @@ import java.util.Locale;
* @author Costin Manolache
*/
public final class MessageBytes implements Cloneable, Serializable {
+
private static final long serialVersionUID = 1L;
+ private static final StringManager sm = StringManager.getManager(MessageBytes.class);
+
// primary type ( whatever is set as original value )
private int type = T_NULL;
@@ -230,29 +236,71 @@ public final class MessageBytes implements Cloneable, Serializable {
* Convert to bytes and fill the ByteChunk with the converted value.
*/
public void toBytes() {
- switch (type) {
- case T_NULL:
- byteC.recycle();
- //$FALL-THROUGH$
- case T_BYTES:
- // No conversion required
- return;
- case T_CHARS:
- toString();
- //$FALL-THROUGH$
- case T_STR: {
- type = T_BYTES;
- Charset charset = byteC.getCharset();
- ByteBuffer result = charset.encode(strValue);
- byteC.setBytes(result.array(), result.arrayOffset(), result.limit());
+ if (type == T_NULL) {
+ byteC.recycle();
+ return;
+ }
+
+ if (type == T_BYTES) {
+ // No conversion required
+ return;
+ }
+
+ if (getCharset() == ByteChunk.DEFAULT_CHARSET) {
+ if (type == T_CHARS) {
+ toBytesSimple(charC.getChars(), charC.getStart(), charC.getLength());
+ } else {
+ // Must be T_STR
+ char[] chars = strValue.toCharArray();
+ toBytesSimple(chars, 0, chars.length);
+ }
+ return;
+ }
+
+ ByteBuffer bb;
+ if (type == T_CHARS) {
+ bb = getCharset().encode(CharBuffer.wrap(charC));
+ } else {
+ // Must be T_STR
+ bb = getCharset().encode(strValue);
+ }
+
+ byteC.setBytes(bb.array(), bb.arrayOffset(), bb.limit());
+ type = T_BYTES;
+ }
+
+
+ /**
+ * Simple conversion of chars to bytes.
+ *
+ * @throws IllegalArgumentException if any of the characters to convert are
+ * above code point 0xFF.
+ */
+ private void toBytesSimple(char[] chars, int start, int len) {
+ byteC.recycle();
+ byteC.allocate(len, byteC.getLimit());
+ byte[] bytes = byteC.getBuffer();
+
+ for (int i = 0; i < len; i++) {
+ if (chars[i + start] > 255) {
+ throw new IllegalArgumentException(sm.getString("messageBytes.illegalCharacter",
+ Character.toString(chars[i + start]), Integer.valueOf(chars[i + start])));
+ } else {
+ bytes[i] = (byte) chars[i + start];
}
}
+
+ byteC.setEnd(len);
+ type = T_BYTES;
}
/**
* Convert to char[] and fill the CharChunk.
- * XXX Not optimized - it converts to String first.
+ *
+ * Note: The conversion from bytes is not optimised - it converts to String
+ * first. However, Tomcat doesn't call this method to convert from
+ * bytes so there is no benefit from optimising that path.
*/
public void toChars() {
switch (type) {
diff --git a/test/org/apache/coyote/http11/TestHttp11OutputBuffer.java b/test/org/apache/coyote/http11/TestHttp11OutputBuffer.java
index affb5d37a9..b84fe8ce1c 100644
--- a/test/org/apache/coyote/http11/TestHttp11OutputBuffer.java
+++ b/test/org/apache/coyote/http11/TestHttp11OutputBuffer.java
@@ -16,6 +16,16 @@
*/
package org.apache.coyote.http11;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
import org.junit.Assert;
import org.junit.Test;
@@ -23,6 +33,7 @@ import org.apache.catalina.Context;
import org.apache.catalina.startup.ExpectationClient;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
public class TestHttp11OutputBuffer extends TomcatBaseTest {
@@ -55,4 +66,74 @@ public class TestHttp11OutputBuffer extends TomcatBaseTest {
Assert.assertTrue(client.isResponse200());
Assert.assertTrue(client.isResponseBodyOK());
}
+
+
+ @Test
+ public void testHTTPHeaderBelow128() throws Exception {
+ doTestHTTPHeaderValue("This should be OK", true);
+ }
+
+
+ @Test
+ public void testHTTPHeader128To255() throws Exception {
+ doTestHTTPHeaderValue("\u00A0 should be OK", true);
+ }
+
+
+ @Test
+ public void testHTTPHeaderAbove255() throws Exception {
+ doTestHTTPHeaderValue("\u0100 should fail", false);
+ }
+
+
+ private void doTestHTTPHeaderValue(String customHeaderValue, boolean valid) throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ Context ctx = tomcat.addContext("", null);
+
+ Tomcat.addServlet(ctx, "header", new HeaderServlet(customHeaderValue));
+ ctx.addServletMappingDecoded("/header", "header");
+
+ tomcat.start();
+
+ Map<String,List<String>> resHeaders = new HashMap<>();
+ int rc = getUrl("http://localhost:" + getPort() + "/header", new ByteChunk(), resHeaders);
+
+ if (valid) {
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+ List<String> values = resHeaders.get(HeaderServlet.CUSTOM_HEADER_NAME);
+ Assert.assertNotNull(values);
+ Assert.assertEquals(1, values.size());
+ Assert.assertEquals(customHeaderValue, values.get(0));
+ } else {
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+ List<String> values = resHeaders.get(HeaderServlet.CUSTOM_HEADER_NAME);
+ Assert.assertNull(values);
+ }
+ }
+
+
+ private static class HeaderServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ private static final String CUSTOM_HEADER_NAME = "X-Test";
+
+ private final String customHeaderValue;
+
+ public HeaderServlet(String customHeaderValue) {
+ this.customHeaderValue = customHeaderValue;
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+
+ resp.setHeader(CUSTOM_HEADER_NAME, customHeaderValue);
+
+ resp.flushBuffer();
+ }
+ }
}
diff --git a/test/org/apache/tomcat/util/buf/TestMessageBytes.java b/test/org/apache/tomcat/util/buf/TestMessageBytes.java
index f9af7fd19d..f2016f478d 100644
--- a/test/org/apache/tomcat/util/buf/TestMessageBytes.java
+++ b/test/org/apache/tomcat/util/buf/TestMessageBytes.java
@@ -16,6 +16,10 @@
*/
package org.apache.tomcat.util.buf;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+
+import org.junit.Assert;
import org.junit.Test;
public class TestMessageBytes {
@@ -66,4 +70,67 @@ public class TestMessageBytes {
mb.recycle();
mb.toChars();
}
+
+
+ /*
+ * Checks the the optimized code is at least twice as fast as the
+ * non-optimized code.
+ */
+ @Test
+ public void testConversionPerformance() {
+ long optimized = -1;
+ long nonOptimized = -1;
+
+ /*
+ * One loop is likely to be enough as the optimised code is
+ * significantly (3x to 4x on markt's desktop) faster than the
+ * non-optimised code. Loop three times allows once to warn up the JVM
+ * once to run the test and once more in case of unexpected CI /GC
+ * slowness. The test will exit early if possible.
+ */
+ for (int i = 0; i < 3; i++) {
+ optimized = doTestConversionPerformance(StandardCharsets.ISO_8859_1);
+ // US_ASCII chosen as the conversion is the same and it is another
+ // Charset available on all platforms.
+ nonOptimized = doTestConversionPerformance(StandardCharsets.US_ASCII);
+
+ System.out.println(optimized + " " + nonOptimized);
+ if (optimized * 2 < nonOptimized) {
+ break;
+ }
+ }
+
+ Assert.assertTrue("Non-optimised code was faster (" + nonOptimized + "ns) compared to optimized (" + optimized + "ns)", optimized < nonOptimized);
+ }
+
+
+ private long doTestConversionPerformance(Charset charset) {
+ MessageBytes mb = MessageBytes.newInstance();
+
+ int loops = 1000000;
+
+ long start = System.nanoTime();
+ for (int i = 0; i < loops; i++) {
+ mb.recycle();
+ mb.setCharset(charset);
+ mb.setString("0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF" +
+ "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF");
+ mb.toBytes();
+ }
+ return System.nanoTime() - start;
+ }
}
diff --git a/test/org/apache/tomcat/util/buf/TestMessageBytesConversion.java b/test/org/apache/tomcat/util/buf/TestMessageBytesConversion.java
index 2b0bddd5a5..8534738195 100644
--- a/test/org/apache/tomcat/util/buf/TestMessageBytesConversion.java
+++ b/test/org/apache/tomcat/util/buf/TestMessageBytesConversion.java
@@ -134,7 +134,7 @@ public class TestMessageBytesConversion {
public static enum MessageBytesType {
BYTE((x) -> x.setBytes(PREVIOUS_BYTES, 0, PREVIOUS_BYTES.length),
(x) -> x.setBytes(EXPECTED_BYTES, 0, EXPECTED_BYTES.length),
- (x) -> {x.toBytes(); Assert.assertArrayEquals(EXPECTED_BYTES, x.getByteChunk().getBytes());},
+ (x) -> {x.toBytes(); Assert.assertTrue(x.getByteChunk().equals(EXPECTED_BYTES, 0, EXPECTED_BYTES.length) );},
(x) -> {x.toBytes(); Assert.assertTrue(x.getByteChunk().isNull());}),
CHAR((x) -> x.setChars(PREVIOUS_CHARS, 0, PREVIOUS_CHARS.length),
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index dad9db1913..2fa8432b47 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -131,6 +131,11 @@
code <code>NO_ERROR</code> so that client does not discard the response.
Based on a suggestion by Lorenzo Dalla Vecchia. (markt)
</fix>
+ <fix>
+ <bug>66196</bug>: Align HTTP/1.1 with HTTP/2 and throw an exception when
+ attempting to commit a response with an header value that includes one
+ or more characters with a code point above 255. (markt)
+ </fix>
<fix>
<bug>66385</bug>: Correct a bug in HTTP/2 where a non-blocking read for
a new frame with the NIO2 connector was incorrectly made using the read
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/02: Refactor MessageBytes make conversions consistent with most recent set
Posted by ma...@apache.org.
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
commit 10a1a6d46d952bab4dfde44c3c0de12b0330da79
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Sep 1 14:05:20 2022 +0100
Refactor MessageBytes make conversions consistent with most recent set
This is preparation for fixing BZ 66196
https://bz.apache.org/bugzilla/show_bug.cgi?id=66196
---
.../apache/catalina/core/ApplicationContext.java | 1 +
java/org/apache/catalina/mapper/Mapper.java | 1 +
.../catalina/valves/rewrite/RewriteValve.java | 18 +-
java/org/apache/tomcat/util/buf/MessageBytes.java | 105 ++++++-----
.../util/buf/TestMessageBytesConversion.java | 207 +++++++++++++++++++++
5 files changed, 267 insertions(+), 65 deletions(-)
diff --git a/java/org/apache/catalina/core/ApplicationContext.java b/java/org/apache/catalina/core/ApplicationContext.java
index 1380913f83..2672861433 100644
--- a/java/org/apache/catalina/core/ApplicationContext.java
+++ b/java/org/apache/catalina/core/ApplicationContext.java
@@ -465,6 +465,7 @@ public class ApplicationContext implements ServletContext {
try {
// Map the URI
+ uriMB.setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
CharChunk uriCC = uriMB.getCharChunk();
try {
uriCC.append(context.getPath());
diff --git a/java/org/apache/catalina/mapper/Mapper.java b/java/org/apache/catalina/mapper/Mapper.java
index 73ebc45163..efdd292ea3 100644
--- a/java/org/apache/catalina/mapper/Mapper.java
+++ b/java/org/apache/catalina/mapper/Mapper.java
@@ -695,6 +695,7 @@ public final class Mapper {
if (defaultHostName == null) {
return;
}
+ host.setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
host.getCharChunk().append(defaultHostName);
}
host.toChars();
diff --git a/java/org/apache/catalina/valves/rewrite/RewriteValve.java b/java/org/apache/catalina/valves/rewrite/RewriteValve.java
index 29929843af..f555e5391d 100644
--- a/java/org/apache/catalina/valves/rewrite/RewriteValve.java
+++ b/java/org/apache/catalina/valves/rewrite/RewriteValve.java
@@ -492,49 +492,39 @@ public class RewriteValve extends ValveBase {
contextPath = request.getContextPath();
}
// Populated the encoded (i.e. undecoded) requestURI
- request.getCoyoteRequest().requestURI().setString(null);
+ request.getCoyoteRequest().requestURI().setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
CharChunk chunk = request.getCoyoteRequest().requestURI().getCharChunk();
- chunk.recycle();
if (context) {
// This is neither decoded nor normalized
chunk.append(contextPath);
}
chunk.append(URLEncoder.DEFAULT.encode(urlStringDecoded, uriCharset));
- request.getCoyoteRequest().requestURI().toChars();
// Decoded and normalized URI
// Rewriting may have denormalized the URL
urlStringDecoded = RequestUtil.normalize(urlStringDecoded);
- request.getCoyoteRequest().decodedURI().setString(null);
+ request.getCoyoteRequest().decodedURI().setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
chunk = request.getCoyoteRequest().decodedURI().getCharChunk();
- chunk.recycle();
if (context) {
// This is decoded and normalized
chunk.append(request.getServletContext().getContextPath());
}
chunk.append(urlStringDecoded);
- request.getCoyoteRequest().decodedURI().toChars();
// Set the new Query if there is one
if (queryStringDecoded != null) {
- request.getCoyoteRequest().queryString().setString(null);
+ request.getCoyoteRequest().queryString().setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
chunk = request.getCoyoteRequest().queryString().getCharChunk();
- chunk.recycle();
chunk.append(URLEncoder.QUERY.encode(queryStringDecoded, uriCharset));
if (qsa && originalQueryStringEncoded != null &&
originalQueryStringEncoded.length() > 0) {
chunk.append('&');
chunk.append(originalQueryStringEncoded);
}
- if (!chunk.isNull()) {
- request.getCoyoteRequest().queryString().toChars();
- }
}
// Set the new host if it changed
if (!host.equals(request.getServerName())) {
- request.getCoyoteRequest().serverName().setString(null);
+ request.getCoyoteRequest().serverName().setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
chunk = request.getCoyoteRequest().serverName().getCharChunk();
- chunk.recycle();
chunk.append(host.toString());
- request.getCoyoteRequest().serverName().toChars();
}
request.getMappingData().recycle();
// Reinvoke the whole request recursively
diff --git a/java/org/apache/tomcat/util/buf/MessageBytes.java b/java/org/apache/tomcat/util/buf/MessageBytes.java
index 64e87d0548..d1d675fd02 100644
--- a/java/org/apache/tomcat/util/buf/MessageBytes.java
+++ b/java/org/apache/tomcat/util/buf/MessageBytes.java
@@ -51,6 +51,8 @@ public final class MessageBytes implements Cloneable, Serializable {
was a char[] */
public static final int T_CHARS = 3;
+ public static final char[] EMPTY_CHAR_ARRAY = new char[0];
+
private int hashCode=0;
// did we compute the hashcode ?
private boolean hasHashCode=false;
@@ -61,9 +63,6 @@ public final class MessageBytes implements Cloneable, Serializable {
// String
private String strValue;
- // true if a String value was computed. Probably not needed,
- // strValue!=null is the same
- private boolean hasStrValue=false;
/**
* Creates a new, uninitialized MessageBytes object.
@@ -87,7 +86,7 @@ public final class MessageBytes implements Cloneable, Serializable {
}
public boolean isNull() {
- return byteC.isNull() && charC.isNull() && !hasStrValue;
+ return type == T_NULL;
}
/**
@@ -100,7 +99,6 @@ public final class MessageBytes implements Cloneable, Serializable {
strValue=null;
- hasStrValue=false;
hasHashCode=false;
hasLongValue=false;
}
@@ -116,7 +114,6 @@ public final class MessageBytes implements Cloneable, Serializable {
public void setBytes(byte[] b, int off, int len) {
byteC.setBytes( b, off, len );
type=T_BYTES;
- hasStrValue=false;
hasHashCode=false;
hasLongValue=false;
}
@@ -131,7 +128,6 @@ public final class MessageBytes implements Cloneable, Serializable {
public void setChars( char[] c, int off, int len ) {
charC.setChars( c, off, len );
type=T_CHARS;
- hasStrValue=false;
hasHashCode=false;
hasLongValue=false;
}
@@ -141,15 +137,13 @@ public final class MessageBytes implements Cloneable, Serializable {
* @param s The string
*/
public void setString( String s ) {
- strValue=s;
- hasHashCode=false;
- hasLongValue=false;
+ strValue = s;
+ hasHashCode = false;
+ hasLongValue = false;
if (s == null) {
- hasStrValue=false;
- type=T_NULL;
+ type = T_NULL;
} else {
- hasStrValue=true;
- type=T_STR;
+ type = T_STR;
}
}
@@ -161,21 +155,22 @@ public final class MessageBytes implements Cloneable, Serializable {
*/
@Override
public String toString() {
- if (hasStrValue) {
- return strValue;
- }
-
switch (type) {
- case T_CHARS:
- strValue = charC.toString();
- hasStrValue = true;
- return strValue;
- case T_BYTES:
- strValue = byteC.toString();
- hasStrValue = true;
- return strValue;
+ case T_NULL:
+ case T_STR:
+ // No conversion required
+ break;
+ case T_BYTES:
+ type = T_STR;
+ strValue = byteC.toString();
+ break;
+ case T_CHARS:
+ type = T_STR;
+ strValue = charC.toString();
+ break;
}
- return null;
+
+ return strValue;
}
//----------------------------------------
@@ -232,21 +227,26 @@ public final class MessageBytes implements Cloneable, Serializable {
/**
- * Do a char->byte conversion.
+ * Convert to bytes and fill the ByteChunk with the converted value.
*/
public void toBytes() {
- if (isNull()) {
- return;
- }
- if (!byteC.isNull()) {
- type = T_BYTES;
- return;
+ switch (type) {
+ case T_NULL:
+ byteC.recycle();
+ //$FALL-THROUGH$
+ case T_BYTES:
+ // No conversion required
+ return;
+ case T_CHARS:
+ toString();
+ //$FALL-THROUGH$
+ case T_STR: {
+ type = T_BYTES;
+ Charset charset = byteC.getCharset();
+ ByteBuffer result = charset.encode(strValue);
+ byteC.setBytes(result.array(), result.arrayOffset(), result.limit());
+ }
}
- toString();
- type = T_BYTES;
- Charset charset = byteC.getCharset();
- ByteBuffer result = charset.encode(strValue);
- byteC.setBytes(result.array(), result.arrayOffset(), result.limit());
}
@@ -255,18 +255,22 @@ public final class MessageBytes implements Cloneable, Serializable {
* XXX Not optimized - it converts to String first.
*/
public void toChars() {
- if (isNull()) {
- return;
- }
- if (!charC.isNull()) {
- type = T_CHARS;
- return;
+ switch (type) {
+ case T_NULL:
+ charC.recycle();
+ //$FALL-THROUGH$
+ case T_CHARS:
+ // No conversion required
+ return;
+ case T_BYTES:
+ toString();
+ //$FALL-THROUGH$
+ case T_STR: {
+ type = T_CHARS;
+ char cc[] = strValue.toCharArray();
+ charC.setChars(cc, 0, cc.length);
+ }
}
- // inefficient
- toString();
- type = T_CHARS;
- char cc[] = strValue.toCharArray();
- charC.setChars(cc, 0, cc.length);
}
@@ -535,7 +539,6 @@ public final class MessageBytes implements Cloneable, Serializable {
end--;
}
longValue=l;
- hasStrValue=false;
hasHashCode=false;
hasLongValue=true;
type=T_BYTES;
diff --git a/test/org/apache/tomcat/util/buf/TestMessageBytesConversion.java b/test/org/apache/tomcat/util/buf/TestMessageBytesConversion.java
new file mode 100644
index 0000000000..2b0bddd5a5
--- /dev/null
+++ b/test/org/apache/tomcat/util/buf/TestMessageBytesConversion.java
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.buf;
+
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.function.Consumer;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * Checks that all MessageBytes getters are consistent with most recently used
+ * setter.
+ */
+@RunWith(Parameterized.class)
+public class TestMessageBytesConversion {
+
+ private static final String PREVIOUS_STRING = "previous-string";
+ private static final byte[] PREVIOUS_BYTES = "previous-bytes".getBytes(StandardCharsets.ISO_8859_1);
+ private static final char[] PREVIOUS_CHARS = "previous-chars".toCharArray();
+
+ private static final String EXPECTED_STRING = "expected";
+ private static final byte[] EXPECTED_BYTES = "expected".getBytes(StandardCharsets.ISO_8859_1);
+ private static final char[] EXPECTED_CHARS = "expected".toCharArray();
+
+ @Parameters(name = "{index}: previous({0}, {1}, {2}, {3}), set {4}, check({5}, {6}, {7}")
+ public static Collection<Object[]> parameters() {
+ List<MessageBytesType> previousTypes = new ArrayList<>();
+ previousTypes.add(MessageBytesType.BYTE);
+ previousTypes.add(MessageBytesType.CHAR);
+ previousTypes.add(MessageBytesType.STRING);
+ previousTypes.add(MessageBytesType.NULL);
+
+ List<MessageBytesType> setTypes = new ArrayList<>();
+ setTypes.add(MessageBytesType.BYTE);
+ setTypes.add(MessageBytesType.CHAR);
+ setTypes.add(MessageBytesType.STRING);
+
+ List<Object[]> parameterSets = new ArrayList<>();
+
+ List<List<MessageBytesType>> previousPermutations = permutations(previousTypes);
+ List<List<MessageBytesType>> checkPermutations = permutations(setTypes);
+
+ for (List<MessageBytesType> setPermutation : previousPermutations) {
+ for (MessageBytesType setType : setTypes) {
+ for (List<MessageBytesType> checkPermutation : checkPermutations) {
+ parameterSets.add(new Object[] {
+ setPermutation.get(0), setPermutation.get(1), setPermutation.get(2), setPermutation.get(3),
+ setType,
+ checkPermutation.get(0), checkPermutation.get(1), checkPermutation.get(2)});
+ }
+ }
+ }
+
+
+ return parameterSets;
+ }
+
+ @Parameter(0)
+ public MessageBytesType setFirst;
+ @Parameter(1)
+ public MessageBytesType setSecond;
+ @Parameter(2)
+ public MessageBytesType setThird;
+ @Parameter(3)
+ public MessageBytesType setFourth;
+
+ @Parameter(4)
+ public MessageBytesType expected;
+
+ @Parameter(5)
+ public MessageBytesType checkFirst;
+ @Parameter(6)
+ public MessageBytesType checkSecond;
+ @Parameter(7)
+ public MessageBytesType checkThird;
+
+
+ @Test
+ public void testConversion() {
+ MessageBytes mb = MessageBytes.newInstance();
+
+ setFirst.setPrevious(mb);
+ setSecond.setPrevious(mb);
+ setThird.setPrevious(mb);
+ setFourth.setPrevious(mb);
+
+ expected.setExpected(mb);
+
+ checkFirst.checkExpected(mb);
+ checkSecond.checkExpected(mb);
+ checkThird.checkExpected(mb);
+ }
+
+
+ @Test
+ public void testConversionNull() {
+ MessageBytes mb = MessageBytes.newInstance();
+
+ setFirst.setPrevious(mb);
+ setSecond.setPrevious(mb);
+ setThird.setPrevious(mb);
+ setFourth.setPrevious(mb);
+
+ mb.setString(null);
+
+ checkFirst.checkNull(mb);
+ checkSecond.checkNull(mb);
+ checkThird.checkNull(mb);
+ }
+
+
+ public static enum MessageBytesType {
+ BYTE((x) -> x.setBytes(PREVIOUS_BYTES, 0, PREVIOUS_BYTES.length),
+ (x) -> x.setBytes(EXPECTED_BYTES, 0, EXPECTED_BYTES.length),
+ (x) -> {x.toBytes(); Assert.assertArrayEquals(EXPECTED_BYTES, x.getByteChunk().getBytes());},
+ (x) -> {x.toBytes(); Assert.assertTrue(x.getByteChunk().isNull());}),
+
+ CHAR((x) -> x.setChars(PREVIOUS_CHARS, 0, PREVIOUS_CHARS.length),
+ (x) -> x.setChars(EXPECTED_CHARS, 0, EXPECTED_CHARS.length),
+ (x) -> {x.toChars(); Assert.assertArrayEquals(EXPECTED_CHARS, x.getCharChunk().getChars());},
+ (x) -> {x.toChars(); Assert.assertTrue(x.getCharChunk().isNull());}),
+
+ STRING((x) -> x.setString(PREVIOUS_STRING),
+ (x) -> x.setString(EXPECTED_STRING),
+ (x) -> Assert.assertEquals(EXPECTED_STRING, x.toString()),
+ (x) -> Assert.assertNull(x.toString())),
+
+ NULL((x) -> x.setString(null),
+ (x) -> x.setString(null),
+ (x) -> Assert.assertTrue(x.isNull()),
+ (x) -> Assert.assertTrue(x.isNull()));
+
+ private final Consumer<MessageBytes> setPrevious;
+ private final Consumer<MessageBytes> setExpected;
+ private final Consumer<MessageBytes> checkExpected;
+ private final Consumer<MessageBytes> checkNull;
+
+ private MessageBytesType(Consumer<MessageBytes> setPrevious, Consumer<MessageBytes> setExpected,
+ Consumer<MessageBytes> checkExpected, Consumer<MessageBytes> checkNull) {
+ this.setPrevious = setPrevious;
+ this.setExpected = setExpected;
+ this.checkExpected = checkExpected;
+ this.checkNull = checkNull;
+ }
+
+ public void setPrevious(MessageBytes mb) {
+ setPrevious.accept(mb);
+ }
+
+ public void setExpected(MessageBytes mb) {
+ setExpected.accept(mb);
+ }
+
+ public void checkExpected(MessageBytes mb) {
+ checkExpected.accept(mb);
+ }
+
+ public void checkNull(MessageBytes mb) {
+ checkNull.accept(mb);
+ }
+ }
+
+
+ private static <T> List<List<T>> permutations(List<T> items) {
+ List<List<T>> results = new ArrayList<>();
+
+ if (items.size() == 1) {
+ results.add(items);
+ } else {
+ List<T> others = new ArrayList<>(items);
+ T first = others.remove(0);
+ List<List<T>> subPermutations = permutations(others);
+
+ for (List<T> subPermutation : subPermutations) {
+ for (int i = 0; i <= subPermutation.size(); i++) {
+ List<T> result = new ArrayList<>(subPermutation);
+ result.add(i, first);
+ results.add(result);
+ }
+ }
+ }
+
+ return results;
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org