Skip to content

Commit

Permalink
Avoid failing HTTP/2 requests with upgrade-insecure-requests (#12799)
Browse files Browse the repository at this point in the history
Motivation:
This is a non-standard header that is not _explicitly_ called out as connection related, even though it can be argued that it is.
Regardless, Chrome and Firefox do actually send this header in their HTTP/2 requests, so rejecting these is quite troublesome.
Safari doesn't send this header.

Modification:
Remove the check for `upgrade-insecure-requests` in the header validation in HpackDecoder.
Also update tests to match.

Result:
HTTP/2 requests from Chrome and Firefox are no longer rejected by the header validation.

Fixes #12798
  • Loading branch information
chrisvest committed Sep 13, 2022
1 parent 75982ea commit 570c5d7
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 6 deletions.
Expand Up @@ -422,13 +422,12 @@ private static HeaderType validate(int streamId, CharSequence name,

@SuppressWarnings("deprecation") // We need to check for deprecated headers as well.
private static boolean isConnectionHeader(CharSequence name) {
// These are the known standard and non-standard connection related headers:
// These are the known standard connection related headers:
// - upgrade (7 chars)
// - connection (10 chars)
// - keep-alive (10 chars)
// - proxy-connection (16 chars)
// - transfer-encoding (17 chars)
// - upgrade-insecure-requests (25 chars)
//
// We scan for these based on the length, then double-check any matching name.
int len = name.length();
Expand All @@ -449,7 +448,7 @@ private static boolean isConnectionHeader(CharSequence name) {
if (len == 16) {
return contentEqualsIgnoreCase(name, HttpHeaderNames.PROXY_CONNECTION);
}
return len == 25 && contentEqualsIgnoreCase(name, HttpHeaderNames.UPGRADE_INSECURE_REQUESTS);
return false;
}

private static boolean contains(Http2Headers headers, CharSequence name) {
Expand Down
Expand Up @@ -167,9 +167,6 @@ public void decodingConnectionRelatedHeadersMustFailValidation() throws Exceptio

// Non-standard connection related headers:
verifyValidationFails(decoder, encode(b(":method"), b("GET"), b("proxy-connection"), b("keep-alive")));
verifyValidationFails(decoder, encode(b(":method"), b("GET"), b("upgrade-insecure-requests"), b("1")));
verifyValidationFails(decoder, encode(b(":method"), b("GET"),
b("content-security-policy"), b("upgrade-insecure-requests"), b("upgrade-insecure-requests"), b("1")));

// Only "trailers" is allowed for the TE header:
verifyValidationFails(decoder, encode(b(":method"), b("GET"), b("te"), b("compress")));
Expand Down

0 comments on commit 570c5d7

Please sign in to comment.