Commit c4f750f8 authored by Kunihiko Sakamoto's avatar Kunihiko Sakamoto Committed by Commit Bot

BundledExchanges: Require content-type header in non-empty responses

This reflects https://github.com/WICG/webpackage/pull/484.

Bug: 969596
Change-Id: I3484b6e76f51a76e507667499dddc37024cc0af7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1750528Reviewed-by: default avatarTakashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686659}
parent ab6e0fe8
...@@ -967,13 +967,7 @@ class BundledExchangesParser::ResponseParser ...@@ -967,13 +967,7 @@ class BundledExchangesParser::ResponseParser
return; return;
} }
// Step 9. "If headers does not contain a Content-Type header, return an // Step 9. "Let payloadLength be the result of getting the length of a CBOR
// error."
// TODO(crbug.com/966753): Implement this once
// https://github.com/WICG/webpackage/issues/445 is resolved.
// Step 10. "Let payloadLength be the result of getting the length of a CBOR
// bytestring header from stream (Section 3.5.2). If payloadLength is an // bytestring header from stream (Section 3.5.2). If payloadLength is an
// error, return that error." // error, return that error."
auto payload_length = input.ReadCBORHeader(CBORType::kByteString); auto payload_length = input.ReadCBORHeader(CBORType::kByteString);
...@@ -982,6 +976,15 @@ class BundledExchangesParser::ResponseParser ...@@ -982,6 +976,15 @@ class BundledExchangesParser::ResponseParser
return; return;
} }
// Step 10. "If payloadLength is greater than 0 and headers does not contain
// a Content-Type header, return an error."
if (*payload_length > 0 &&
!parsed_headers->headers.contains("content-type")) {
RunErrorCallbackAndDestroy(
"Non-empty response must have a content-type header.");
return;
}
// Step 11. "If stream.currentOffset + payloadLength != // Step 11. "If stream.currentOffset + payloadLength !=
// requestMetadata.offset + requestMetadata.length, return an error." // requestMetadata.offset + requestMetadata.length, return an error."
if (input.CurrentOffset() + *payload_length != response_length_) { if (input.CurrentOffset() + *payload_length != response_length_) {
......
...@@ -443,6 +443,31 @@ TEST_F(BundledExchangeParserTest, InvalidHeaderValue) { ...@@ -443,6 +443,31 @@ TEST_F(BundledExchangeParserTest, InvalidHeaderValue) {
ASSERT_FALSE(ParseResponse(&data_source, location)); ASSERT_FALSE(ParseResponse(&data_source, location));
} }
TEST_F(BundledExchangeParserTest, NoContentTypeWithNonEmptyContent) {
BundleBuilder builder(kFallbackUrl, kManifestUrl);
builder.AddExchange("https://test.example.com/", {{":status", "200"}},
"payload");
TestDataSource data_source(builder.CreateBundle());
mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
ASSERT_TRUE(metadata);
auto location = FindResponse(metadata, GURL("https://test.example.com/"));
ASSERT_TRUE(location);
ASSERT_FALSE(ParseResponse(&data_source, location));
}
TEST_F(BundledExchangeParserTest, NoContentTypeWithEmptyContent) {
BundleBuilder builder(kFallbackUrl, kManifestUrl);
builder.AddExchange("https://test.example.com/", {{":status", "301"}}, "");
TestDataSource data_source(builder.CreateBundle());
mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
ASSERT_TRUE(metadata);
auto location = FindResponse(metadata, GURL("https://test.example.com/"));
ASSERT_TRUE(location);
ASSERT_TRUE(ParseResponse(&data_source, location));
}
TEST_F(BundledExchangeParserTest, Variants) { TEST_F(BundledExchangeParserTest, Variants) {
BundleBuilder builder(kFallbackUrl, kManifestUrl); BundleBuilder builder(kFallbackUrl, kManifestUrl);
auto location1 = builder.AddResponse( auto location1 = builder.AddResponse(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment