Author: markt
Date: Fri Jun 5 20:17:07 2015
New Revision: 1683857
URL: http://svn.apache.org/r1683857
Log:
Change state on start of headers not end of headers
More renames to improve clarity
Fill some gaps in the debug logging
Add a (currently disabled because it fails) test that should trigger a stream
close
Modified:
tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
tomcat/trunk/java/org/apache/coyote/http2/Stream.java
tomcat/trunk/java/org/apache/coyote/http2/StreamStateMachine.java
tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java
tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
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=1683857&r1=1683856&r2=1683857&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
(original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Fri Jun
5 20:17:07 2015
@@ -133,6 +133,9 @@ public class Http2UpgradeHandler extends
// Initial HTTP request becomes stream 1.
if (coyoteRequest != null) {
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("upgradeHandler.upgrade",
connectionId));
+ }
Integer key = Integer.valueOf(1);
Stream stream = new Stream(key, this, coyoteRequest);
streams.put(key, stream);
@@ -238,9 +241,10 @@ public class Http2UpgradeHandler extends
if (log.isDebugEnabled()) {
log.debug(sm.getString("upgradeHandler.connectionError"), h2e);
}
- close(h2e);
+ closeConnecion(h2e);
break;
} else {
+
// Stream error
// TODO Reset stream
}
@@ -263,7 +267,7 @@ public class Http2UpgradeHandler extends
if (h2e.getStreamId() == 0) {
// Connection error
log.warn(sm.getString("upgradeHandler.connectionError"),
h2e);
- close(h2e);
+ closeConnecion(h2e);
break;
} else {
// Stream error
@@ -335,7 +339,7 @@ public class Http2UpgradeHandler extends
}
- private void close(Http2Exception h2e) {
+ private void closeConnecion(Http2Exception h2e) {
// Write a GOAWAY frame.
byte[] fixedPayload = new byte[8];
// TODO needs to be correct value
@@ -729,6 +733,7 @@ public class Http2UpgradeHandler extends
public HeaderEmitter headersStart(int streamId) throws Http2Exception {
Stream stream = getStream(streamId);
stream.checkState(FrameType.HEADERS);
+ stream.receivedStartOfHeaders();
return stream;
}
@@ -749,7 +754,6 @@ public class Http2UpgradeHandler extends
@Override
public void headersEnd(int streamId) {
Stream stream = getStream(streamId);
- stream.receivedEndOfHeaders();
// Process this stream on a container thread
StreamProcessor streamProcessor = new StreamProcessor(stream, adapter,
socketWrapper);
streamProcessor.setSslSupport(sslSupport);
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=1683857&r1=1683856&r2=1683857&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Fri Jun
5 20:17:07 2015
@@ -64,7 +64,6 @@ streamProcessor.httpupgrade.notsupported
streamStateMachine.debug.change=Connection [{0}], Stream [{1}], State changed
from [{2}] to [{3}]
streamStateMachine.invalidFrame=Connection [{0}], Stream [{1}], State [{2}],
Frame type [{3}]
-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}]
@@ -72,8 +71,10 @@ upgradeHandler.sendPrefaceFail=Failed to
upgradeHandler.socketCloseFailed=Error closing socket
upgradeHandler.unexpectedEos=Unexpected end of stream
upgradeHandler.unexpectedStatus=An unexpected value of status ([{0}]) was
passed to this method
+upgradeHandler.upgrade=Connection [{0}], HTTP/1.1 upgrade to stream [1]
upgradeHandler.upgradeDispatch.entry=Entry, Connection [{0}], SocketStatus
[{1}]
upgradeHandler.upgradeDispatch.exit=Exit, Connection [{0}], SocketState [{1}]
+upgradeHandler.writeBody=Connection [{0}], Stream [{1}], Data length [{2}]
upgradeHandler.writeHeaders=Connection [{0}], Stream [{1}]
writeStateMachine.endWrite.ise=It is illegal to specify [{0}] for the new
state once a write has completed
Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1683857&r1=1683856&r2=1683857&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Fri Jun 5 20:17:07
2015
@@ -66,7 +66,7 @@ public class Stream extends AbstractStre
this.coyoteRequest = coyoteRequest;
this.inputBuffer = null;
// Headers have been populated by this point
- state.receivedEndOfHeaders();
+ state.receivedStartOfHeaders();
// TODO Assuming the body has been read at this point is not valid
state.recievedEndOfStream();
}
@@ -235,8 +235,8 @@ public class Stream extends AbstractStre
}
- void receivedEndOfHeaders() {
- state.receivedEndOfHeaders();
+ void receivedStartOfHeaders() {
+ state.receivedStartOfHeaders();
}
Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamStateMachine.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamStateMachine.java?rev=1683857&r1=1683856&r2=1683857&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/StreamStateMachine.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/StreamStateMachine.java Fri Jun
5 20:17:07 2015
@@ -58,13 +58,13 @@ public class StreamStateMachine {
}
- public synchronized void sentEndOfHeaders() {
+ public synchronized void sentStartOfHeaders() {
stateChange(State.IDLE, State.OPEN);
stateChange(State.RESERVED_LOCAL, State.HALF_CLOSED_REMOTE);
}
- public synchronized void receivedEndOfHeaders() {
+ public synchronized void receivedStartOfHeaders() {
stateChange(State.IDLE, State.OPEN);
stateChange(State.RESERVED_REMOTE, State.HALF_CLOSED_LOCAL);
}
@@ -82,6 +82,16 @@ public class StreamStateMachine {
}
+ public synchronized void sendReset() {
+ stateChange(state, State.CLOSED_TX);
+ }
+
+
+ public synchronized void receiveReset() {
+ stateChange(state, State.CLOSED_RST);
+ }
+
+
private void stateChange(State oldState, State newState) {
if (state == oldState) {
state = newState;
@@ -93,16 +103,6 @@ public class StreamStateMachine {
}
- public synchronized void sendReset() {
- state = State.CLOSED_TX;
- }
-
-
- public synchronized void receiveReset() {
- state = State.CLOSED_RST;
- }
-
-
public synchronized void checkFrameType(FrameType frameType) throws
Http2Exception {
// No state change. Checks that the frame type is valid for the current
// state of this stream.
Modified: tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java?rev=1683857&r1=1683856&r2=1683857&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Fri Jun 5
20:17:07 2015
@@ -355,6 +355,23 @@ public abstract class Http2TestBase exte
}
+ void sendRst(int streamId, long errorCode) throws IOException {
+ byte[] rstFrame = new byte[13];
+ // length is always 4
+ rstFrame[2] = 0x04;
+ // type is always 3
+ rstFrame[3] = 0x03;
+ // no flags
+ // Stream ID
+ ByteUtil.set31Bits(rstFrame, 5, streamId);
+ // Payload
+ ByteUtil.setFourBytes(rstFrame, 9, errorCode);
+
+ os.write(rstFrame);
+ os.flush();
+ }
+
+
void sendPing() throws IOException {
os.write(PING_FRAME);
os.flush();
Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_1.java?rev=1683857&r1=1683856&r2=1683857&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_1.java
(original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_5_1.java Fri Jun
5 20:17:07 2015
@@ -16,7 +16,10 @@
*/
package org.apache.coyote.http2;
+import java.nio.ByteBuffer;
+
import org.junit.Assert;
+import org.junit.Ignore;
import org.junit.Test;
/**
@@ -82,7 +85,39 @@ public class TestHttp2Section_5_1 extend
@Test
- public void testClosedInvalidFrame() throws Exception {
+ @Ignore // Need to handle stream closes
+ public void testClosedInvalidFrame01() throws Exception {
+ hpackEncoder = new
HpackEncoder(ConnectionSettings.DEFAULT_HEADER_TABLE_SIZE);
+
+ // HTTP2 upgrade
+ http2Connect();
+
+ // Build the simple request
+ byte[] frameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+ buildSimpleRequest(frameHeader, headersPayload, 3);
+
+ // Remove the end of stream and end of headers flags
+ frameHeader[4] = 0;
+
+ // Process the request
+ writeFrame(frameHeader, headersPayload);
+
+ // Send a rst
+ sendRst(3, ErrorCode.INTERNAL_ERROR.getErrorCode());
+
+ // Then try sending some data (which should fail)
+ sendData(3, new byte[] {});
+ parser.readFrame(true);
+
+ Assert.assertTrue(output.getTrace(),
+ output.getTrace().startsWith("0-Goaway-[2147483647]-[" +
+ ErrorCode.STREAM_CLOSED.getErrorCode() + "]-["));
+ }
+
+
+ @Test
+ public void testClosedInvalidFrame02() throws Exception {
http2Connect();
// Stream 1 is closed. This should trigger a stream error
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
