Skip to content

Commit

Permalink
Swtich to new Enum for error code
Browse files Browse the repository at this point in the history 
Add parsing for the Goaway frame (some TODOs in this code)
Enable test for header decoding issues that now passes

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1683412 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@markt-asf
markt-asf committed Jun 3, 2015
1 parent 3592641 commit 91d6b0e
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 45 deletions.
8 changes: 4 additions & 4 deletions java/org/apache/coyote/http2/ConnectionSettings.java
View file
Expand Up @@ -84,7 +84,7 @@ 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);
Long.toString(headerTableSize)), 0, ErrorCode.PROTOCOL_ERROR);
}
this.headerTableSize = (int) headerTableSize;
}
Expand All @@ -98,7 +98,7 @@ public void setEnablePush(long enablePush) throws IOException {
// will never be negative
if (enablePush > 1) {
throw new Http2Exception(sm.getString("connectionSettings.enablePushInvalid",
Long.toString(enablePush)), 0, Http2Exception.PROTOCOL_ERROR);
Long.toString(enablePush)), 0, ErrorCode.PROTOCOL_ERROR);
}
this.enablePush = (enablePush == 1);
}
Expand All @@ -119,7 +119,7 @@ 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);
0, ErrorCode.PROTOCOL_ERROR);
}
this.initialWindowSize = (int) initialWindowSize;
}
Expand All @@ -132,7 +132,7 @@ 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), Integer.toString(MIN_MAX_FRAME_SIZE),
Integer.toString(MAX_MAX_FRAME_SIZE)), 0, Http2Exception.PROTOCOL_ERROR);
Integer.toString(MAX_MAX_FRAME_SIZE)), 0, ErrorCode.PROTOCOL_ERROR);
}
this.maxFrameSize = (int) maxFrameSize;
}
Expand Down
22 changes: 3 additions & 19 deletions java/org/apache/coyote/http2/Http2Exception.java
View file
Expand Up @@ -22,27 +22,11 @@ public class Http2Exception extends IOException {

private static final long serialVersionUID = 1L;

public static final byte[] NO_ERROR = { 0x00, 0x00, 0x00, 0x00 };
public static final byte[] PROTOCOL_ERROR = { 0x00, 0x00, 0x00, 0x01 };
public static final byte[] INTERNAL_ERROR = { 0x00, 0x00, 0x00, 0x02 };
public static final byte[] FLOW_CONTROL_ERROR = { 0x00, 0x00, 0x00, 0x03 };
public static final byte[] SETTINGS_TIMEOUT = { 0x00, 0x00, 0x00, 0x04 };
public static final byte[] STREAM_CLOSED = { 0x00, 0x00, 0x00, 0x05 };
public static final byte[] FRAME_SIZE_ERROR = { 0x00, 0x00, 0x00, 0x06};
public static final byte[] REFUSED_STREAM = { 0x00, 0x00, 0x00, 0x07};
public static final byte[] CANCEL = { 0x00, 0x00, 0x00, 0x08};
public static final byte[] COMPRESSION_ERROR= { 0x00, 0x00, 0x00, 0x09};
public static final byte[] CONNECT_ERROR = { 0x00, 0x00, 0x00, 0x0a};
public static final byte[] ENHANCE_YOUR_CALM = { 0x00, 0x00, 0x00, 0x0b};
public static final byte[] INADEQUATE_SECURITY = { 0x00, 0x00, 0x00, 0x0c};
public static final byte[] HTTP_1_1_REQUIRED = { 0x00, 0x00, 0x00, 0x0d};


private final int streamId;
private final byte[] errorCode;
private final ErrorCode errorCode;


public Http2Exception(String msg, int streamId, byte[] errorCode) {
public Http2Exception(String msg, int streamId, ErrorCode errorCode) {
super(msg);
this.streamId = streamId;
this.errorCode = errorCode;
Expand All @@ -54,7 +38,7 @@ public int getStreamId() {
}


public byte[] getErrorCode() {
public ErrorCode getErrorCode() {
return errorCode;
}
}
68 changes: 53 additions & 15 deletions java/org/apache/coyote/http2/Http2Parser.java
View file
Expand Up @@ -38,6 +38,7 @@ class Http2Parser {
private static final int FRAME_TYPE_PRIORITY = 2;
private static final int FRAME_TYPE_SETTINGS = 4;
private static final int FRAME_TYPE_PING = 6;
private static final int FRAME_TYPE_GOAWAY = 7;
private static final int FRAME_TYPE_WINDOW_UPDATE = 8;

private final String connectionId;
Expand Down Expand Up @@ -88,13 +89,13 @@ private boolean readFrame(boolean block, Integer expected) throws IOException {
if (expected != null && frameType != expected.intValue()) {
throw new Http2Exception(sm.getString("http2Parser.processFrame.unexpectedType",
expected, Integer.toString(frameType)),
streamId, Http2Exception.PROTOCOL_ERROR);
streamId, ErrorCode.PROTOCOL_ERROR);
}

if (payloadSize > maxPayloadSize) {
throw new Http2Exception(sm.getString("http2Parser.payloadTooBig",
Integer.toString(payloadSize), Integer.toString(maxPayloadSize)),
streamId, Http2Exception.FRAME_SIZE_ERROR);
streamId, ErrorCode.FRAME_SIZE_ERROR);
}

switch (frameType) {
Expand All @@ -113,6 +114,9 @@ private boolean readFrame(boolean block, Integer expected) throws IOException {
case FRAME_TYPE_PING:
readPingFrame(streamId, flags, payloadSize);
break;
case FRAME_TYPE_GOAWAY:
readGoawayFrame(streamId, flags, payloadSize);
break;
case FRAME_TYPE_WINDOW_UPDATE:
readWindowUpdateFrame(streamId, flags, payloadSize);
break;
Expand All @@ -135,7 +139,7 @@ private void readDataFrame(int streamId, int flags, int payloadSize) throws IOEx
// Validate the stream
if (streamId == 0) {
throw new Http2Exception(sm.getString("http2Parser.processFrameData.invalidStream"),
0, Http2Exception.PROTOCOL_ERROR);
0, ErrorCode.PROTOCOL_ERROR);
}

// Process the Stream
Expand Down Expand Up @@ -180,7 +184,7 @@ private void readHeadersFrame(int streamId, int flags, int payloadSize) throws I
// Validate the stream
if (streamId == 0) {
throw new Http2Exception(sm.getString("http2Parser.processFrameHeaders.invalidStream"),
0, Http2Exception.PROTOCOL_ERROR);
0, ErrorCode.PROTOCOL_ERROR);
}

// TODO Handle end of headers flag
Expand Down Expand Up @@ -230,7 +234,7 @@ private void readHeadersFrame(int streamId, int flags, int payloadSize) throws I
} catch (HpackException hpe) {
throw new Http2Exception(
sm.getString("http2Parser.processFrameHeaders.decodingFailed"),
0, Http2Exception.PROTOCOL_ERROR);
0, ErrorCode.PROTOCOL_ERROR);
}
// switches to write mode
headerReadBuffer.compact();
Expand All @@ -240,7 +244,7 @@ private void readHeadersFrame(int streamId, int flags, int payloadSize) throws I
if (headerReadBuffer.position() > 0) {
throw new Http2Exception(
sm.getString("http2Parser.processFrameHeaders.decodingDataLeft"),
0, Http2Exception.PROTOCOL_ERROR);
0, ErrorCode.PROTOCOL_ERROR);
}

