Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let header validator find host header field when :authority pseudo-header field is missing #324

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Let header validator find host header field when :authority pseudo-header field is missing #324

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da31eec
Let header validator find host header field when :authority pseud…
naokiiwakami Feb 4, 2025
6525874
Use netty's HttpHeaderNames class for host header field names
naokiiwakami Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions src/main/java/io/netty/incubator/codec/http3/Http3Headers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,35 @@ enum PseudoHeaderName {
/**
* {@code :method}.
*/
METHOD(":method", true),
METHOD(":method", true, 0x1),

/**
* {@code :scheme}.
*/
SCHEME(":scheme", true),
SCHEME(":scheme", true, 0x2),

/**
* {@code :authority}.
*/
AUTHORITY(":authority", true),
AUTHORITY(":authority", true, 0x4),

/**
* {@code :path}.
*/
PATH(":path", true),
PATH(":path", true, 0x8),

/**
* {@code :status}.
*/
STATUS(":status", false);
STATUS(":status", false, 0x10);

private static final char PSEUDO_HEADER_PREFIX = ':';
private static final byte PSEUDO_HEADER_PREFIX_BYTE = (byte) PSEUDO_HEADER_PREFIX;

private final AsciiString value;
private final boolean requestOnly;
// The position of the bit in the flag indicates the type of the header field
private final int flag;
private static final CharSequenceMap<PseudoHeaderName> PSEUDO_HEADERS = new CharSequenceMap<PseudoHeaderName>();

static {
Expand All @@ -66,9 +68,10 @@ enum PseudoHeaderName {
}
}

PseudoHeaderName(String value, boolean requestOnly) {
PseudoHeaderName(String value, boolean requestOnly, int flag) {
this.value = AsciiString.cached(value);
this.requestOnly = requestOnly;
this.flag = flag;
}

public AsciiString value() {
Expand Down Expand Up @@ -120,6 +123,10 @@ public static PseudoHeaderName getPseudoHeader(CharSequence name) {
public boolean isRequestOnly() {
return requestOnly;
}

public int getFlag() {
return flag;
}
}

/**
Expand Down
59 changes: 31 additions & 28 deletions src/main/java/io/netty/incubator/codec/http3/Http3HeadersSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

import java.util.function.BiConsumer;

import static io.netty.incubator.codec.http3.Http3Headers.PseudoHeaderName.AUTHORITY;
import static io.netty.incubator.codec.http3.Http3Headers.PseudoHeaderName.METHOD;
import static io.netty.incubator.codec.http3.Http3Headers.PseudoHeaderName.PATH;
import static io.netty.incubator.codec.http3.Http3Headers.PseudoHeaderName.SCHEME;
import static io.netty.incubator.codec.http3.Http3Headers.PseudoHeaderName.STATUS;
import static io.netty.incubator.codec.http3.Http3Headers.PseudoHeaderName.getPseudoHeader;
import static io.netty.incubator.codec.http3.Http3Headers.PseudoHeaderName.hasPseudoHeaderFormat;

Expand All @@ -36,7 +41,7 @@ final class Http3HeadersSink implements BiConsumer<CharSequence, CharSequence> {
private Http3HeadersValidationException validationException;
private HeaderType previousType;
private boolean request;
private int pseudoHeadersCount;
private int receivedPseudoHeaders;

Http3HeadersSink(Http3Headers headers, long maxHeaderListSize, boolean validate, boolean trailer) {
this.headers = headers;
Expand All @@ -58,7 +63,7 @@ void finish() throws Http3HeadersValidationException, Http3Exception {
}
if (validate) {
if (trailer) {
if (pseudoHeadersCount != 0) {
if (receivedPseudoHeaders != 0) {
// Trailers must not have pseudo headers.
throw new Http3HeadersValidationException("Pseudo-header(s) included in trailers.");
}
Expand All @@ -69,16 +74,12 @@ void finish() throws Http3HeadersValidationException, Http3Exception {
if (request) {
CharSequence method = headers.method();
// fast-path
if (pseudoHeadersCount < 2) {
// There can't be any duplicates for pseudy header names.
throw new Http3HeadersValidationException("Not all mandatory pseudo-headers included.");
}
if (HttpMethod.CONNECT.asciiName().contentEqualsIgnoreCase(method)) {
// For CONNECT we must only include:
// - :method
// - :authority
if (pseudoHeadersCount != 2 || headers.authority() == null) {
// There can't be any duplicates for pseudy header names.
final int requiredPseudoHeaders = METHOD.getFlag() | AUTHORITY.getFlag();
if (receivedPseudoHeaders != requiredPseudoHeaders) {
throw new Http3HeadersValidationException("Not all mandatory pseudo-headers included.");
}
} else if (HttpMethod.OPTIONS.asciiName().contentEqualsIgnoreCase(method)) {
Expand All @@ -90,36 +91,42 @@ void finish() throws Http3HeadersValidationException, Http3Exception {
// - :scheme
// - :authority
// - :path
if (pseudoHeadersCount != 4 &&
// - :method
// - :scheme
// - :path
!(pseudoHeadersCount == 3 && headers.authority() == null &&
"*".contentEquals(headers.path()))) {
final int requiredPseudoHeaders = METHOD.getFlag() | SCHEME.getFlag() | PATH.getFlag();
if ((receivedPseudoHeaders & requiredPseudoHeaders) != requiredPseudoHeaders ||
(!authorityOrHostHeaderReceived() && !"*".contentEquals(headers.path()))) {
throw new Http3HeadersValidationException("Not all mandatory pseudo-headers included.");
}
} else {
// For requests we must include:
// For other requests we must include:
// - :method
// - :scheme
// - :authority
// - :path
if (pseudoHeadersCount != 4) {
// There can't be any duplicates for pseudy header names.
final int requiredPseudoHeaders = METHOD.getFlag() | SCHEME.getFlag() | PATH.getFlag();
if ((receivedPseudoHeaders & requiredPseudoHeaders) != requiredPseudoHeaders ||
!authorityOrHostHeaderReceived()) {
throw new Http3HeadersValidationException("Not all mandatory pseudo-headers included.");
}
}
} else {
// For responses we must include:
// - :status
if (pseudoHeadersCount != 1) {
// There can't be any duplicates for pseudy header names.
if (receivedPseudoHeaders != STATUS.getFlag()) {
throw new Http3HeadersValidationException("Not all mandatory pseudo-headers included.");
}
}
}
}

/**
* Find host header field in case the :authority pseudo header is not specified.
* See:
* https://www.rfc-editor.org/rfc/rfc9110#section-7.2
*/
private boolean authorityOrHostHeaderReceived() {
return (receivedPseudoHeaders & AUTHORITY.getFlag()) == AUTHORITY.getFlag() || headers.contains("host");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use HttpHeaderNames.HOST

}

@Override
public void accept(CharSequence name, CharSequence value) {
headersLength += QpackHeaderField.sizeOf(name, value);
Expand Down Expand Up @@ -154,19 +161,15 @@ private void validate(Http3Headers headers, CharSequence name) {
throw new Http3HeadersValidationException(
String.format("Invalid HTTP/3 pseudo-header '%s' encountered.", name));
}

final HeaderType currentHeaderType = pseudoHeader.isRequestOnly() ?
HeaderType.REQUEST_PSEUDO_HEADER : HeaderType.RESPONSE_PSEUDO_HEADER;
if (previousType != null && currentHeaderType != previousType) {
throw new Http3HeadersValidationException("Mix of request and response pseudo-headers.");
}

if (headers.contains(name)) {
if ((receivedPseudoHeaders & pseudoHeader.getFlag()) != 0) {
// There can't be any duplicates for pseudy header names.
throw new Http3HeadersValidationException(
String.format("Pseudo-header field '%s' exists already.", name));
}
pseudoHeadersCount++;
receivedPseudoHeaders |= pseudoHeader.getFlag();

final HeaderType currentHeaderType = pseudoHeader.isRequestOnly() ?
HeaderType.REQUEST_PSEUDO_HEADER : HeaderType.RESPONSE_PSEUDO_HEADER;
request = pseudoHeader.isRequestOnly();
previousType = currentHeaderType;
} else {
Expand Down
34 changes: 33 additions & 1 deletion src/test/java/io/netty/incubator/codec/http3/Http3HeadersSinkTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package io.netty.incubator.codec.http3;


import com.google.common.net.HttpHeaders;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace by our own HttpHeaderNames class

import io.netty.util.AsciiString;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -143,14 +145,44 @@ public void testAuthorityNotRequiredForOptionsWildcard() throws Http3Exception {
}

@Test
public void testAuthorityRequiredForOptionsNonWildcard() throws Http3Exception {
public void testOptionsNonWildcardWithAuthority() throws Http3Exception {
Http3HeadersSink sink = new Http3HeadersSink(new DefaultHttp3Headers(), 512, true, false);
sink.accept(Http3Headers.PseudoHeaderName.METHOD.value(), "OPTIONS");
sink.accept(Http3Headers.PseudoHeaderName.PATH.value(), "/something");
sink.accept(Http3Headers.PseudoHeaderName.SCHEME.value(), "https");
sink.accept(Http3Headers.PseudoHeaderName.AUTHORITY.value(), "example.com:4433");
sink.finish();
}

@Test
public void testOptionsNonWildcardWithHost() throws Http3Exception {
Http3HeadersSink sink = new Http3HeadersSink(new DefaultHttp3Headers(), 512, true, false);
sink.accept(Http3Headers.PseudoHeaderName.METHOD.value(), "OPTIONS");
sink.accept(Http3Headers.PseudoHeaderName.PATH.value(), "/something");
sink.accept(Http3Headers.PseudoHeaderName.SCHEME.value(), "https");
sink.accept(new AsciiString(HttpHeaders.HOST.toLowerCase()), "example.com:4433");
sink.finish();
}

@Test
public void testAuthorityOrHostRequiredForOptionsNonWildcard() throws Http3Exception {
Http3HeadersSink sink = new Http3HeadersSink(new DefaultHttp3Headers(), 512, true, false);
sink.accept(Http3Headers.PseudoHeaderName.METHOD.value(), "OPTIONS");
sink.accept(Http3Headers.PseudoHeaderName.PATH.value(), "/something");
sink.accept(Http3Headers.PseudoHeaderName.SCHEME.value(), "https");
assertThrows(Http3HeadersValidationException.class, () -> sink.finish());
}

@Test
public void testHostExistsInsteadOfAuthority() throws Http3Exception {
Http3HeadersSink sink = new Http3HeadersSink(new DefaultHttp3Headers(), 512, true, false);
sink.accept(Http3Headers.PseudoHeaderName.METHOD.value(), "GET");
sink.accept(Http3Headers.PseudoHeaderName.PATH.value(), "/");
sink.accept(Http3Headers.PseudoHeaderName.SCHEME.value(), "https");
sink.accept(new AsciiString(HttpHeaders.HOST.toLowerCase()), "example.com:4433");
sink.finish();
}

private static void addMandatoryPseudoHeaders(Http3HeadersSink sink, boolean req) {
if (req) {
sink.accept(Http3Headers.PseudoHeaderName.METHOD.value(), "GET");
Expand Down
Loading