Do not expose HTTP/2 pseudo headers in header maps - fixes #2941

This commit is contained in:
Julien Viet
2019-05-11 07:51:38 +02:00
parent af350f60e3
commit ab52200505
5 changed files with 75 additions and 96 deletions

View File

@@ -325,6 +325,9 @@ class Http2ClientConnection extends Http2ConnectionBase implements HttpClientCon
writeReset(0x01 /* PROTOCOL_ERROR */);
return;
}
headers.remove(":status");
response = new HttpClientResponseImpl(
request,
HttpVersion.HTTP_2,

View File

@@ -59,15 +59,16 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
private final String serverOrigin;
private final Http2ServerResponseImpl response;
private final Http2Headers headers;
private MultiMap headersMap;
private MultiMap params;
private HttpMethod method;
private String rawMethod;
private String absoluteURI;
private String uri;
private final MultiMap headersMap;
private final String rawMethod;
private final String scheme;
private final String host;
private final String uri;
private String path;
private String query;
private HttpMethod method;
private MultiMap params;
private String absoluteURI;
private MultiMap attributes;
private Handler<Buffer> dataHandler;
@@ -89,8 +90,18 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
super(conn, stream, writable);
this.serverOrigin = serverOrigin;
this.headers = headers;
this.streamEnded = streamEnded;
this.uri = headers.get(":path") != null ? headers.get(":path").toString() : null;
this.scheme = headers.get(":scheme") != null ? headers.get(":scheme").toString() : null;
this.rawMethod = headers.get(":method") != null ? headers.get(":method").toString() : null;
this.host = headers.get(":authority") != null ? headers.get(":authority").toString() : null;
headers.remove(":method");
headers.remove(":scheme");
headers.remove(":path");
headers.remove(":authority");
headersMap = new Http2HeadersAdaptor(headers);
String host = host();
if (host == null) {
@@ -287,8 +298,7 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
public HttpMethod method() {
synchronized (conn) {
if (method == null) {
String sMethod = headers.method().toString();
method = HttpUtils.toVertxMethod(sMethod);
method = HttpUtils.toVertxMethod(rawMethod);
}
return method;
}
@@ -296,12 +306,7 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
@Override
public String rawMethod() {
synchronized (conn) {
if (rawMethod == null) {
rawMethod = headers.method().toString();
}
return rawMethod;
}
return rawMethod;
}
@Override
@@ -311,26 +316,13 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
@Override
public String uri() {
synchronized (conn) {
if (uri == null) {
CharSequence path = headers.path();
if (path != null) {
uri = path.toString();
}
}
return uri;
}
return uri;
}
@Override
public String path() {
synchronized (conn) {
if (path == null) {
CharSequence path = headers.path();
if (path != null) {
this.path = HttpUtils.parsePath(path.toString());
}
}
this.path = uri != null ? HttpUtils.parsePath(uri) : null;
return path;
}
}
@@ -338,26 +330,19 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
@Override
public String query() {
synchronized (conn) {
if (query == null) {
CharSequence path = headers.path();
if (path != null) {
this.query = HttpUtils.parseQuery(path.toString());
}
}
this.query = uri != null ? HttpUtils.parseQuery(uri) : null;
return query;
}
}
@Override
public String scheme() {
CharSequence scheme = headers.scheme();
return scheme != null ? scheme.toString() : null;
return scheme;
}
@Override
public String host() {
CharSequence authority = headers.authority();
return authority != null ? authority.toString() : null;
return host;
}
@Override
@@ -375,12 +360,7 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
@Override
public MultiMap headers() {
synchronized (conn) {
if (headersMap == null) {
headersMap = new Http2HeadersAdaptor(headers);
}
return headersMap;
}
return headersMap;
}
@Override
@@ -456,9 +436,9 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
checkEnded();
if (expect) {
if (postRequestDecoder == null) {
CharSequence contentType = headers.get(HttpHeaderNames.CONTENT_TYPE);
String contentType = headersMap.get(HttpHeaderNames.CONTENT_TYPE);
if (contentType != null) {
io.netty.handler.codec.http.HttpMethod method = io.netty.handler.codec.http.HttpMethod.valueOf(headers.method().toString());
io.netty.handler.codec.http.HttpMethod method = io.netty.handler.codec.http.HttpMethod.valueOf(rawMethod);
String lowerCaseContentType = contentType.toString().toLowerCase();
boolean isURLEncoded = lowerCaseContentType.startsWith(HttpHeaderValues.APPLICATION_X_WWW_FORM_URLENCODED.toString());
if ((lowerCaseContentType.startsWith(HttpHeaderValues.MULTIPART_FORM_DATA.toString()) || isURLEncoded) &&
@@ -469,7 +449,7 @@ public class Http2ServerRequestImpl extends VertxHttp2Stream<Http2ServerConnecti
HttpRequest req = new DefaultHttpRequest(
io.netty.handler.codec.http.HttpVersion.HTTP_1_1,
method,
headers.path().toString());
uri);
req.headers().add(HttpHeaderNames.CONTENT_TYPE, contentType);
postRequestDecoder = new HttpPostRequestDecoder(new NettyFileUploadDataFactory(context, this, () -> uploadHandler), req);
}

View File

@@ -256,6 +256,7 @@ public class Http2ClientTest extends Http2TestBase {
assertEquals(2, req.headers().getAll("juu_request").size());
assertEquals("juu_request_value_1", req.headers().getAll("juu_request").get(0));
assertEquals("juu_request_value_2", req.headers().getAll("juu_request").get(1));
assertEquals(new HashSet<>(Arrays.asList("foo_request", "bar_request", "juu_request")), new HashSet<>(req.headers().names()));
reqCount.incrementAndGet();
HttpServerResponse resp = req.response();
resp.putHeader("content-type", "text/plain");
@@ -265,28 +266,27 @@ public class Http2ClientTest extends Http2TestBase {
resp.end();
});
startServer();
HttpClientRequest req = client.get(DEFAULT_HTTPS_PORT, DEFAULT_HTTPS_HOST, "/somepath");
req.handler(resp -> {
Context ctx = vertx.getOrCreateContext();
assertOnIOContext(ctx);
assertEquals(1, req.streamId());
assertEquals(1, reqCount.get());
assertEquals(HttpVersion.HTTP_2, resp.version());
assertEquals(200, resp.statusCode());
assertEquals("OK", resp.statusMessage());
assertEquals("text/plain", resp.getHeader("content-type"));
assertEquals("200", resp.getHeader(":status"));
assertEquals("foo_value", resp.getHeader("foo_response"));
assertEquals("bar_value", resp.getHeader("bar_response"));
assertEquals(2, resp.headers().getAll("juu_response").size());
assertEquals("juu_value_1", resp.headers().getAll("juu_response").get(0));
assertEquals("juu_value_2", resp.headers().getAll("juu_response").get(1));
resp.endHandler(v -> {
client.get(DEFAULT_HTTPS_PORT, DEFAULT_HTTPS_HOST, "/somepath",
resp -> {
Context ctx = vertx.getOrCreateContext();
assertOnIOContext(ctx);
testComplete();
});
})
.putHeader("Foo_request", "foo_request_value")
assertEquals(1, resp.request().streamId());
assertEquals(1, reqCount.get());
assertEquals(HttpVersion.HTTP_2, resp.version());
assertEquals(200, resp.statusCode());
assertEquals("OK", resp.statusMessage());
assertEquals("text/plain", resp.getHeader("content-type"));
assertEquals("foo_value", resp.getHeader("foo_response"));
assertEquals("bar_value", resp.getHeader("bar_response"));
assertEquals(2, resp.headers().getAll("juu_response").size());
assertEquals("juu_value_1", resp.headers().getAll("juu_response").get(0));
assertEquals("juu_value_2", resp.headers().getAll("juu_response").get(1));
assertEquals(new HashSet<>(Arrays.asList("content-type", "content-length", "foo_response", "bar_response", "juu_response")), new HashSet<>(resp.headers().names()));
resp.endHandler(v -> {
assertOnIOContext(ctx);
testComplete();
});
}).putHeader("Foo_request", "foo_request_value")
.putHeader("bar_request", "bar_request_value")
.putHeader("juu_request", Arrays.<CharSequence>asList("juu_request_value_1", "juu_request_value_2"))
.exceptionHandler(err -> fail())

View File

@@ -437,11 +437,9 @@ public class Http2ServerTest extends Http2TestBase {
assertEquals(HttpMethod.GET, req.method());
assertEquals(DEFAULT_HTTPS_HOST_AND_PORT, req.host());
assertEquals("/", req.path());
assertEquals(DEFAULT_HTTPS_HOST_AND_PORT, req.getHeader(":authority"));
assertTrue(req.isSSL());
assertEquals("https", req.getHeader(":scheme"));
assertEquals("/", req.getHeader(":path"));
assertEquals("GET", req.getHeader(":method"));
assertEquals("https", req.scheme());
assertEquals("/", req.uri());
assertEquals("foo_request_value", req.getHeader("Foo_request"));
assertEquals("bar_request_value", req.getHeader("bar_request"));
assertEquals(2, req.headers().getAll("juu_request").size());
@@ -525,7 +523,6 @@ public class Http2ServerTest extends Http2TestBase {
assertEquals("foo=foo_value&bar=bar_value_1&bar=bar_value_2", req.query());
assertEquals("/some/path?foo=foo_value&bar=bar_value_1&bar=bar_value_2", req.uri());
assertEquals("http://whatever.com/some/path?foo=foo_value&bar=bar_value_1&bar=bar_value_2", req.absoluteURI());
assertEquals("/some/path?foo=foo_value&bar=bar_value_1&bar=bar_value_2", req.getHeader(":path"));
assertEquals("whatever.com", req.host());
MultiMap params = req.params();
Set<String> names = params.names();
@@ -2679,7 +2676,6 @@ public class Http2ServerTest extends Http2TestBase {
assertEquals(HttpVersion.HTTP_2, resp.version());
complete();
}).exceptionHandler(this::fail).pushHandler(pushedReq -> {
assertEquals("http", pushedReq.headers().get(":scheme"));
pushedReq.handler(pushResp -> {
pushResp.bodyHandler(buff -> {
assertEquals("the-pushed-response", buff.toString());

View File

@@ -786,11 +786,7 @@ public abstract class HttpTest extends HttpTestBase {
assertEquals(1, req.headers().size());
assertEquals("localhost:" + DEFAULT_HTTP_PORT, req.headers().get("host"));
} else {
assertEquals(4, req.headers().size());
assertEquals("https", req.headers().get(":scheme"));
assertEquals("GET", req.headers().get(":method"));
assertEquals("some-uri", req.headers().get(":path"));
assertEquals("localhost:" + DEFAULT_HTTP_PORT, req.headers().get(":authority"));
assertEquals(0, req.headers().size());
}
req.response().end();
});
@@ -804,17 +800,20 @@ public abstract class HttpTest extends HttpTestBase {
@Test
public void testRequestHeadersWithCharSequence() {
HashMap<CharSequence, String> headers = new HashMap<>();
headers.put(HttpHeaders.TEXT_HTML, "text/html");
headers.put(HttpHeaders.USER_AGENT, "User-Agent");
headers.put(HttpHeaders.APPLICATION_X_WWW_FORM_URLENCODED, "application/x-www-form-urlencoded");
HashMap<CharSequence, String> expectedHeaders = new HashMap<>();
expectedHeaders.put(HttpHeaders.TEXT_HTML, "text/html");
expectedHeaders.put(HttpHeaders.USER_AGENT, "User-Agent");
expectedHeaders.put(HttpHeaders.APPLICATION_X_WWW_FORM_URLENCODED, "application/x-www-form-urlencoded");
server.requestHandler(req -> {
assertTrue(headers.size() < req.headers().size());
MultiMap headers = req.headers();
headers.remove("host");
headers.forEach((k, v) -> assertEquals(v, req.headers().get(k)));
headers.forEach((k, v) -> assertEquals(v, req.getHeader(k)));
assertEquals(expectedHeaders.size(), headers.size());
expectedHeaders.forEach((k, v) -> assertEquals(v, headers.get(k)));
expectedHeaders.forEach((k, v) -> assertEquals(v, req.getHeader(k)));
req.response().end();
});
@@ -822,7 +821,7 @@ public abstract class HttpTest extends HttpTestBase {
server.listen(testAddress, onSuccess(server -> {
HttpClientRequest req = client.request(HttpMethod.GET, testAddress, DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, DEFAULT_TEST_URI, resp -> testComplete());
headers.forEach((k, v) -> req.headers().add(k, v));
expectedHeaders.forEach((k, v) -> req.headers().add(k, v));
req.end();
}));
@@ -841,11 +840,13 @@ public abstract class HttpTest extends HttpTestBase {
}
private void testRequestHeaders(boolean individually) {
MultiMap headers = getHeaders(10);
MultiMap expectedHeaders = getHeaders(10);
server.requestHandler(req -> {
assertTrue(headers.size() < req.headers().size());
for (Map.Entry<String, String> entry : headers) {
MultiMap headers = req.headers();
headers.remove("host");
assertEquals(expectedHeaders.size(), expectedHeaders.size());
for (Map.Entry<String, String> entry : expectedHeaders) {
assertEquals(entry.getValue(), req.headers().get(entry.getKey()));
assertEquals(entry.getValue(), req.getHeader(entry.getKey()));
}
@@ -855,11 +856,11 @@ public abstract class HttpTest extends HttpTestBase {
server.listen(testAddress, onSuccess(server -> {
HttpClientRequest req = client.request(HttpMethod.GET, testAddress, DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, DEFAULT_TEST_URI, resp -> testComplete());
if (individually) {
for (Map.Entry<String, String> header : headers) {
for (Map.Entry<String, String> header : expectedHeaders) {
req.headers().add(header.getKey(), header.getValue());
}
} else {
req.headers().setAll(headers);
req.headers().setAll(expectedHeaders);
}
req.end();
}));
@@ -4812,7 +4813,6 @@ public abstract class HttpTest extends HttpTestBase {
resp.headers().forEach(header -> {
String name = header.getKey();
switch (name.toLowerCase()) {
case ":status":
case "content-length":
break;
default: