Skip to content

Commit

Permalink
Expand testing of section 3.2.1
Browse files Browse the repository at this point in the history
 - Refactor Http2Parser.Output for empty settings to enable explicit testing
   for empty settings frames from the server
 - Refactor tests to reduce code required for 'standard' values and to expand
   options for adding non-standard values
 - Add a test for zero HTTP2-Settings headers
 

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1683085 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
markt-asf committed Jun 2, 2015
1 parent 73c2c57 commit 25fc390
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 42 deletions.
6 changes: 2 additions & 4 deletions java/org/apache/coyote/http2/Http2Parser.java
Expand Up @@ -289,9 +289,7 @@ private void readSettingsFrame(int streamId, int flags, int payloadSize) throws

if (payloadSize == 0) {
// Either an ACK or an empty settings frame
if ((flags & 0x1) != 0) {
output.settingsAck();
}
output.settingsEmpty((flags & 0x1) != 0);
} else {
// Process the settings
byte[] setting = new byte[6];
Expand Down Expand Up @@ -482,7 +480,7 @@ static interface Output {
void headersEnd(int streamId);

// Settings frames
void settingsAck();
void settingsEmpty(boolean ack);
void setting(int identifier, long value) throws IOException;

// Ping frames
Expand Down
6 changes: 4 additions & 2 deletions java/org/apache/coyote/http2/Http2UpgradeHandler.java
Expand Up @@ -751,8 +751,10 @@ public void headersEnd(int streamId) {


@Override
public void settingsAck() {
// TODO Auto-generated method stub
public void settingsEmpty(boolean ack) {
if (ack) {
// TODO Process ACK
}
}


Expand Down
51 changes: 42 additions & 9 deletions test/org/apache/coyote/http2/Http2TestBase.java
Expand Up @@ -22,6 +22,7 @@
import java.net.Socket;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Locale;

import javax.net.SocketFactory;
import javax.servlet.ServletException;
Expand All @@ -48,11 +49,11 @@
*/
public abstract class Http2TestBase extends TomcatBaseTest {

private static final String HTTP2_SETTINGS;
static final String EMPTY_HTTP2_SETTINGS;

static {
byte[] empty = new byte[0];
HTTP2_SETTINGS = Base64.encodeBase64String(empty);
EMPTY_HTTP2_SETTINGS = "HTTP2-Settings: " + Base64.encodeBase64String(empty) + "\r\n";
}

private Socket s;
Expand All @@ -70,14 +71,15 @@ protected void http2Connect() throws Exception {
enableHttp2();
configureAndStartWebApplication();
openClientConnection();
doHttpUpgrade("h2c", true);
doHttpUpgrade();
sendClientPreface();
// Need to read 3 frames (settings, headers and response body)
parser.readFrame(true);
parser.readFrame(true);
parser.readFrame(true);

Assert.assertEquals("1-HeadersStart\n" +
Assert.assertEquals("0-Settings-Empty\n" +
"1-HeadersStart\n" +
"1-Header-[:status]-[200]\n" +
"1-HeadersEnd\n" +
"1-Body-8192\n" +
Expand Down Expand Up @@ -122,12 +124,17 @@ protected void openClientConnection() throws IOException {
}


protected void doHttpUpgrade(String upgrade, boolean validate) throws IOException {
protected void doHttpUpgrade() throws IOException {
doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS, true);
}

protected void doHttpUpgrade(String upgrade, String settings, boolean validate)
throws IOException {
byte[] upgradeRequest = ("GET / HTTP/1.1\r\n" +
"Host: localhost:" + getPort() + "\r\n" +
"Connection: Upgrade, HTTP2-Settings\r\n" +
"Upgrade: " + upgrade + "\r\n" +
"HTTP2-Settings: "+ HTTP2_SETTINGS + "\r\n" +
settings +
"\r\n").getBytes(StandardCharsets.ISO_8859_1);
os.write(upgradeRequest);
os.flush();
Expand Down Expand Up @@ -203,6 +210,28 @@ String[] readHttpResponseHeaders() throws IOException {
}


void parseHttp11Response() throws IOException {
String[] responseHeaders = readHttpResponseHeaders();
Assert.assertTrue(responseHeaders[0], responseHeaders[0].startsWith("HTTP/1.1 200"));

// Find the content length (chunked responses not handled)
for (int i = 1; i < responseHeaders.length; i++) {
if (responseHeaders[i].toLowerCase(Locale.ENGLISH).startsWith("content-length")) {
String cl = responseHeaders[i];
int pos = cl.indexOf(':');
if (pos == -1) {
throw new IOException("Invalid: [" + cl + "]");
}
int len = Integer.parseInt(cl.substring(pos + 1).trim());
byte[] content = new byte[len];
input.fill(true, content);
return;
}
}
Assert.fail("No content-length in response");
}


private void sendClientPreface() throws IOException {
os.write(Http2Parser.CLIENT_PREFACE_START);
os.flush();
Expand Down Expand Up @@ -293,11 +322,15 @@ public void headersEnd(int streamId) {


@Override
public void settingsAck() {
trace.append("0-Settings-Ack\n");

public void settingsEmpty(boolean ack) {
if (ack) {
trace.append("0-Settings-Ack\n");
} else {
trace.append("0-Settings-Empty\n");
}
}


@Override
public void setting(int identifier, long value) throws IOException {
trace.append("0-Settings-[" + identifier + "]-[" + value + "]\n");
Expand Down
30 changes: 3 additions & 27 deletions test/org/apache/coyote/http2/TestHttp2Section_3_2.java
Expand Up @@ -18,9 +18,7 @@

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Locale;

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

/**
Expand All @@ -41,7 +39,7 @@ public class TestHttp2Section_3_2 extends Http2TestBase {
public void testConnectionNoHttp2Support() throws Exception {
configureAndStartWebApplication();
openClientConnection();
doHttpUpgrade("h2c", false);
doHttpUpgrade("h2c", EMPTY_HTTP2_SETTINGS, false);
parseHttp11Response();
}

Expand All @@ -51,7 +49,7 @@ public void testConnectionUpgradeWrongProtocol() throws Exception {
enableHttp2();
configureAndStartWebApplication();
openClientConnection();
doHttpUpgrade("h2", false);
doHttpUpgrade("h2", EMPTY_HTTP2_SETTINGS, false);
parseHttp11Response();
}

Expand Down Expand Up @@ -115,28 +113,6 @@ private void setupAsFarAsUpgrade() throws Exception {
enableHttp2();
configureAndStartWebApplication();
openClientConnection();
doHttpUpgrade("h2c", true);
}


private void parseHttp11Response() throws IOException {
String[] responseHeaders = readHttpResponseHeaders();
Assert.assertTrue(responseHeaders[0], responseHeaders[0].startsWith("HTTP/1.1 200"));

// Find the content length (chunked responses not handled)
for (int i = 1; i < responseHeaders.length; i++) {
if (responseHeaders[i].toLowerCase(Locale.ENGLISH).startsWith("content-length")) {
String cl = responseHeaders[i];
int pos = cl.indexOf(':');
if (pos == -1) {
throw new IOException("Invalid: [" + cl + "]");
}
int len = Integer.parseInt(cl.substring(pos + 1).trim());
byte[] content = new byte[len];
input.fill(true, content);
return;
}
}
Assert.fail("No content-length in response");
doHttpUpgrade();
}
}
12 changes: 12 additions & 0 deletions test/org/apache/coyote/http2/TestHttp2Section_3_2_1.java
Expand Up @@ -16,8 +16,20 @@
*/
package org.apache.coyote.http2;

import org.junit.Test;

public class TestHttp2Section_3_2_1 extends Http2TestBase {

@Test
public void testZeroHttp2Settings() throws Exception{
enableHttp2();
configureAndStartWebApplication();
openClientConnection();
doHttpUpgrade("h2", "", false);
parseHttp11Response();
}


// TODO: Test zero Http2-Settings headers

// TODO: Test multiple Http2-Settings headers
Expand Down

0 comments on commit 25fc390

Please sign in to comment.