Commit 6c652aa3 authored by Aaron Tagliaboschi's avatar Aaron Tagliaboschi Committed by Commit Bot

Fix Accepted-Encoding to "identity" whenever there's a range request

Unit test for encoding advertisement on range requests.


Simple logic to change accepted encoding to identity in a range request

Bug: 849952
Change-Id: I436ac5e4db118faa4f362a0b8c1943b5fe7020ea
Reviewed-on: https://chromium-review.googlesource.com/c/1352672Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Commit-Queue: Aaron Tagliaboschi <aarontag@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612816}
parent 89035f09
...@@ -645,6 +645,12 @@ void URLRequestHttpJob::StartTransactionInternal() { ...@@ -645,6 +645,12 @@ void URLRequestHttpJob::StartTransactionInternal() {
void URLRequestHttpJob::AddExtraHeaders() { void URLRequestHttpJob::AddExtraHeaders() {
if (!request_info_.extra_headers.HasHeader( if (!request_info_.extra_headers.HasHeader(
HttpRequestHeaders::kAcceptEncoding)) { HttpRequestHeaders::kAcceptEncoding)) {
// If a range is specifically requested, set the "Accepted Encoding" header
// to "identity"
if (request_info_.extra_headers.HasHeader(HttpRequestHeaders::kRange)) {
request_info_.extra_headers.SetHeader(HttpRequestHeaders::kAcceptEncoding,
"identity");
} else {
// Advertise "br" encoding only if transferred data is opaque to proxy. // Advertise "br" encoding only if transferred data is opaque to proxy.
bool advertise_brotli = false; bool advertise_brotli = false;
if (request()->context()->enable_brotli()) { if (request()->context()->enable_brotli()) {
...@@ -654,11 +660,11 @@ void URLRequestHttpJob::AddExtraHeaders() { ...@@ -654,11 +660,11 @@ void URLRequestHttpJob::AddExtraHeaders() {
} }
} }
// Supply Accept-Encoding headers first so that it is more likely that they // Supply Accept-Encoding headers first so that it is more likely that
// will be in the first transmitted packet. This can sometimes make it // they will be in the first transmitted packet. This can sometimes make
// easier to filter and analyze the streams to assure that a proxy has not // it easier to filter and analyze the streams to assure that a proxy has
// damaged these headers. Some proxies deliberately corrupt Accept-Encoding // not damaged these headers. Some proxies deliberately corrupt
// headers. // Accept-Encoding headers.
std::string advertised_encodings = "gzip, deflate"; std::string advertised_encodings = "gzip, deflate";
if (advertise_brotli) if (advertise_brotli)
advertised_encodings += ", br"; advertised_encodings += ", br";
...@@ -666,6 +672,7 @@ void URLRequestHttpJob::AddExtraHeaders() { ...@@ -666,6 +672,7 @@ void URLRequestHttpJob::AddExtraHeaders() {
request_info_.extra_headers.SetHeader(HttpRequestHeaders::kAcceptEncoding, request_info_.extra_headers.SetHeader(HttpRequestHeaders::kAcceptEncoding,
advertised_encodings); advertised_encodings);
} }
}
if (http_user_agent_settings_) { if (http_user_agent_settings_) {
// Only add default Accept-Language if the request didn't have it // Only add default Accept-Language if the request didn't have it
......
...@@ -1209,6 +1209,82 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest, ...@@ -1209,6 +1209,82 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest,
histograms.ExpectTotalCount(kCTRequiredHistogramName, 0); histograms.ExpectTotalCount(kCTRequiredHistogramName, 0);
} }
TEST_F(URLRequestHttpJobWithMockSocketsTest, EncodingAdvertisementOnRange) {
MockWrite writes[] = {
MockWrite("GET / HTTP/1.1\r\n"
"Host: www.example.com\r\n"
"Connection: keep-alive\r\n"
"User-Agent: \r\n"
"Accept-Encoding: identity\r\n"
"Accept-Language: en-us,fr\r\n"
"Range: bytes=0-1023\r\n\r\n")};
MockRead reads[] = {MockRead("HTTP/1.1 200 OK\r\n"
"Accept-Ranges: bytes\r\n"
"Content-Length: 12\r\n\r\n"),
MockRead("Test Content")};
StaticSocketDataProvider socket_data(reads, writes);
socket_factory_.AddSocketDataProvider(&socket_data);
TestDelegate delegate;
std::unique_ptr<URLRequest> request =
context_->CreateRequest(GURL("http://www.example.com"), DEFAULT_PRIORITY,
&delegate, TRAFFIC_ANNOTATION_FOR_TESTS);
// Make the extra header to trigger the change in "Accepted-Encoding"
HttpRequestHeaders headers;
headers.SetHeader("Range", "bytes=0-1023");
request->SetExtraRequestHeaders(headers);
request->Start();
base::RunLoop().RunUntilIdle();
EXPECT_THAT(delegate.request_status(), IsOk());
EXPECT_EQ(12, request->received_response_content_length());
EXPECT_EQ(CountWriteBytes(writes), request->GetTotalSentBytes());
EXPECT_EQ(CountReadBytes(reads), request->GetTotalReceivedBytes());
}
TEST_F(URLRequestHttpJobWithMockSocketsTest, RangeRequestOverrideEncoding) {
MockWrite writes[] = {
MockWrite("GET / HTTP/1.1\r\n"
"Host: www.example.com\r\n"
"Connection: keep-alive\r\n"
"Accept-Encoding: gzip, deflate\r\n"
"User-Agent: \r\n"
"Accept-Language: en-us,fr\r\n"
"Range: bytes=0-1023\r\n\r\n")};
MockRead reads[] = {MockRead("HTTP/1.1 200 OK\r\n"
"Accept-Ranges: bytes\r\n"
"Content-Length: 12\r\n\r\n"),
MockRead("Test Content")};
StaticSocketDataProvider socket_data(reads, writes);
socket_factory_.AddSocketDataProvider(&socket_data);
TestDelegate delegate;
std::unique_ptr<URLRequest> request =
context_->CreateRequest(GURL("http://www.example.com"), DEFAULT_PRIORITY,
&delegate, TRAFFIC_ANNOTATION_FOR_TESTS);
// Explicitly set "Accept-Encoding" to make sure it's not overridden by
// AddExtraHeaders
HttpRequestHeaders headers;
headers.SetHeader("Accept-Encoding", "gzip, deflate");
headers.SetHeader("Range", "bytes=0-1023");
request->SetExtraRequestHeaders(headers);
request->Start();
base::RunLoop().RunUntilIdle();
EXPECT_THAT(delegate.request_status(), IsOk());
EXPECT_EQ(12, request->received_response_content_length());
EXPECT_EQ(CountWriteBytes(writes), request->GetTotalSentBytes());
EXPECT_EQ(CountReadBytes(reads), request->GetTotalReceivedBytes());
}
TEST_F(URLRequestHttpJobTest, TestCancelWhileReadingCookies) { TEST_F(URLRequestHttpJobTest, TestCancelWhileReadingCookies) {
DelayedCookieMonster cookie_monster; DelayedCookieMonster cookie_monster;
TestURLRequestContext context(true); TestURLRequestContext context(true);
......
This is a testharness.js-based test.
PASS Range header setting allowed for guard type: none
PASS Range header setting allowed for guard type: response
PASS Range header setting allowed for guard type: request
PASS Privileged header not allowed for guard type: request-no-cors
FAIL Fetch with range header will be sent with Accept-Encoding: identity assert_equals: Expect identity accept-encoding if range header is "bytes=0-10" expected "identity" but got "gzip, deflate"
Harness: the test ran to completion.
This is a testharness.js-based test.
PASS Range header setting allowed for guard type: none
PASS Range header setting allowed for guard type: response
PASS Range header setting allowed for guard type: request
PASS Privileged header not allowed for guard type: request-no-cors
FAIL Fetch with range header will be sent with Accept-Encoding: identity assert_equals: Expect identity accept-encoding if range header is "bytes=0-10" expected "identity" but got "gzip, deflate"
Harness: the test ran to completion.
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