swallow(padLength);
Expand All @@ -258,11 +262,11 @@ private void readPriorityFrame(int streamId, int flags, int payloadSize) throws
// Validate the frame
if (streamId == 0) {
throw new Http2Exception(sm.getString("http2Parser.processFramePriority.invalidStream"),
0, Http2Exception.PROTOCOL_ERROR);
0, ErrorCode.PROTOCOL_ERROR);
}
if (payloadSize != 5) {
throw new Http2Exception(sm.getString("http2Parser.processFramePriority.invalidPayloadSize",
Integer.toString(payloadSize)), streamId, Http2Exception.FRAME_SIZE_ERROR);
Integer.toString(payloadSize)), streamId, ErrorCode.FRAME_SIZE_ERROR);
}

byte[] payload = new byte[5];
Expand All @@ -286,16 +290,16 @@ private void readSettingsFrame(int streamId, int flags, int payloadSize) throws
// Validate the frame
if (streamId != 0) {
throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.invalidStream",
Integer.toString(streamId)), 0, Http2Exception.FRAME_SIZE_ERROR);
Integer.toString(streamId)), 0, ErrorCode.FRAME_SIZE_ERROR);
}
if (payloadSize % 6 != 0) {
throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.invalidPayloadSize",
Integer.toString(payloadSize)), 0, Http2Exception.FRAME_SIZE_ERROR);
Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
}
boolean ack = (flags & 0x1) != 0;
if (payloadSize > 0 && ack) {
throw new Http2Exception(sm.getString("http2Parser.processFrameSettings.ackWithNonZeroPayload"),
0, Http2Exception.FRAME_SIZE_ERROR);
0, ErrorCode.FRAME_SIZE_ERROR);
}

