Skip to content

Commit

Permalink
Refactor HttpTest to run the tests with Http2 when it makes sense + f…
Browse files Browse the repository at this point in the history 
…ix related bugs in http2
  • Loading branch information
@vietj
vietj committed Mar 22, 2016
1 parent a6b334d commit 2a2bce3
Show file tree
Hide file tree
Showing 12 changed files with 3,528 additions and 3,426 deletions.
1 change: 0 additions & 1 deletion src/main/java/io/vertx/core/http/HttpConnection.java
View file
Expand Up @@ -35,7 +35,6 @@
* - add rawMethod
* - byte distribution algorithm configurability (options ? connection ?)
* - netSocket() interaction are not exactly the same than with http/1.x : see if we can make is the same
* - run HttpTest with HTTP2
* - handle data frame padding
* - documentation
* - examples
Expand Down
78 changes: 42 additions & 36 deletions src/main/java/io/vertx/core/http/impl/Http2ClientConnection.java
View file
Expand Up @@ -137,17 +137,18 @@ public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promi

static class Http2ClientStream extends VertxHttp2Stream<Http2ClientConnection> implements HttpClientStream {

private HttpClientRequestBase req;
private HttpClientResponseImpl resp;
private boolean ended;
private HttpClientRequestBase request;
private HttpClientResponseImpl response;
private boolean requestEnded;
private boolean responseEnded;

public Http2ClientStream(Http2ClientConnection conn, Http2Stream stream) throws Http2Exception {
this(conn, null, stream);
}

public Http2ClientStream(Http2ClientConnection conn, HttpClientRequestBase req, Http2Stream stream) throws Http2Exception {
public Http2ClientStream(Http2ClientConnection conn, HttpClientRequestBase request, Http2Stream stream) throws Http2Exception {
super(conn, stream);
this.req = req;
this.request = request;
}

@Override
Expand All @@ -158,104 +159,104 @@ public HttpVersion version() {
@Override
void callEnd() {
if (conn.metrics.isEnabled()) {
HttpClientRequestBase req = this.req;
if (req.exceptionOccurred) {
conn.metrics.requestReset(req.metric());
if (request.exceptionOccurred) {
conn.metrics.requestReset(request.metric());
} else {
conn.metrics.responseEnd(req.metric(), resp);
conn.metrics.responseEnd(request.metric(), response);
}
}
ended = true;
responseEnded = true;
// Should use a shared immutable object for CaseInsensitiveHeaders ?
resp.handleEnd(null, new CaseInsensitiveHeaders());
response.handleEnd(null, new CaseInsensitiveHeaders());
}

@Override
void callHandler(Buffer buf) {
resp.handleChunk(buf);
response.handleChunk(buf);
}

@Override
void callReset(long errorCode) {
if (!ended) {
ended = true;
if (!responseEnded) {
responseEnded = true;
if (conn.metrics.isEnabled()) {
HttpClientRequestBase req = this.req;
conn.metrics.requestReset(req.metric());
conn.metrics.requestReset(request.metric());
}
handleException(new StreamResetException(errorCode));
}
}

@Override
void handleClose() {
if (!ended) {
ended = true;
if (!responseEnded) {
responseEnded = true;
if (conn.metrics.isEnabled()) {
HttpClientRequestBase req = this.req;
conn.metrics.requestReset(req.metric());
conn.metrics.requestReset(request.metric());
}
handleException(new VertxException("Connection was closed")); // Put that in utility class
}
}

@Override
public void handleInterestedOpsChanged() {
if (req instanceof HttpClientRequestImpl && !isNotWritable()) {
if (request instanceof HttpClientRequestImpl && !isNotWritable()) {
if (!isNotWritable()) {
((HttpClientRequestImpl)req).handleDrained();
((HttpClientRequestImpl) request).handleDrained();
}
}
}

@Override
void handleUnknownFrame(int type, int flags, Buffer buff) {
resp.handleUnknowFrame(new HttpFrameImpl(type, flags, buff));
response.handleUnknowFrame(new HttpFrameImpl(type, flags, buff));
}

void handleHeaders(Http2Headers headers, boolean end) {
if (resp == null || resp.statusCode() == 100) {
if (response == null || response.statusCode() == 100) {
int status;
String statusMessage;
try {
status = Integer.parseInt(headers.status().toString());
statusMessage = HttpResponseStatus.valueOf(status).reasonPhrase();
} catch (Exception e) {
e.printStackTrace();
handleException(e);
encoder.writeRstStream(handlerContext, stream.id(), 0x01 /* PROTOCOL_ERROR */, handlerContext.newPromise());
channel.flush();
return;
}
resp = new HttpClientResponseImpl(
req,
response = new HttpClientResponseImpl(
request,
HttpVersion.HTTP_2,
this,
status,
statusMessage,
new Http2HeadersAdaptor(headers)
);
req.handleResponse(resp);
request.handleResponse(response);
if (end) {
handleEnd();
}
} else if (end) {
resp.handleEnd(null, new Http2HeadersAdaptor(headers));
response.handleEnd(null, new Http2HeadersAdaptor(headers));
}
}

void handleException(Throwable exception) {
context.executeFromIO(() -> {
req.handleException(exception);
});
if (resp != null) {
if (!requestEnded || response == null) {
context.executeFromIO(() -> {
resp.handleException(exception);
request.handleException(exception);
});
}
if (response != null) {
context.executeFromIO(() -> {
response.handleException(exception);
});
}
}

void handlePushPromise(HttpClientRequestBase promised) throws Http2Exception {
((HttpClientRequestImpl)req).handlePush(promised);
((HttpClientRequestImpl) request).handlePush(promised);
}

@Override
Expand Down Expand Up @@ -291,7 +292,7 @@ public void writeHeadWithContent(HttpMethod method, String uri, MultiMap headers
h.set(HttpHeaderNames.ACCEPT_ENCODING, DEFLATE_GZIP);
}
if (conn.metrics.isEnabled()) {
((HttpClientRequestImpl)req).metric(conn.metrics.requestBegin(conn.metric, conn.localAddress(), conn.remoteAddress(), req));
request.metric(conn.metrics.requestBegin(conn.metric, conn.localAddress(), conn.remoteAddress(), request));
}
encoder.writeHeaders(handlerContext, stream.id(), h, 0, end && content == null, handlerContext.newPromise());
if (content != null) {
Expand Down Expand Up @@ -323,17 +324,22 @@ public Context getContext() {
@Override
public void doSetWriteQueueMaxSize(int size) {
}

@Override
public boolean isNotWritable() {
return !conn.handler.connection().remote().flowController().isWritable(stream);
}

@Override
public void beginRequest(HttpClientRequestImpl request) {
req = request;
this.request = request;
}

@Override
public void endRequest() {
requestEnded = true;
}

@Override
public void reset(long code) {
encoder.writeRstStream(handlerContext, stream.id(), code, handlerContext.newPromise());
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/vertx/core/http/impl/Http2ServerRequestImpl.java
View file
Expand Up @@ -431,12 +431,12 @@ public MultiMap formAttributes() {

@Override
public ServerWebSocket upgrade() {
throw new UnsupportedOperationException();
throw new UnsupportedOperationException("HTTP/2 request cannot be upgraded to a websocket");
}

@Override
public boolean isEnded() {
throw new UnsupportedOperationException();
return ended;
}

@Override
Expand Down
20 changes: 13 additions & 7 deletions src/main/java/io/vertx/core/http/impl/Http2ServerResponseImpl.java
View file
Expand Up @@ -61,7 +61,7 @@ public class Http2ServerResponseImpl implements HttpServerResponse {
private final boolean push;
private final Object metric;
private final String host;
private Http2Headers headers = new DefaultHttp2Headers().status(OK.codeAsText());
private Http2Headers headers = new DefaultHttp2Headers();
private Http2HeadersAdaptor headersMap;
private Http2Headers trailers;
private Http2HeadersAdaptor trailedMap;
Expand Down Expand Up @@ -162,7 +162,6 @@ public HttpServerResponse setStatusCode(int statusCode) {
}
checkHeadWritten();
this.statusCode = statusCode;
headers.status("" + statusCode);
return this;
}

Expand Down Expand Up @@ -359,6 +358,7 @@ private boolean checkSendHeaders(boolean end) {
headersEndHandler.handle(null);
}
headWritten = true;
headers.status(Integer.toString(statusCode));
encoder.writeHeaders(ctx, stream.id(), headers, 0, end, ctx.newPromise());
if (end) {
ctx.flush();
Expand All @@ -376,12 +376,12 @@ void write(ByteBuf chunk, boolean end) {
}
int len = chunk.readableBytes();
boolean empty = len == 0;
boolean sent = checkSendHeaders(empty && end);
boolean sent = checkSendHeaders(end && empty && trailers == null);
if (!empty || (!sent && end)) {
stream_.writeData(chunk, end && trailers == null);
bytesWritten += len;
}
if (trailers != null && end) {
if (end && trailers != null) {
encoder.writeHeaders(ctx, stream.id(), trailers, 0, true, ctx.newPromise());
}
if (end && bodyEndHandler != null) {
Expand Down Expand Up @@ -482,8 +482,14 @@ public HttpServerResponse sendFile(String filename, long offset, long length, Ha
}

long contentLength = Math.min(length, file.length() - offset);
if (headers.get("content-length") == null) {
putHeader("content-length", String.valueOf(contentLength));
if (headers.get(HttpHeaderNames.CONTENT_LENGTH) == null) {
putHeader(HttpHeaderNames.CONTENT_LENGTH, String.valueOf(contentLength));
}
if (headers.get(HttpHeaderNames.CONTENT_TYPE) == null) {
String contentType = MimeMapping.getMimeTypeForFilename(filename);
if (contentType != null) {
putHeader(HttpHeaderNames.CONTENT_TYPE, contentType);
}
}
checkSendHeaders(false);

Expand All @@ -507,7 +513,7 @@ public HttpServerResponse sendFile(String filename, long offset, long length, Ha

@Override
public void close() {
throw new UnsupportedOperationException();
connection.close();
}

@Override
Expand Down
10 changes: 3 additions & 7 deletions src/main/java/io/vertx/core/http/impl/HttpServerResponseImpl.java
View file
Expand Up @@ -454,13 +454,9 @@ private void doSendFile(String filename, long offset, long length, Handler<Async
putHeader(HttpHeaders.CONTENT_LENGTH, String.valueOf(contentLength));
}
if (!contentTypeSet()) {
int li = filename.lastIndexOf('.');
if (li != -1 && li != filename.length() - 1) {
String ext = filename.substring(li + 1, filename.length());
String contentType = MimeMapping.getMimeTypeForExtension(ext);
if (contentType != null) {
putHeader(HttpHeaders.CONTENT_TYPE, contentType);
}
String contentType = MimeMapping.getMimeTypeForFilename(filename);
if (contentType != null) {
putHeader(HttpHeaders.CONTENT_TYPE, contentType);
}
}
prepareHeaders();
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/io/vertx/core/http/impl/MimeMapping.java
View file
Expand Up @@ -1015,4 +1015,12 @@ public class MimeMapping {
public static String getMimeTypeForExtension(String ext) {
return m.get(ext);
}
public static String getMimeTypeForFilename(String filename) {
int li = filename.lastIndexOf('.');
if (li != -1 && li != filename.length() - 1) {
String ext = filename.substring(li + 1, filename.length());
return MimeMapping.getMimeTypeForExtension(ext);
}
return null;
}
}
2 changes: 1 addition & 1 deletion src/test/java/io/vertx/test/core/ClusteredEventBusWithSSLTest.java
View file
Expand Up @@ -78,7 +78,7 @@ public static Iterable<Object[]> data() {
{KeyCert.PEM, Trust.JKS, false, false, false, Collections.emptyList()},
{KeyCert.PKCS12_CA, Trust.JKS_CA, false, false, false, Collections.emptyList()},
{KeyCert.PEM_CA, Trust.PKCS12_CA, false, false, false, Collections.emptyList()},
{KeyCert.JKS, Trust.PEM_CA, false, true, false, Arrays.asList(HttpTest.ENABLED_CIPHER_SUITES)},
{KeyCert.JKS, Trust.PEM_CA, false, true, false, Arrays.asList(Http1xTest.ENABLED_CIPHER_SUITES)},
});
}

Expand Down

0 comments on commit 2a2bce3

Please sign in to comment.