Author: markt Date: Mon Jun 1 20:03:13 2015 New Revision: 1682984 URL: http://svn.apache.org/r1682984 Log: Fix an overflow issue in 4 byte to unsigned integer conversion (needed to be a long). Fix a related TODO in ConnectionSettings
Added: tomcat/trunk/test/org/apache/coyote/http2/TestByteUtil.java (with props) Modified: tomcat/trunk/java/org/apache/coyote/http2/ByteUtil.java tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Modified: tomcat/trunk/java/org/apache/coyote/http2/ByteUtil.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/ByteUtil.java?rev=1682984&r1=1682983&r2=1682984&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/ByteUtil.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/ByteUtil.java Mon Jun 1 20:03:13 2015 @@ -68,8 +68,8 @@ public class ByteUtil { } - public static int getFourBytes(byte[] input, int firstByte) { - return ((input[firstByte] & 0xFF) << 24) + ((input[firstByte + 1] & 0xFF) << 16) + + public static long getFourBytes(byte[] input, int firstByte) { + return ((long)(input[firstByte] & 0xFF) << 24) + ((input[firstByte + 1] & 0xFF) << 16) + ((input[firstByte + 2] & 0xFF) << 8) + (input[firstByte + 3] & 0xFF); } } Modified: tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java?rev=1682984&r1=1682983&r2=1682984&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/ConnectionSettings.java Mon Jun 1 20:03:13 2015 @@ -28,20 +28,19 @@ public class ConnectionSettings { private final StringManager sm = StringManager.getManager(ConnectionSettings.class); public static final int DEFAULT_WINDOW_SIZE = (1 << 16) - 1; - // TODO: The maximum allowed in a settings frame as 2^32 (unsigned) - private static final int UNLIMITED = (1 << 31) -1; // Use the maximum possible + private static final long UNLIMITED = ((long)1 << 32); // Use the maximum possible private static final int MAX_WINDOW_SIZE = (1 << 31) - 1; private static final int MIN_MAX_FRAME_SIZE = 1 << 14; private static final int MAX_MAX_FRAME_SIZE = (1 << 24) - 1; private volatile int headerTableSize = 4096; - private volatile int enablePush = 1; - private volatile int maxConcurrentStreams = UNLIMITED; + private volatile boolean enablePush = true; + private volatile long maxConcurrentStreams = UNLIMITED; private volatile int initialWindowSize = DEFAULT_WINDOW_SIZE; private volatile int maxFrameSize = MIN_MAX_FRAME_SIZE; - private volatile int maxHeaderListSize = UNLIMITED; + private volatile long maxHeaderListSize = UNLIMITED; - public void set(int parameterId, int value) throws IOException { + public void set(int parameterId, long value) throws IOException { if (log.isDebugEnabled()) { log.debug(sm.getString("connectionSettings.debug", Integer.toString(parameterId), Long.toString(value))); @@ -77,29 +76,34 @@ public class ConnectionSettings { public int getHeaderTableSize() { return headerTableSize; } - public void setHeaderTableSize(int headerTableSize) { - this.headerTableSize = headerTableSize; + public void setHeaderTableSize(long headerTableSize) throws IOException { + // Need to put a sensible limit on this. Start with 16k (default is 4k) + if (headerTableSize > (16 * 1024)) { + throw new Http2Exception(sm.getString("connectionSettings.headerTableSizeLimit", + Long.toString(headerTableSize)), 0, Http2Exception.PROTOCOL_ERROR); + } + this.headerTableSize = (int) headerTableSize; } - public int getEnablePush() { + public boolean getEnablePush() { return enablePush; } - public void setEnablePush(int enablePush) throws IOException { + public void setEnablePush(long enablePush) throws IOException { // Can't be less than zero since the result of the byte->long conversion // will never be negative if (enablePush > 1) { throw new Http2Exception(sm.getString("connectionSettings.enablePushInvalid", Long.toString(enablePush)), 0, Http2Exception.PROTOCOL_ERROR); } - this.enablePush = enablePush; + this.enablePush = (enablePush == 1); } - public int getMaxConcurrentStreams() { + public long getMaxConcurrentStreams() { return maxConcurrentStreams; } - public void setMaxConcurrentStreams(int maxConcurrentStreams) { + public void setMaxConcurrentStreams(long maxConcurrentStreams) { this.maxConcurrentStreams = maxConcurrentStreams; } @@ -107,33 +111,33 @@ public class ConnectionSettings { public int getInitialWindowSize() { return initialWindowSize; } - public void setInitialWindowSize(int initialWindowSize) throws IOException { + public void setInitialWindowSize(long initialWindowSize) throws IOException { if (initialWindowSize > MAX_WINDOW_SIZE) { throw new Http2Exception(sm.getString("connectionSettings.windowSizeTooBig", Long.toString(initialWindowSize), Long.toString(MAX_WINDOW_SIZE)), 0, Http2Exception.PROTOCOL_ERROR); } - this.initialWindowSize = initialWindowSize; + this.initialWindowSize = (int) initialWindowSize; } public int getMaxFrameSize() { return maxFrameSize; } - public void setMaxFrameSize(int maxFrameSize) throws IOException { + public void setMaxFrameSize(long maxFrameSize) throws IOException { if (maxFrameSize < MIN_MAX_FRAME_SIZE || maxFrameSize > MAX_MAX_FRAME_SIZE) { throw new Http2Exception(sm.getString("connectionSettings.maxFrameSizeInvalid", - Long.toString(maxFrameSize), Long.toString(MIN_MAX_FRAME_SIZE), - Long.toString(MAX_MAX_FRAME_SIZE)), 0, Http2Exception.PROTOCOL_ERROR); + Long.toString(maxFrameSize), Integer.toString(MIN_MAX_FRAME_SIZE), + Integer.toString(MAX_MAX_FRAME_SIZE)), 0, Http2Exception.PROTOCOL_ERROR); } - this.maxFrameSize = maxFrameSize; + this.maxFrameSize = (int) maxFrameSize; } - public int getMaxHeaderListSize() { + public long getMaxHeaderListSize() { return maxHeaderListSize; } - public void setMaxHeaderListSize(int maxHeaderListSize) { + public void setMaxHeaderListSize(long maxHeaderListSize) { this.maxHeaderListSize = maxHeaderListSize; } } Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java?rev=1682984&r1=1682983&r2=1682984&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Mon Jun 1 20:03:13 2015 @@ -184,7 +184,7 @@ public class Http2UpgradeHandler extends for (int i = 0; i < settings.length % 6; i++) { int id = ByteUtil.getTwoBytes(settings, i * 6); - int value = ByteUtil.getFourBytes(settings, (i * 6) + 2); + long value = ByteUtil.getFourBytes(settings, (i * 6) + 2); remoteSettings.set(id, value); } @@ -568,7 +568,7 @@ public class Http2UpgradeHandler extends for (int i = 0; i < payloadSize / 6; i++) { readFully(setting); int id = ByteUtil.getTwoBytes(setting, 0); - int value = ByteUtil.getFourBytes(setting, 2); + long value = ByteUtil.getFourBytes(setting, 2); remoteSettings.set(id, value); } } Modified: tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties?rev=1682984&r1=1682983&r2=1682984&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Mon Jun 1 20:03:13 2015 @@ -21,6 +21,7 @@ connectionPrefaceParser.mismatch=An unex connectionSettings.debug=Parameter type [{0}] set to [{1}] connectionSettings.enablePushInvalid=The requested value for enable push [{0}] is not one of the permitted values (zero or one) +connectionSettings.headerTableSizeLimit=Attempted to set a header table size of [{0}] but the limit is 16k connectionSettings.maxFrameSizeInvalid=The requested maximum frame size of [{0}] is ouside the permitted range of [{1}] to [{2}] connectionSettings.unknown=An unknown setting with identifier [{0}] and value [{1}] was ignored connectionSettings.windowSizeTooBig=The requested window size of [{0}] is bigger than the maximum permitted value of [{1}] Added: tomcat/trunk/test/org/apache/coyote/http2/TestByteUtil.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestByteUtil.java?rev=1682984&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/coyote/http2/TestByteUtil.java (added) +++ tomcat/trunk/test/org/apache/coyote/http2/TestByteUtil.java Mon Jun 1 20:03:13 2015 @@ -0,0 +1,39 @@ +/* + * 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.coyote.http2; + +import org.junit.Assert; +import org.junit.Test; + +public class TestByteUtil { + + @Test + public void testGet31Bits() { + byte[] input = new byte[] { (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff }; + int result = ByteUtil.get31Bits(input, 0); + Assert.assertEquals(0x7fffffff, result); + } + + + @Test + public void testGetFourBytes() { + byte[] input = new byte[] { (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff }; + long result = ByteUtil.getFourBytes(input, 0); + Assert.assertEquals(0xffffffffL, result); + } + +} Propchange: tomcat/trunk/test/org/apache/coyote/http2/TestByteUtil.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: tomcat/trunk/test/org/apache/coyote/http2/TestByteUtil.java ------------------------------------------------------------------------------ svn:mime-type = text/plain --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org