Author: markt
Date: Tue Jun 2 13:44:16 2015
New Revision: 1683114
URL: http://svn.apache.org/r1683114
Log:
More work on the setting up h2c connections
- clients must send a settings frame in the preface (as well as the upgrade)
- refactor readFrame() so an expected type can be specified
- ensure the first frame the server sends is a settings frame
- restore sending the ack for a settings frame
Modified:
tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java
tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java
tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java
Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java?rev=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Tue Jun 2
13:44:16 2015
@@ -72,6 +72,10 @@ class Http2Parser {
* @throws IOException If an IO error occurs while trying to read a frame
*/
boolean readFrame(boolean block) throws IOException {
+ return readFrame(block, null);
+ }
+
+ private boolean readFrame(boolean block, Integer expected) throws
IOException {
if (!input.fill(block, frameHeaderBuffer)) {
return false;
}
@@ -81,6 +85,12 @@ class Http2Parser {
int flags = ByteUtil.getOneByte(frameHeaderBuffer, 4);
int streamId = ByteUtil.get31Bits(frameHeaderBuffer, 5);
+ if (expected != null && frameType != expected.intValue()) {
+ throw new
Http2Exception(sm.getString("http2Parser.processFrame.unexpectedType",
+ expected, Integer.toString(frameType)),
+ streamId, Http2Exception.PROTOCOL_ERROR);
+ }
+
if (payloadSize > maxPayloadSize) {
throw new Http2Exception(sm.getString("http2Parser.payloadTooBig",
Integer.toString(payloadSize),
Integer.toString(maxPayloadSize)),
@@ -282,15 +292,13 @@ class Http2Parser {
throw new
Http2Exception(sm.getString("http2Parser.processFrameSettings.invalidPayloadSize",
Integer.toString(payloadSize)), 0,
Http2Exception.FRAME_SIZE_ERROR);
}
- if (payloadSize > 0 && (flags & 0x1) != 0) {
+ boolean ack = (flags & 0x1) != 0;
+ if (payloadSize > 0 && ack) {
throw new
Http2Exception(sm.getString("http2Parser.processFrameSettings.ackWithNonZeroPayload"),
0, Http2Exception.FRAME_SIZE_ERROR);
}
- if (payloadSize == 0) {
- // Either an ACK or an empty settings frame
- output.settingsEmpty((flags & 0x1) != 0);
- } else {
+ if (payloadSize != 0) {
// Process the settings
byte[] setting = new byte[6];
for (int i = 0; i < payloadSize / 6; i++) {
@@ -300,6 +308,7 @@ class Http2Parser {
output.setting(id, value);
}
}
+ output.settingsEnd(ack);
}
@@ -389,7 +398,7 @@ class Http2Parser {
*
* @return <code>true</code> if a valid preface was read, otherwise false.
*/
- boolean readConnectionPreface() {
+ boolean readConnectionPreface() throws IOException {
if (readPreface) {
return true;
}
@@ -414,6 +423,9 @@ class Http2Parser {
}
}
+ // Must always be followed by a settings frame
+ readFrame(true, Integer.valueOf(FRAME_TYPE_SETTINGS));
+
readPreface = true;
return true;
}
@@ -480,8 +492,8 @@ class Http2Parser {
void headersEnd(int streamId);
// Settings frames
- void settingsEmpty(boolean ack);
void setting(int identifier, long value) throws IOException;
+ void settingsEnd(boolean ack) throws IOException;
// Ping frames
void pingReceive(byte[] payload) throws IOException;
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=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
(original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Tue Jun
2 13:44:16 2015
@@ -91,6 +91,7 @@ public class Http2UpgradeHandler extends
private static final byte[] PING_ACK = { 0x00, 0x00, 0x08, 0x06, 0x01,
0x00, 0x00, 0x00, 0x00 };
private static final byte[] SETTINGS_EMPTY = { 0x00, 0x00, 0x00, 0x04,
0x00, 0x00, 0x00, 0x00, 0x00 };
+ private static final byte[] SETTINGS_ACK = { 0x00, 0x00, 0x00, 0x04, 0x01,
0x00, 0x00, 0x00, 0x00 };
private static final byte[] GOAWAY = { 0x07, 0x00, 0x00, 0x00, 0x00 };
@@ -182,10 +183,6 @@ public class Http2UpgradeHandler extends
long value = ByteUtil.getFourBytes(settings, (i * 6) + 2);
remoteSettings.set(id, value);
}
-
- if (!parser.readConnectionPreface()) {
- throw new
ProtocolException(sm.getString("upgradeHandler.invalidPreface"));
- }
} catch (IOException ioe) {
// TODO i18n
throw new ProtocolException();
@@ -196,7 +193,6 @@ public class Http2UpgradeHandler extends
try {
socketWrapper.write(true, SETTINGS_EMPTY, 0,
SETTINGS_EMPTY.length);
socketWrapper.flush(true);
-
} catch (IOException ioe) {
throw new
IllegalStateException(sm.getString("upgradeHandler.sendPrefaceFail"), ioe);
}
@@ -237,14 +233,13 @@ public class Http2UpgradeHandler extends
switch(status) {
case OPEN_READ:
- if (!parser.readConnectionPreface()) {
- close();
- result = SocketState.CLOSED;
- break;
- }
-
- // Process all the incoming data
try {
+ if (!parser.readConnectionPreface()) {
+ close();
+ result = SocketState.CLOSED;
+ break;
+ }
+
while (parser.readFrame(false)) {
}
} catch (Http2Exception h2e) {
@@ -751,16 +746,21 @@ public class Http2UpgradeHandler extends
@Override
- public void settingsEmpty(boolean ack) {
- if (ack) {
- // TODO Process ACK
- }
+ public void setting(int identifier, long value) throws IOException {
+ remoteSettings.set(identifier, value);
}
@Override
- public void setting(int identifier, long value) throws IOException {
- remoteSettings.set(identifier, value);
+ public void settingsEnd(boolean ack) throws IOException {
+ if (ack) {
+ // TODO Process ACK
+ } else {
+ synchronized (socketWrapper) {
+ socketWrapper.write(true, SETTINGS_ACK, 0,
SETTINGS_ACK.length);
+ socketWrapper.flush(true);
+ }
+ }
}
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=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Tue Jun
2 13:44:16 2015
@@ -36,6 +36,7 @@ http2Parser.payloadTooBig=The payload is
http2Parser.preface.invalid=Invalid connection preface [{0}] presented
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.processFrameHeaders.invalidStream=Headers frame received for
stream [0]
http2Parser.processFrameHeaders.decodingFailed=There was an error during the
HPACK decoding of HTTP headers
@@ -59,8 +60,6 @@ streamProcessor.httpupgrade.notsupported
upgradeHandler.connectionError=An error occurred that requires the HTTP/2
connection to be closed.
upgradeHandler.init=Connection [{0}]
upgradeHandler.ioerror=Connection [{0}]
-upgradeHandler.invalidPreface=And invalid connection preface was received from
the client
-upgradeHandler.receivePrefaceNotSettings=The first frame received from the
client was not a settings frame
upgradeHandler.sendPrefaceFail=Failed to send preface to client
upgradeHandler.socketCloseFailed=Error closing socket
upgradeHandler.unexpectedEos=Unexpected end of 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=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http2/Http2TestBase.java Tue Jun 2
13:44:16 2015
@@ -49,11 +49,13 @@ import org.apache.tomcat.util.codec.bina
*/
public abstract class Http2TestBase extends TomcatBaseTest {
- static final String EMPTY_HTTP2_SETTINGS;
+ private static final byte[] EMPTY_SETTINGS_FRAME =
+ { 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00 };
+ static final String EMPTY_HTTP2_SETTINGS_HEADER;
static {
byte[] empty = new byte[0];
- EMPTY_HTTP2_SETTINGS = "HTTP2-Settings: " +
Base64.encodeBase64String(empty) + "\r\n";
+ EMPTY_HTTP2_SETTINGS_HEADER = "HTTP2-Settings: " +
Base64.encodeBase64String(empty) + "\r\n";
}
private Socket s;
@@ -73,12 +75,19 @@ public abstract class Http2TestBase exte
openClientConnection();
doHttpUpgrade();
sendClientPreface();
- // Need to read 3 frames (settings, headers and response body)
+ // - 101 response acts as acknowledgement of the HTTP2-Settings header
+ // Need to read 4 frames
+ // - settings (server settings - must be first)
+ // - settings ack (for the settings frame in the client preface)
+ // - headers (for response)
+ // - data (for response body)
+ parser.readFrame(true);
parser.readFrame(true);
parser.readFrame(true);
parser.readFrame(true);
- Assert.assertEquals("0-Settings-Empty\n" +
+ Assert.assertEquals("0-Settings-End\n" +
+ "0-Settings-Ack\n" +
"1-HeadersStart\n" +
"1-Header-[:status]-[200]\n" +
"1-HeadersEnd\n" +
@@ -125,7 +134,7 @@ public abstract class Http2TestBase exte
protected void doHttpUpgrade() throws IOException {
- doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS, true);
+ doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS_HEADER, true);
}
protected void doHttpUpgrade(String upgrade, String settings, boolean
validate)
@@ -234,6 +243,7 @@ public abstract class Http2TestBase exte
private void sendClientPreface() throws IOException {
os.write(Http2Parser.CLIENT_PREFACE_START);
+ os.write(EMPTY_SETTINGS_FRAME);
os.flush();
}
@@ -322,19 +332,19 @@ public abstract class Http2TestBase exte
@Override
- public void settingsEmpty(boolean ack) {
- if (ack) {
- trace.append("0-Settings-Ack\n");
- } else {
- trace.append("0-Settings-Empty\n");
- }
+ public void setting(int identifier, long value) throws IOException {
+ trace.append("0-Settings-[" + identifier + "]-[" + value + "]\n");
+ remoteSettings.set(identifier, value);
}
@Override
- public void setting(int identifier, long value) throws IOException {
- trace.append("0-Settings-[" + identifier + "]-[" + value + "]\n");
- remoteSettings.set(identifier, value);
+ public void settingsEnd(boolean ack) {
+ if (ack) {
+ trace.append("0-Settings-Ack\n");
+ } else {
+ trace.append("0-Settings-End\n");
+ }
}
Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java?rev=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java
(original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2.java Tue Jun
2 13:44:16 2015
@@ -39,7 +39,7 @@ public class TestHttp2Section_3_2 extend
public void testConnectionNoHttp2Support() throws Exception {
configureAndStartWebApplication();
openClientConnection();
- doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS, false);
+ doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS_HEADER, false);
parseHttp11Response();
}
@@ -49,7 +49,7 @@ public class TestHttp2Section_3_2 extend
enableHttp2();
configureAndStartWebApplication();
openClientConnection();
- doHttpUpgrade("h2", EMPTY_HTTP2_SETTINGS, false);
+ doHttpUpgrade("h2", EMPTY_HTTP2_SETTINGS_HEADER, false);
parseHttp11Response();
}
Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java?rev=1683114&r1=1683113&r2=1683114&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java
(original)
+++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java Tue
Jun 2 13:44:16 2015
@@ -35,8 +35,8 @@ public class TestHttp2Section_3_2_1 exte
enableHttp2();
configureAndStartWebApplication();
openClientConnection();
- doHttpUpgrade("h2c", Http2TestBase.EMPTY_HTTP2_SETTINGS +
Http2TestBase.EMPTY_HTTP2_SETTINGS,
- false);
+ doHttpUpgrade("h2c", Http2TestBase.EMPTY_HTTP2_SETTINGS_HEADER +
+ Http2TestBase.EMPTY_HTTP2_SETTINGS_HEADER, false);
parseHttp11Response();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
