Skip to content

Commit

Permalink
HTTP/2 HelloWorld Client Example Bug
Browse files Browse the repository at this point in the history
Motivation:
The HTTP/2 helloworld client example has 2 bugs:
1. HttpResponseHandler has a map which is accessed from multiple threads, but the map is not thread safe.
2. Requests are flushed and maybe completely written and the responses may be received/processed by Netty before an element is inserted into the HttpResponseHandler map. This may result in an 'unexpected message' error even though the message has actually been sent.

Modifications:
- HttpResponseHandler should use a thread safe map
- Http2Client shouldn't flush until entries are added to the HttpResponseHandler map

Result:
Fixes netty#6165.
  • Loading branch information
Scottmitch authored and liuzhengyang committed Sep 10, 2017
1 parent 317a38d commit 273c076
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package io.netty.example.http2.helloworld.client;

import io.netty.bootstrap.Bootstrap;
import io.netty.buffer.Unpooled;
import io.netty.channel.Channel;
import io.netty.channel.ChannelOption;
import io.netty.channel.EventLoopGroup;
Expand Down Expand Up @@ -44,6 +43,7 @@

import java.util.concurrent.TimeUnit;

import static io.netty.buffer.Unpooled.wrappedBuffer;
import static io.netty.handler.codec.http.HttpMethod.GET;
import static io.netty.handler.codec.http.HttpMethod.POST;
import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
Expand Down Expand Up @@ -118,20 +118,20 @@ public static void main(String[] args) throws Exception {
request.headers().add(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), scheme.name());
request.headers().add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.GZIP);
request.headers().add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.DEFLATE);
responseHandler.put(streamId, channel.writeAndFlush(request), channel.newPromise());
responseHandler.put(streamId, channel.write(request), channel.newPromise());
streamId += 2;
}
if (URL2 != null) {
// Create a simple POST request with a body.
FullHttpRequest request = new DefaultFullHttpRequest(HTTP_1_1, POST, URL2,
Unpooled.copiedBuffer(URL2DATA.getBytes(CharsetUtil.UTF_8)));
wrappedBuffer(URL2DATA.getBytes(CharsetUtil.UTF_8)));
request.headers().add(HttpHeaderNames.HOST, hostName);
request.headers().add(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), scheme.name());
request.headers().add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.GZIP);
request.headers().add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.DEFLATE);
responseHandler.put(streamId, channel.writeAndFlush(request), channel.newPromise());
streamId += 2;
responseHandler.put(streamId, channel.write(request), channel.newPromise());
}
channel.flush();
responseHandler.awaitResponses(5, TimeUnit.SECONDS);
System.out.println("Finished HTTP/2 request(s)");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,25 @@
import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.handler.codec.http2.HttpConversionUtil;
import io.netty.util.CharsetUtil;
import io.netty.util.internal.PlatformDependent;

import java.util.AbstractMap.SimpleEntry;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.concurrent.TimeUnit;

/**
* Process {@link io.netty.handler.codec.http.FullHttpResponse} translated from HTTP/2 frames
*/
public class HttpResponseHandler extends SimpleChannelInboundHandler<FullHttpResponse> {

private SortedMap<Integer, Entry<ChannelFuture, ChannelPromise>> streamidPromiseMap;
private Map<Integer, Entry<ChannelFuture, ChannelPromise>> streamidPromiseMap;

public HttpResponseHandler() {
streamidPromiseMap = new TreeMap<Integer, Entry<ChannelFuture, ChannelPromise>>();
// Use a concurrent map because we add and iterate from the main thread (just for the purposes of the example),
// but Netty also does a get on the map when messages are received in a EventLoop thread.
streamidPromiseMap = PlatformDependent.newConcurrentHashMap();
}

/**
Expand Down

0 comments on commit 273c076

Please sign in to comment.