if (payloadSize != 0) {
Expand All @@ -322,11 +326,11 @@ private void readPingFrame(int streamId, int flags, int payloadSize)
// Validate the frame
if (streamId != 0) {
throw new Http2Exception(sm.getString("http2Parser.processFramePing.invalidStream",
Integer.toString(streamId)), 0, Http2Exception.FRAME_SIZE_ERROR);
Integer.toString(streamId)), 0, ErrorCode.FRAME_SIZE_ERROR);
}
if (payloadSize != 8) {
throw new Http2Exception(sm.getString("http2Parser.processFramePing.invalidPayloadSize",
Integer.toString(payloadSize)), 0, Http2Exception.FRAME_SIZE_ERROR);
Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
}
if ((flags & 0x1) == 0) {
// Read the payload
Expand All @@ -339,6 +343,37 @@ private void readPingFrame(int streamId, int flags, int payloadSize)
}


private void readGoawayFrame(int streamId, int flags, int payloadSize)
throws IOException {
if (log.isDebugEnabled()) {
log.debug(sm.getString("http2Parser.processFrame", connectionId,
Integer.toString(streamId), Integer.toString(flags),
Integer.toString(payloadSize)));
}

// Validate the frame
if (streamId != 0) {
throw new Http2Exception(sm.getString("http2Parser.processFrameGoaway.invalidStream",
Integer.toString(streamId)), 0, ErrorCode.PROTOCOL_ERROR);
}

if (payloadSize < 8) {
throw new Http2Exception(sm.getString("http2Parser.processFrameGoaway.payloadTooSmall",
connectionId, Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
}

byte[] payload = new byte[payloadSize];
input.fill(true, payload);

int lastStreamId = ByteUtil.get31Bits(payload, 0);
long errorCode = ByteUtil.getFourBytes(payload, 4);
String debugData = null;
if (payloadSize > 8) {
debugData = new String(payload, 8, payloadSize - 8, StandardCharsets.UTF_8);
}
output.goaway(lastStreamId, errorCode, debugData);
}

private void readWindowUpdateFrame(int streamId, int flags, int payloadSize)
throws IOException {
if (log.isDebugEnabled()) {
Expand All @@ -350,7 +385,7 @@ private void readWindowUpdateFrame(int streamId, int flags, int payloadSize)
if (payloadSize != 4) {
// Use stream 0 since this is always a connection error
throw new Http2Exception(sm.getString("http2Parser.processFrameWindowUpdate.invalidPayloadSize",
Integer.toString(payloadSize)), 0, Http2Exception.FRAME_SIZE_ERROR);
Integer.toString(payloadSize)), 0, ErrorCode.FRAME_SIZE_ERROR);
}

byte[] payload = new byte[4];
Expand All @@ -365,7 +400,7 @@ private void readWindowUpdateFrame(int streamId, int flags, int payloadSize)
// Validate the data
if (windowSizeIncrement == 0) {
throw new Http2Exception("http2Parser.processFrameWindowUpdate.invalidIncrement",
streamId, Http2Exception.PROTOCOL_ERROR);
streamId, ErrorCode.PROTOCOL_ERROR);
}

output.incrementWindowSize(streamId, windowSizeIncrement);
Expand Down Expand Up @@ -499,6 +534,9 @@ static interface Output {
void pingReceive(byte[] payload) throws IOException;
void pingAck();

// Goaway
void goaway(int lastStreamId, long errorCode, String debugData);

// Window size
void incrementWindowSize(int streamId, int increment);

Expand Down
24 changes: 21 additions & 3 deletions java/org/apache/coyote/http2/Http2UpgradeHandler.java
View file
Expand Up @@ -122,6 +122,7 @@ public class Http2UpgradeHandler extends AbstractStream implements InternalHttpU
private long writeTimeout = 10000;

private final Map<Integer,Stream> streams = new HashMap<>();
private int maxStreamId = -1;

// Tracking for when the connection is blocked (windowSize < 1)
private final Object backLogLock = new Object();
Expand Down Expand Up @@ -346,15 +347,20 @@ public void destroy() {

private void close(Http2Exception h2e) {
// Write a GOAWAY frame.
byte[] payload = h2e.getMessage().getBytes(StandardCharsets.UTF_8);
byte[] fixedPayload = new byte[8];
// TODO needs to be correct value
ByteUtil.set31Bits(fixedPayload, 0, (2 << 31) - 1);
ByteUtil.setFourBytes(fixedPayload, 4, h2e.getErrorCode().getErrorCode());
byte[] debugMessage = h2e.getMessage().getBytes(StandardCharsets.UTF_8);
byte[] payloadLength = new byte[3];
ByteUtil.setThreeBytes(payloadLength, 0, payload.length);
ByteUtil.setThreeBytes(payloadLength, 0, debugMessage.length + 8);

try {
synchronized (socketWrapper) {
socketWrapper.write(true, payloadLength, 0, payloadLength.length);
socketWrapper.write(true, GOAWAY, 0, GOAWAY.length);
socketWrapper.write(true, payload, 0, payload.length);
socketWrapper.write(true, fixedPayload, 0, 8);
socketWrapper.write(true, debugMessage, 0, debugMessage.length);
socketWrapper.flush(true);
}
} catch (IOException ioe) {
Expand Down Expand Up @@ -783,6 +789,18 @@ public void pingAck() {
}


@Override
public void goaway(int lastStreamId, long errorCode, String debugData) {
if (log.isDebugEnabled()) {
log.debug(sm.getString("upgradeHandler.goaway.debug", connectionId,
Integer.toString(lastStreamId), Long.toHexString(errorCode), debugData));
}

// TODO: Do more than just record this
maxStreamId = lastStreamId;
}


@Override
public void incrementWindowSize(int streamId, int increment) {
AbstractStream stream = getStream(streamId);
Expand Down
3 changes: 3 additions & 0 deletions java/org/apache/coyote/http2/LocalStrings.properties
View file
Expand Up @@ -38,6 +38,8 @@ http2Parser.preface.io=Unable to read connection preface
http2Parser.processFrame=Connection [{0}], Stream [{1}], Flags [{2}], Payload size [{3}]
http2Parser.processFrame.unexpectedType=Expected frame type [{0}] but received frame type [{1}]
http2Parser.processFrameData.invalidStream=Data frame received for stream [0]
http2Parser.processFrameGoaway.invalidStream=Goaway frame received for stream [{0}]
http2Parser.processFrameGoaway.payloadTooSmall=Connection [{0}]: Goaway payload size was [{1}] which is less than the minimum 8
http2Parser.processFrameHeaders.invalidStream=Headers frame received for stream [0]
http2Parser.processFrameHeaders.decodingFailed=There was an error during the HPACK decoding of HTTP headers
http2Parser.processFrameHeaders.decodingDataLeft=Data left over after HPACK decoding - it should have been consumed
Expand All @@ -58,6 +60,7 @@ stream.write=Connection [{0}], Stream [{1}]
streamProcessor.httpupgrade.notsupported=HTTP upgrade is not supported within HTTP/2 streams

upgradeHandler.connectionError=An error occurred that requires the HTTP/2 connection to be closed.
upgradeHandler.goaway.debug=Connection [{0}], Goaway, Last stream [{1}], Error code [{2}], Debug data [{3}]
upgradeHandler.init=Connection [{0}]
upgradeHandler.ioerror=Connection [{0}]
upgradeHandler.sendPrefaceFail=Failed to send preface to client
Expand Down
6 changes: 6 additions & 0 deletions test/org/apache/coyote/http2/Http2TestBase.java
View file
Expand Up @@ -446,6 +446,12 @@ public void pingAck() {
}


@Override
public void goaway(int lastStreamId, long errorCode, String debugData) {
trace.append("0-Goaway-[" + lastStreamId + "]-[" + errorCode + "]-[" + debugData + "]");
}


@Override
public void incrementWindowSize(int streamId, int increment) {
trace.append(streamId + "-WindowSize-[" + increment + "]\n");
Expand Down
8 changes: 4 additions & 4 deletions test/org/apache/coyote/http2/TestHttp2Section_4_3.java
View file
Expand Up @@ -19,7 +19,6 @@
import java.nio.ByteBuffer;

import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;

/**
Expand All @@ -31,7 +30,6 @@
*/
public class TestHttp2Section_4_3 extends Http2TestBase {

@Ignore // Appears to trigger various problems
@Test
public void testHeaderDecodingError() throws Exception {
hpackEncoder = new HpackEncoder(ConnectionSettings.DEFAULT_HEADER_TABLE_SIZE);
Expand All @@ -50,9 +48,11 @@ public void testHeaderDecodingError() throws Exception {
// Process the request
writeSimpleRequest(frameHeader, headersPayload);

readSimpleResponse();
Assert.assertEquals(getSimpleResponseTrace(3), output.getTrace());
// Read GOAWAY frame
parser.readFrame(true);

Assert.assertTrue(output.getTrace(),
output.getTrace().startsWith("0-Goaway-[2147483647]-[1]-["));
}


Expand Down

0 comments on commit 91d6b0e

Please sign in to comment.