Author: markt
Date: Mon Mar 4 11:10:19 2013
New Revision: 1452253
URL: http://svn.apache.org/r1452253
Log:
Move the single byte tests to the new framework
Expand the single byte test cases
Add leading and trailing valid bytes to invalid sequences to check the valid
bytes are not replaced
Improve handling for when JVM decoder spots error later than the earliest point
Add handling for when the JVM decoder swallows the trailing valid byte
Add malformed replacement checking to the old framework
Update Tomcat's UTF-8 decoder to use rules from unicode 6.2, chapter 3, table
3-7 for valid byte sequences
Update test cases for these stricter tests
Modified:
tomcat/trunk/java/org/apache/tomcat/util/buf/Utf8Decoder.java
tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8.java
tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8Extended.java
Modified: tomcat/trunk/java/org/apache/tomcat/util/buf/Utf8Decoder.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/Utf8Decoder.java?rev=1452253&r1=1452252&r2=1452253&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/buf/Utf8Decoder.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/buf/Utf8Decoder.java Mon Mar 4
11:10:19 2013
@@ -21,7 +21,6 @@ import java.nio.CharBuffer;
import java.nio.charset.CharsetDecoder;
import java.nio.charset.CoderResult;
-
/**
* Decodes bytes to UTF-8. Extracted from Apache Harmony and modified to reject
* code points from U+D800 to U+DFFF as per RFC3629. The standard Java decoder
@@ -45,17 +44,17 @@ public class Utf8Decoder extends Charset
// 1111ouuu 1ouuzzzz 1oyyyyyy 1oxxxxxx 000uuuuu zzzzyyyy yyxxxxxx
private static final int remainingBytes[] = {
// 1owwwwww
- -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
- -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
- -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
- -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
// 11oyyyyy
- -1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
- 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+ -1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
// 111ozzzz
2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
// 1111ouuu
- 3, 3, 3, 3, 3, 3, 3, 3,
+ 3, 3, 3, 3, 3, -1, -1, -1,
// > 11110111
-1, -1, -1, -1, -1, -1, -1, -1};
private static final int remainingNumbers[] = {0, // 0 1 2 3
@@ -107,7 +106,7 @@ public class Utf8Decoder extends Charset
for (int i = 0; i < tail; i++) {
nextByte = in.get() & 0xFF;
if ((nextByte & 0xC0) != 0x80) {
- return CoderResult.malformedForLength(1 + i);
+ return CoderResult.malformedForLength(1);
}
jchar = (jchar << 6) + nextByte;
}
@@ -161,40 +160,91 @@ public class Utf8Decoder extends Charset
int jchar = bArr[inIndex];
if (jchar < 0) {
jchar = jchar & 0x7F;
+ // If first byte is invalid, tail will be set to -1
int tail = remainingBytes[jchar];
if (tail == -1) {
in.position(inIndex - in.arrayOffset());
out.position(outIndex - out.arrayOffset());
return CoderResult.malformedForLength(1);
}
- if (inIndexLimit - inIndex < 1 + tail) {
- // Apache Tomcat added tests - detect invalid sequences as
- // early as possible
- if (jchar == 0x74 && inIndexLimit > inIndex + 1) {
- if ((bArr[inIndex + 1] & 0xFF) > 0x8F) {
- // 11110100 1yyyxxxx xxxxxxxx xxxxxxxx
- // Any non-zero y is > max code point
- return CoderResult.unmappableForLength(1);
- }
+ // Additional checks to detect invalid sequences ASAP
+ // Checks derived from Unicode 6.2, Chapter 3, Table 3-7
+ // Check 2nd byte
+ int tailAvailable = inIndexLimit - inIndex - 1;
+ if (tailAvailable > 0) {
+ // First byte C2..DF, second byte 80..BF
+ if (jchar > 0x41 && jchar < 0x60 &&
+ (bArr[inIndex + 1] & 0x80) != 0x80) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(1);
}
- if (jchar == 0x60 && inIndexLimit > inIndex + 1) {
- if ((bArr[inIndex + 1] & 0x60) == 0) {
- // 11100000 100yyyyy 10xxxxxx
- // should have been
- // 11oyyyyy 1oxxxxxx
- // or possibly
- // 00xxxxxx
- return CoderResult.malformedForLength(1);
- }
+ // First byte E0, second byte A0..BF
+ if (jchar == 0x60 && (bArr[inIndex + 1] & 0xE0) != 0xA0) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(1);
}
- if (jchar == 0x70 && inIndexLimit > inIndex + 1) {
- if ((bArr[inIndex + 1] & 0x70) == 0) {
- // 11110000 1000zzzz 1oyyyyyy 1oxxxxxx
- // should have been
- // 111ozzzz 1oyyyyyy 1oxxxxxx
- return CoderResult.malformedForLength(1);
- }
+ // First byte E1..EC, second byte 80..BF
+ if (jchar > 0x60 && jchar < 0x6D &&
+ (bArr[inIndex + 1] & 0x80) != 0x80) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(1);
+ }
+ // First byte ED, second byte 80..9F
+ if (jchar == 0x6D && (bArr[inIndex + 1] & 0xE0) != 0x80) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(1);
+ }
+ // First byte EE..EF, second byte 80..BF
+ if (jchar > 0x6D && jchar < 0x70 &&
+ (bArr[inIndex + 1] & 0x80) != 0x80) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(1);
+ }
+ // First byte F0, second byte 90..BF
+ if (jchar == 0x70 &&
+ ((bArr[inIndex + 1] & 0xFF) < 0x90 ||
+ (bArr[inIndex + 1] & 0xFF) > 0xBF)) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(1);
+ }
+ // First byte F1..F3, second byte 80..BF
+ if (jchar > 0x70 && jchar < 0x74 &&
+ (bArr[inIndex + 1] & 0x80) != 0x80) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(1);
}
+ // First byte F4, second byte 80..8F
+ if (jchar == 0x74 &&
+ (bArr[inIndex + 1] & 0xF0) != 0x80) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(1);
+ }
+ }
+ // Check third byte if present and expected
+ if (tailAvailable > 1 && tail > 1) {
+ if ((bArr[inIndex + 2] & 0x80) != 0x80) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(2);
+ }
+ }
+ // Check fourth byte if present and expected
+ if (tailAvailable > 2 && tail > 2) {
+ if ((bArr[inIndex + 3] & 0x80) != 0x80) {
+ in.position(inIndex - in.arrayOffset());
+ out.position(outIndex - out.arrayOffset());
+ return CoderResult.malformedForLength(3);
+ }
+ }
+ if (tailAvailable < tail) {
break;
}
for (int i = 0; i < tail; i++) {
@@ -202,7 +252,7 @@ public class Utf8Decoder extends Charset
if ((nextByte & 0xC0) != 0x80) {
in.position(inIndex - in.arrayOffset());
out.position(outIndex - out.arrayOffset());
- return CoderResult.malformedForLength(1 + i);
+ return CoderResult.malformedForLength(1);
}
jchar = (jchar << 6) + nextByte;
}
Modified: tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8.java?rev=1452253&r1=1452252&r2=1452253&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8.java Mon Mar 4
11:10:19 2013
@@ -18,13 +18,20 @@ package org.apache.tomcat.util.buf;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
+import java.nio.charset.Charset;
import java.nio.charset.CharsetDecoder;
import java.nio.charset.CoderResult;
import java.nio.charset.CodingErrorAction;
import static org.junit.Assert.assertEquals;
+
+import org.junit.Assert;
import org.junit.Test;
+/**
+ * Tests the behaviour of the custom UTF-8 decoder and compares it to the JVM
+ * implementation.
+ */
public class TestUtf8 {
// Invalid UTF-8
@@ -34,11 +41,6 @@ public class TestUtf8 {
// Various invalid UTF-8 sequences
private static final byte[][] MALFORMED = {
- // One-byte sequences:
- {(byte)0xFF },
- {(byte)0xC0 },
- {(byte)0x80 },
-
// Two-byte sequences:
{(byte)0xC0, (byte)0x80}, // U+0000 zero-padded
{(byte)0xC1, (byte)0xBF}, // U+007F zero-padded
@@ -82,10 +84,39 @@ public class TestUtf8 {
{(byte)0xFC, (byte)0x80, (byte)0x80, (byte)0x8F, (byte)0xBF,
(byte)0xBF }, // U+FFFF zero-padded
};
+ // Expected result after UTF-8 decoding with replacement
+ private static final String[] MALFORMED_REPLACE_UTF8 = {
+ // two byte sequences
+ "\uFFFD\uFFFD", "\uFFFD\uFFFD", "\uFFFD\uFFFD", "\uFFFD\uFFFD",
+ "\uFFFD\uFFFD", "\uFFFD\u0000", "\uFFFD\uFFFD",
+
+ // three byte sequences
+ "\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD",
+
+ // four byte sequences
+ "\uFFFD\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD\uFFFD",
+
+ // five byte sequences
+ "\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD", "\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD",
+
+ // six byte sequences
+ "\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD",
+ "\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD\uFFFD" };
+
+
@Test
public void testJvmDecoder1() {
// This should trigger an error but currently passes. Once the JVM is
- // fixed, s/false/true/ and s/20/13/
+ // fixed, s/false/true/ and s/20/12/
doJvmDecoder(SRC_BYTES_1, false, false, 20);
}
@@ -129,7 +160,7 @@ public class TestUtf8 {
@Test
public void testHarmonyDecoder1() {
- doHarmonyDecoder(SRC_BYTES_1, false, true, 13);
+ doHarmonyDecoder(SRC_BYTES_1, false, true, 12);
}
@@ -173,7 +204,7 @@ public class TestUtf8 {
// Known failures
// JVM UTF-8 decoder spots invalid sequences but not if they occur
// at the end of the input and endOfInput is not true
- if (i == 1 || i == 6 || i == 14 | i == 22) {
+ if (i == 3 || i == 11 | i == 19) {
doJvmDecoder(MALFORMED[i], false, false, -1);
} else {
doJvmDecoder(MALFORMED[i], false, true, -1);
@@ -194,8 +225,83 @@ public class TestUtf8 {
@Test
public void testUtf8MalformedHarmony() {
+ // Harmony UTF-8 decoder fails as soon as an invalid sequence is
+ // detected
for (byte[] input : MALFORMED) {
doHarmonyDecoder(input, false, true, -1);
}
}
+
+
+ @Test
+ public void testUtf8MalformedReplacementHarmony() throws Exception {
+ CharsetDecoder decoder = new Utf8Decoder();
+ decoder.onMalformedInput(CodingErrorAction.REPLACE);
+ decoder.onUnmappableCharacter(CodingErrorAction.REPLACE);
+
+ for (int i = 0; i < MALFORMED.length; i++) {
+ doMalformed(decoder, i, MALFORMED[i], MALFORMED_REPLACE_UTF8[i]);
+ decoder.reset();
+ }
+ }
+
+
+ @Test
+ public void testUtf8MalformedReplacementJvm() throws Exception {
+ CharsetDecoder decoder = Charset.forName("UTF-8").newDecoder();
+ decoder.onMalformedInput(CodingErrorAction.REPLACE);
+ decoder.onUnmappableCharacter(CodingErrorAction.REPLACE);
+
+ for (int i = 0; i < MALFORMED.length; i++) {
+ // Handle JVM inconsistencies
+ String expected;
+ // In all other cases first invalid byte is replaced and processing
+ // continues as if the next byte is the start of a new sequence
+ // This does not happen for these tests
+ if (i == 3 | i == 11 | i == 19 | i == 23 | i == 24 | i == 25 |
+ i == 26 | i == 27 | i == 28 | i == 29 | i == 30) {
+ expected = "\uFFFD";
+ } else {
+ expected = MALFORMED_REPLACE_UTF8[i];
+ }
+ doMalformed(decoder, i, MALFORMED[i], expected);
+ decoder.reset();
+ }
+ }
+
+
+ private void doMalformed(CharsetDecoder decoder, int test, byte[] input,
+ String expected) throws Exception {
+
+ ByteBuffer bb = ByteBuffer.allocate(input.length);
+ CharBuffer cb = CharBuffer.allocate(bb.limit());
+
+ int i = 0;
+ for (; i < input.length; i++) {
+ bb.put(input[i]);
+ bb.flip();
+ CoderResult cr = decoder.decode(bb, cb, false);
+ if (cr.isError()) {
+ throw new Exception();
+ }
+ bb.compact();
+ }
+ bb.flip();
+ CoderResult cr = decoder.decode(bb, cb, true);
+ if (cr.isError()) {
+ throw new Exception();
+ }
+
+ cb.flip();
+
+ StringBuilder ashex = new StringBuilder(input.length * 4);
+ for (int j = 0; j < input.length; j++) {
+ if (i > 0) ashex.append(' ');
+ ashex.append(Integer.toBinaryString(input[j] & 0xff));
+ }
+ String hex = ashex.toString();
+
+ String result = cb.toString();
+ Assert.assertEquals(test + ": " + hex, expected, result);
+ }
}
Modified: tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8Extended.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8Extended.java?rev=1452253&r1=1452252&r2=1452253&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8Extended.java
(original)
+++ tomcat/trunk/test/org/apache/tomcat/util/buf/TestUtf8Extended.java Mon Mar
4 11:10:19 2013
@@ -36,9 +36,18 @@ import org.junit.Test;
*/
public class TestUtf8Extended {
- // Indicates that at invalid sequence is detected at the end of the
sequence
- // rather than as early as possible
- private static final int ERROR_POS_END = 1;
+ // Indicates that at invalid sequence is detected one character later than
+ // the earliest possible moment
+ private static final int ERROR_POS_PLUS1 = 1;
+ // Indicates that at invalid sequence is detected two characters later than
+ // the earliest possible moment
+ private static final int ERROR_POS_PLUS2 = 2;
+ // Indicates that at invalid sequence is detected three characters later
+ // than the earliest possible moment
+ private static final int ERROR_POS_PLUS3 = 4;
+ // Indicates that the trailing valid byte is included in replacement of the
+ // previous error
+ private static final int REPLACE_SWALLOWS_TRAILER = 8;
private List<Utf8TestCase> testCases = new ArrayList<>();
@@ -72,27 +81,52 @@ public class TestUtf8Extended {
// JVM decoder does not report error until all 4 bytes are available
testCases.add(new Utf8TestCase(
"Invalid code point - out of range",
- new int[] {0xF4, 0x90, 0x80, 0x80},
- 1,
- "\uFFFD\uFFFD\uFFFD\uFFFD").addForJvm(ERROR_POS_END));
+ new int[] {0x41, 0xF4, 0x90, 0x80, 0x80, 0x41},
+ 2,
+ "A\uFFFD\uFFFD\uFFFD\uFFFDA").addForJvm(ERROR_POS_PLUS2));
// JVM decoder does not report error until all 2 bytes are available
testCases.add(new Utf8TestCase(
"Valid sequence padded from one byte to two",
- new int[] {0xC0, 0xC1},
- 0,
- "\uFFFD\uFFFD").addForJvm(ERROR_POS_END));
+ new int[] {0x41, 0xC0, 0xC1, 0x41},
+ 1,
+ "A\uFFFD\uFFFDA").addForJvm(ERROR_POS_PLUS1));
// JVM decoder does not report error until all 3 bytes are available
testCases.add(new Utf8TestCase(
"Valid sequence padded from one byte to three",
- new int[] {0xE0, 0x80, 0xC1},
- 1,
- "\uFFFD\uFFFD\uFFFD").addForJvm(ERROR_POS_END));
+ new int[] {0x41, 0xE0, 0x80, 0xC1, 0x41},
+ 2,
+ "A\uFFFD\uFFFD\uFFFDA").addForJvm(ERROR_POS_PLUS1));
// JVM decoder does not report error until all 4 bytes are available
testCases.add(new Utf8TestCase(
"Valid sequence padded from one byte to four",
- new int[] {0xF0, 0x80, 0x80, 0xC1},
+ new int[] {0x41, 0xF0, 0x80, 0x80, 0xC1, 0x41},
+ 2,
+ "A\uFFFD\uFFFD\uFFFD\uFFFDA").addForJvm(ERROR_POS_PLUS2));
+ testCases.add(new Utf8TestCase(
+ "Invalid one byte 1111 1111",
+ new int[] {0x41, 0xFF, 0x41},
+ 1,
+ "A\uFFFDA"));
+ testCases.add(new Utf8TestCase(
+ "Invalid one byte 1111 0000",
+ new int[] {0x41, 0xF0, 0x41},
+ 2,
+ "A\uFFFDA").addForJvm(REPLACE_SWALLOWS_TRAILER));
+ testCases.add(new Utf8TestCase(
+ "Invalid one byte 1110 0000",
+ new int[] {0x41, 0xE0, 0x41},
+ 2,
+ "A\uFFFDA").addForJvm(REPLACE_SWALLOWS_TRAILER));
+ testCases.add(new Utf8TestCase(
+ "Invalid one byte 1100 0000",
+ new int[] {0x41, 0xC0, 0x41},
1,
- "\uFFFD\uFFFD\uFFFD\uFFFD").addForJvm(ERROR_POS_END));
+ "A\uFFFDA").addForJvm(ERROR_POS_PLUS1));
+ testCases.add(new Utf8TestCase(
+ "Invalid one byte 1000 000",
+ new int[] {0x41, 0x80, 0x41},
+ 1,
+ "A\uFFFDA"));
}
@Test
@@ -132,12 +166,15 @@ public class TestUtf8Extended {
bb.flip();
CoderResult cr = decoder.decode(bb, cb, false);
if (cr.isError()) {
- if ((flags & ERROR_POS_END) == 0) {
- Assert.assertEquals(testCase.description,
- testCase.invalidIndex, i);
- } else {
- Assert.assertEquals(testCase.description, len - 1, i);
+ int expected = testCase.invalidIndex;
+ if ((flags & ERROR_POS_PLUS1) != 0) {
+ expected += 1;
+ } else if ((flags & ERROR_POS_PLUS2) != 0) {
+ expected += 2;
+ } else if ((flags & ERROR_POS_PLUS3) != 0) {
+ expected += 3;
}
+ Assert.assertEquals(testCase.description, expected, i);
break;
}
bb.compact();
@@ -168,8 +205,15 @@ public class TestUtf8Extended {
Assert.fail(testCase.description);
}
cb.flip();
- Assert.assertEquals(testCase.description, testCase.outputReplaced,
- cb.toString());
+ if ((flags & REPLACE_SWALLOWS_TRAILER) == 0) {
+ Assert.assertEquals(testCase.description, testCase.outputReplaced,
+ cb.toString());
+ } else {
+ Assert.assertEquals(testCase.description,
+ testCase.outputReplaced.subSequence(0,
+ testCase.outputReplaced.length() - 1),
+ cb.toString());
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
