Handle authority value computation in Http2ServerRequest instead of the headers implementation.

Author: vietj

vertx-core/src/main/java/io/vertx/core/http/impl/http2/Http2HeadersMultiMap.java

@@ -57,8 +57,7 @@ private Http2HeadersMultiMap(boolean mutable, Headers<CharSequence, CharSequence
 
   private Integer status;
   private HttpMethod method;
-  private HostAndPort computedAuthority;
-  private HostAndPort realAuthority;
+  private HostAndPort authority;
   private String uri;
   private String scheme;
 
@@ -76,25 +75,25 @@ public boolean validate(boolean isRequest) {
       CharSequence pathHeader = headers.get(HttpHeaders.PSEUDO_PATH);
       String uri = pathHeader != null ? pathHeader.toString() : null;
 
-      HostAndPort currentAuthority = null;
-      String authorityHeaderAsString;
+      HostAndPort authority;
       CharSequence authorityHeader = headers.get(HttpHeaders.PSEUDO_AUTHORITY);
       if (authorityHeader != null) {
-        authorityHeaderAsString = authorityHeader.toString();
-        this.realAuthority = HostAndPort.parseAuthority(authorityHeaderAsString, -1);
-        currentAuthority = this.realAuthority;
+        String authorityHeaderAsString = authorityHeader.toString();
+        authority = HostAndPort.parseAuthority(authorityHeaderAsString, -1);
+      } else {
+        authority = null;
       }
 
+      HostAndPort authorityPresence;
       CharSequence hostHeader = headers.get(HttpHeaders.HOST);
-      if (currentAuthority == null) {
-        headers.remove(HttpHeaders.HOST);
-        if (hostHeader != null) {
-          currentAuthority = HostAndPort.parseAuthority(hostHeader.toString(), -1);
-        }
+      if (authority == null && hostHeader != null) {
+        authorityPresence = HostAndPort.parseAuthority(hostHeader.toString(), -1);
+      } else {
+        authorityPresence = authority;
       }
 
       if (method == HttpMethod.CONNECT) {
-        if (scheme != null || uri != null || currentAuthority == null) {
+        if (scheme != null || uri != null || authorityPresence == null) {
           return false;
         }
       } else {
@@ -105,20 +104,21 @@ public boolean validate(boolean isRequest) {
 
       boolean hasAuthority = authorityHeader != null || hostHeader != null;
       if (hasAuthority) {
-        if (currentAuthority == null) {
+        if (authorityPresence == null) {
+          // Malformed authority
           return false;
         }
         if (hostHeader != null) {
           HostAndPort host = HostAndPort.parseAuthority(hostHeader.toString(), -1);
-          if (host == null || (!currentAuthority.host().equals(host.host()) || currentAuthority.port() != host.port())) {
+          if (host == null || (!authorityPresence.host().equals(host.host()) || authorityPresence.port() != host.port())) {
             return false;
           }
         }
       }
 
       this.method = method;
       this.uri = uri;
-      this.computedAuthority = currentAuthority;
+      this.authority = authority;
       this.scheme = scheme;
 
       return true;
@@ -158,8 +158,8 @@ public Http2HeadersMultiMap prepare() {
     if (scheme != null) {
       headers.set(HttpHeaders.PSEUDO_SCHEME, scheme);
     }
-    if (computedAuthority != null) {
-      headers.set(HttpHeaders.PSEUDO_AUTHORITY, computedAuthority.toString(ssl));
+    if (authority != null) {
+      headers.set(HttpHeaders.PSEUDO_AUTHORITY, authority.toString(ssl));
     }
     if (scheme != null) {
       headers.set(HttpHeaders.PSEUDO_SCHEME, scheme);
@@ -207,16 +207,12 @@ public HttpMethod method() {
   }
 
   public Http2HeadersMultiMap authority(HostAndPort authority) {
-    this.computedAuthority = authority;
+    this.authority = authority;
     return this;
   }
 
   public HostAndPort authority() {
-    return computedAuthority;
-  }
-
-  public HostAndPort authority(boolean real) {
-    return real ? realAuthority : computedAuthority;
+    return authority;
   }
 
   public String scheme() {

vertx-core/src/main/java/io/vertx/core/http/impl/http2/Http2ServerRequest.java

@@ -16,11 +16,13 @@
 import io.netty.handler.codec.http.multipart.Attribute;
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder;
 import io.netty.handler.codec.http.multipart.InterfaceHttpData;
+import io.netty.handler.codec.http2.Http2Headers;
 import io.vertx.codegen.annotations.Nullable;
 import io.vertx.core.Future;
 import io.vertx.core.Handler;
 import io.vertx.core.MultiMap;
 import io.vertx.core.buffer.Buffer;
+import io.vertx.core.http.HttpHeaders;
 import io.vertx.core.http.impl.HttpEventHandler;
 import io.vertx.core.http.impl.HttpRequestHead;
 import io.vertx.core.http.impl.HttpUtils;
@@ -61,6 +63,8 @@ public class Http2ServerRequest extends HttpServerRequestInternal {
 
   // Accessed on context thread
   private MultiMap headersMap;
+  private HostAndPort authority;
+  private HostAndPort realAuthority;
   private Charset paramsCharset = StandardCharsets.UTF_8;
   private MultiMap params;
   private boolean semicolonIsNormalCharInParams;
@@ -108,7 +112,21 @@ private HttpEventHandler eventHandler(boolean create) {
   }
 
   public void handleHeaders(HttpRequestHead headers) {
-    this.headersMap = headers.headers;
+    Http2HeadersMultiMap map = (Http2HeadersMultiMap)headers.headers;
+
+    realAuthority = headers.authority;
+    authority = headers.authority;
+    if (authority == null) {
+      String hostHeader = headers.headers.get(HttpHeaders.HOST);
+      if (hostHeader != null) {
+        headers.headers.remove(HttpHeaders.HOST);
+        authority = HostAndPort.parseAuthority(hostHeader, -1);
+      }
+    }
+
+    map.sanitize();
+
+    this.headersMap = map;
 
     // Check expect header and implement 100 continue automatically
     CharSequence value = headersMap.get(HttpHeaderNames.EXPECT);
@@ -334,12 +352,12 @@ public String scheme() {
 
   @Override
   public @Nullable HostAndPort authority() {
-    return stream.authority();
+    return authority;
   }
 
   @Override
   public @Nullable HostAndPort authority(boolean real) {
-    return stream.authority(real);
+    return real ? realAuthority : authority;
   }
 
   @Override

vertx-core/src/main/java/io/vertx/core/http/impl/http2/Http2ServerStream.java

@@ -118,11 +118,11 @@ public String scheme() {
   }
 
   public HostAndPort authority() {
-    return requestHeaders.authority(false);
+    return requestHeaders.authority();
   }
 
   public HostAndPort authority(boolean real) {
-    return requestHeaders.authority(real);
+    return requestHeaders.authority();
   }
 
   public Object metric() {

vertx-core/src/main/java/io/vertx/core/http/impl/http2/codec/Http2ServerConnectionImpl.java

@@ -125,7 +125,6 @@ protected synchronized void onHeadersRead(int streamId, Http2Headers headers, St
         handler.writeReset(streamId, Http2Error.PROTOCOL_ERROR.code(), null);
         return;
       }
-      headersMap.sanitize();
       if (streamId == 1 && handler.upgraded) {
         stream = createStream(headers, true);
       } else {

vertx-core/src/main/java/io/vertx/core/http/impl/http2/multiplex/Http2MultiplexServerConnection.java

@@ -79,7 +79,6 @@ void receiveHeaders(ChannelHandlerContext chctx, Http2FrameStream frameStream, H
         if (handler == null) {
           chctx.writeAndFlush(new DefaultHttp2ResetFrame(Http2Error.REFUSED_STREAM.code()));
         } else {
-          headersMap.sanitize();
           Http2ServerStream stream = new Http2ServerStream(this, serverMetrics, metric(),
                   streamContextSupplier.get(), null);;
           stream.init(streamId, chctx.channel().isWritable());

vertx-core/src/test/java/io/vertx/tests/http/Http2ClientTest.java

@@ -382,6 +382,7 @@ public void testOverrideAuthority() throws Exception {
   @Test
   public void testNoAuthority() throws Exception {
     server.requestHandler(req -> {
+      assertNotNull("fromHost", req.authority());
       assertEquals("fromHost", req.authority().host());
       assertEquals(1234, req.authority().port());
       assertNull(req.authority(true));

vertx-core/src/test/java/io/vertx/tests/http/headers/Http2HeadersAdaptorsTest.java

@@ -13,6 +13,7 @@
 
 import io.netty.handler.codec.http2.DefaultHttp2Headers;
 import io.vertx.core.MultiMap;
+import io.vertx.core.http.HttpMethod;
 import io.vertx.core.http.impl.http2.Http2HeadersMultiMap;
 import org.junit.Before;
 import org.junit.Ignore;
@@ -42,7 +43,7 @@ public void setUp() {
   }
 
   @Override
-  protected MultiMap newMultiMap() {
+  protected Http2HeadersMultiMap newMultiMap() {
     return new Http2HeadersMultiMap(new DefaultHttp2Headers());
   }
 
@@ -111,4 +112,38 @@ public void testEntries() {
   private void assertHeaderNames(String... expected) {
     assertEquals(new HashSet<>(Arrays.asList(expected)), headers.names().stream().map(CharSequence::toString).collect(Collectors.toSet()));
   }
+
+  private Http2HeadersMultiMap headers(HttpMethod method, String authority, String host) {
+    DefaultHttp2Headers s = new DefaultHttp2Headers();
+    s.set(":method", method.name());
+    if (method != HttpMethod.CONNECT) {
+      s.set(":path", "/");
+      s.set(":scheme", "http");
+    }
+    if (authority != null) {
+      s.set(":authority", authority);
+    }
+    if (host != null) {
+      s.set("host", host);
+    }
+    return new Http2HeadersMultiMap(s);
+  }
+
+  @Test
+  public void testAuthorityValidation() {
+    assertTrue(headers(HttpMethod.GET, null, null).validate(true));
+    assertTrue(headers(HttpMethod.GET, "localhost:8080", null).validate(true));
+    assertTrue(headers(HttpMethod.GET, null, "localhost:8080").validate(true));
+    assertTrue(headers(HttpMethod.GET, "localhost:8080", "localhost:8080").validate(true));
+    assertFalse(headers(HttpMethod.GET, "localhost:8080", "localhost:8081").validate(true));
+    assertFalse(headers(HttpMethod.GET, "localhost:a", null).validate(true));
+    assertFalse(headers(HttpMethod.GET, null, "localhost:a").validate(true));
+    assertFalse(headers(HttpMethod.CONNECT, null, null).validate(true));
+    assertTrue(headers(HttpMethod.CONNECT, "localhost:8080", null).validate(true));
+    assertTrue(headers(HttpMethod.CONNECT, null, "localhost:8080").validate(true));
+    assertTrue(headers(HttpMethod.CONNECT, "localhost:8080", "localhost:8080").validate(true));
+    assertFalse(headers(HttpMethod.CONNECT, "localhost:8080", "localhost:8081").validate(true));
+    assertFalse(headers(HttpMethod.CONNECT, "localhost:a", null).validate(true));
+    assertFalse(headers(HttpMethod.CONNECT, null, "localhost:a").validate(true));
+  }
 }