This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6d6e899482e889a37fa6c5fe1d7190d7b0ebbf38 Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue Sep 17 22:03:31 2019 +0100 Fix HTTP/2 related TCK failures Use URL safe base 64 encoding rather than standard base 64 encoding when generating or parsing the <code>HTTP2-Settings</code> header as part of an HTTP upgrade to <code>h2c</code> as required by RFC 7540. --- .../apache/coyote/http2/Http2UpgradeHandler.java | 2 +- .../apache/tomcat/util/codec/binary/Base64.java | 32 +++++++++++++++++----- test/org/apache/coyote/http2/Http2TestBase.java | 2 +- webapps/docs/changelog.xml | 9 ++++++ 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java index 10a1f65..e0f3dde 100644 --- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java +++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java @@ -214,7 +214,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH // Process the initial settings frame stream = getStream(1, true); String base64Settings = stream.getCoyoteRequest().getHeader(HTTP2_SETTINGS_HEADER); - byte[] settings = Base64.decodeBase64(base64Settings); + byte[] settings = Base64.decodeBase64URLSafe(base64Settings); // Settings are only valid on stream 0 FrameType.SETTINGS.check(0, settings.length); diff --git a/java/org/apache/tomcat/util/codec/binary/Base64.java b/java/org/apache/tomcat/util/codec/binary/Base64.java index 99a501c..da1487f 100644 --- a/java/org/apache/tomcat/util/codec/binary/Base64.java +++ b/java/org/apache/tomcat/util/codec/binary/Base64.java @@ -110,18 +110,30 @@ public class Base64 extends BaseNCodec { * Thanks to "commons" project in ws.apache.org for this code. * https://svn.apache.org/repos/asf/webservices/commons/trunk/modules/util/ */ - private static final byte[] DECODE_TABLE = { + private static final byte[] STANDARD_DECODE_TABLE = { // 0 1 2 3 4 5 6 7 8 9 A B C D E F -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, // 00-0f -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, // 10-1f - -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 62, -1, 62, -1, 63, // 20-2f + - / + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 62, -1, -1, -1, 63, // 20-2f + / 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -1, -1, -1, -1, -1, -1, // 30-3f 0-9 -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, // 40-4f A-O - 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, -1, -1, -1, -1, 63, // 50-5f P-Z _ + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, -1, -1, -1, -1, -1, // 50-5f P-Z -1, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, // 60-6f a-o 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51 // 70-7a p-z }; + private static final byte[] URL_SAFE_DECODE_TABLE = { + // 0 1 2 3 4 5 6 7 8 9 A B C D E F + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, // 00-0f + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, // 10-1f + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 62, -1, -1, // 20-2f - + 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -1, -1, -1, -1, -1, -1, // 30-3f 0-9 + -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, // 40-4f A-O + 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, -1, -1, -1, -1, 63, // 50-5f P-Z _ + -1, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, // 60-6f a-o + 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51 // 70-7a p-z + }; + /** * Base64 uses 6-bit fields. */ @@ -140,7 +152,7 @@ public class Base64 extends BaseNCodec { private final byte[] encodeTable; // Only one decode table currently; keep for consistency with Base32 code - private final byte[] decodeTable = DECODE_TABLE; + private final byte[] decodeTable; /** * Line separator for encoding. Not used when decoding. Only used if lineLength > 0. @@ -273,6 +285,8 @@ public class Base64 extends BaseNCodec { super(BYTES_PER_UNENCODED_BLOCK, BYTES_PER_ENCODED_BLOCK, lineLength, lineSeparator == null ? 0 : lineSeparator.length); + // Needs to be set early to avoid NPE during call to containsAlphabetOrPad() below + this.decodeTable = urlSafe ? URL_SAFE_DECODE_TABLE : STANDARD_DECODE_TABLE; // TODO could be simplified if there is no requirement to reject invalid line sep when length <=0 // @see test case Base64Test.testConstructors() if (lineSeparator != null) { @@ -441,8 +455,8 @@ public class Base64 extends BaseNCodec { context.eof = true; break; } - if (b >= 0 && b < DECODE_TABLE.length) { - final int result = DECODE_TABLE[b]; + if (b >= 0 && b < decodeTable.length) { + final int result = decodeTable[b]; if (result >= 0) { context.modulus = (context.modulus+1) % BYTES_PER_ENCODED_BLOCK; context.ibitWorkArea = (context.ibitWorkArea << BITS_PER_ENCODED_BYTE) + result; @@ -495,7 +509,7 @@ public class Base64 extends BaseNCodec { * @since 1.4 */ public static boolean isBase64(final byte octet) { - return octet == PAD_DEFAULT || (octet >= 0 && octet < DECODE_TABLE.length && DECODE_TABLE[octet] != -1); + return octet == PAD_DEFAULT || (octet >= 0 && octet < STANDARD_DECODE_TABLE.length && STANDARD_DECODE_TABLE[octet] != -1); } /** @@ -678,6 +692,10 @@ public class Base64 extends BaseNCodec { return new Base64().decode(base64String); } + public static byte[] decodeBase64URLSafe(final String base64String) { + return new Base64(true).decode(base64String); + } + /** * Decodes Base64 data into octets. * <p> diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java index a394927..dc0eabf 100644 --- a/test/org/apache/coyote/http2/Http2TestBase.java +++ b/test/org/apache/coyote/http2/Http2TestBase.java @@ -81,7 +81,7 @@ public abstract class Http2TestBase extends TomcatBaseTest { static { byte[] empty = new byte[0]; - EMPTY_HTTP2_SETTINGS_HEADER = "HTTP2-Settings: " + Base64.encodeBase64String(empty) + "\r\n"; + EMPTY_HTTP2_SETTINGS_HEADER = "HTTP2-Settings: " + Base64.encodeBase64URLSafeString(empty) + "\r\n"; } protected static final String TRAILER_HEADER_NAME = "x-trailertest"; diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 6427313..48f3408 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -45,6 +45,15 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 9.0.27 (markt)" rtext="in development"> + <subsection name="Coyote"> + <changelog> + <fix> + Use URL safe base 64 encoding rather than standard base 64 encoding when + generating or parsing the <code>HTTP2-Settings</code> header as part of + an HTTP upgrade to <code>h2c</code> as required by RFC 7540. (markt) + </fix> + </changelog> + </subsection> </section> <section name="Tomcat 9.0.26 (markt)" rtext="release in progress"> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org