Skip to content

Commit

Permalink
Fix a regression with the introduction of HTTP authority handling wit…
Browse files Browse the repository at this point in the history
…h HTTP/1.1 in which the server fails when handling an empty host portion of the host header. The authority method can also return null when no host header is present for HTTP/1.x
  • Loading branch information
vietj committed Oct 6, 2023
1 parent f0ae90f commit 3230e4d
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 16 deletions.
6 changes: 4 additions & 2 deletions src/main/java/io/vertx/core/http/HttpServerRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,10 @@ default boolean isSSL() {
String query();

/**
* @return the request authority. For HTTP2 it returns the {@literal :authority} pseudo header otherwise it returns the {@literal Host} header.
* When the authority string does not carry a port, the {@link HostAndPort#port()} returns {@code -1} to indicate the scheme port is prevalent.
* @return the request authority. For HTTP/2 the {@literal :authority} pseudo header is returned, for HTTP/1.x the
* {@literal Host} header is returned or {@code null} when no such header is present. When the authority
* string does not carry a port, the {@link HostAndPort#port()} returns {@code -1} to indicate the
* scheme port is prevalent.
*/
@Nullable
HostAndPort authority();
Expand Down
21 changes: 12 additions & 9 deletions src/main/java/io/vertx/core/http/impl/EndpointKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import java.util.Objects;

class EndpointKey {
final class EndpointKey {

final boolean ssl;
final SocketAddress server;
Expand All @@ -27,9 +27,6 @@ class EndpointKey {
if (server == null) {
throw new NullPointerException("No null server address");
}
if (authority == null) {
throw new NullPointerException("No null authority address");
}
this.ssl = ssl;
this.proxyOptions = proxyOptions;
this.authority = authority;
Expand All @@ -38,17 +35,23 @@ class EndpointKey {

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
EndpointKey that = (EndpointKey) o;
return ssl == that.ssl && server.equals(that.server) && authority.equals(that.authority) && equals(proxyOptions, that.proxyOptions);
if (this == o) {
return true;
};
if (o instanceof EndpointKey) {
EndpointKey that = (EndpointKey) o;
return ssl == that.ssl && server.equals(that.server) && Objects.equals(authority, that.authority) && equals(proxyOptions, that.proxyOptions);
}
return false;
}

@Override
public int hashCode() {
int result = ssl ? 1 : 0;
result = 31 * result + authority.hashCode();
result = 31 * result + server.hashCode();
if (authority != null) {
result = 31 * result + authority.hashCode();
}
if (proxyOptions != null) {
result = 31 * result + hashCode(proxyOptions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ public abstract class HttpClientRequestBase implements HttpClientRequest {
}

protected String authority() {
if ((authority.port() == 80 && !ssl) || (authority.port() == 443 && ssl) || authority.port() < 0) {
if (authority == null) {
return null;
} else if ((authority.port() == 80 && !ssl) || (authority.port() == 443 && ssl) || authority.port() < 0) {
return authority.host();
} else {
return authority.host() + ':' + authority.port();
Expand Down
3 changes: 0 additions & 3 deletions src/main/java/io/vertx/core/net/impl/HostAndPortImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,6 @@ static boolean isHEXDIG(char ch) {
*/
public static HostAndPortImpl parseHostAndPort(String s, int schemePort) {
int pos = parseHost(s, 0, s.length());
if (pos < 1) {
return null;
}
if (pos == s.length()) {
return new HostAndPortImpl(s, schemePort);
}
Expand Down
40 changes: 40 additions & 0 deletions src/test/java/io/vertx/core/http/Http1xTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import io.netty.channel.ChannelHandler;
import io.netty.channel.EventLoop;
import io.netty.handler.codec.TooLongFrameException;
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.TooLongHttpHeaderException;
import io.vertx.core.*;
import io.vertx.core.Future;
Expand Down Expand Up @@ -5508,4 +5509,43 @@ public void testShutdown() throws Exception {
awaitFuture(shutdown);
await();
}

@Test
public void testEmptyHostHeader() throws Exception {
testEmptyHostPortionOfHostHeader("", -1);
}

@Test
public void testEmptyHostPortionOfHostHeader() throws Exception {
testEmptyHostPortionOfHostHeader(":8080", 8080);
}

private void testEmptyHostPortionOfHostHeader(String hostHeader, int expectedPort) throws Exception {
server.requestHandler(req -> {
assertEquals("", req.authority().host());
assertEquals(expectedPort, req.authority().port());
req.response().end();
});
startServer(testAddress);
client.request(new RequestOptions().setServer(testAddress).putHeader(HttpHeaderNames.HOST, hostHeader))
.compose(req -> req
.send()
.compose(HttpClientResponse::body))
.onComplete(onSuccess(v -> testComplete()));
await();
}

@Test
public void testMissingHostHeader() throws Exception {
server.requestHandler(req -> {
assertEquals(null, req.authority());
testComplete();
});
startServer(testAddress);
NetClient nc = vertx.createNetClient();
nc.connect(testAddress).onComplete(onSuccess(so -> {
so.write("GET / HTTP/1.1\r\n\r\n");
}));
await();
}
}
3 changes: 2 additions & 1 deletion src/test/java/io/vertx/core/net/impl/HostAndPortTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ public void testParseHostAndPort() {
assertHostAndPort("example.com", -1, "example.com");
assertHostAndPort("0.1.2.3", -1, "0.1.2.3");
assertHostAndPort("[0::]", -1, "[0::]");
assertNull(HostAndPortImpl.parseHostAndPort("", -1));
assertHostAndPort("", -1, "");
assertHostAndPort("", 8080, ":8080");
assertNull(HostAndPortImpl.parseHostAndPort("/", -1));
}

Expand Down

0 comments on commit 3230e4d

Please sign in to comment.