Cleaned up the API of HttpUtil::ParseContentRangeHeader().
HttpUtil::ParseContentRangeHeader() is used to validate and extract
values from a Content-Range header for a 206 response.
Before this CL, ParseContentRangeHeader() also had partial support for
extracting values from a Content-Range header for a 416 response,
without validating the returned values.
This behavior didn't appear to be used by any production code, so this
CL removes that support and makes ParseContentRangeHeader return true
only if all three of |first_byte_position|, |last_byte_position|, and
|instance_length| are specified and valid for a 206 response; otherwise,
all outputs are set to -1 and returns false.
BUG=670913
Review-Url: https://codereview.chromium.org/2549143003
Cr-Commit-Position: refs/heads/master@{#439305}
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index f173653..60c7ed7 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -501,7 +501,7 @@
int64_t range_start, range_end, object_size;
ASSERT_TRUE(
- headers->GetContentRange(&range_start, &range_end, &object_size));
+ headers->GetContentRangeFor206(&range_start, &range_end, &object_size));
int64_t content_length = headers->GetContentLength();
int length = end - start + 1;
diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index 6aa79511..b849d78 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -1303,9 +1303,10 @@
return result;
}
-bool HttpResponseHeaders::GetContentRange(int64_t* first_byte_position,
- int64_t* last_byte_position,
- int64_t* instance_length) const {
+bool HttpResponseHeaders::GetContentRangeFor206(
+ int64_t* first_byte_position,
+ int64_t* last_byte_position,
+ int64_t* instance_length) const {
size_t iter = 0;
std::string content_range_spec;
if (!EnumerateHeader(&iter, kContentRange, &content_range_spec)) {
@@ -1313,9 +1314,9 @@
return false;
}
- return HttpUtil::ParseContentRangeHeader(content_range_spec,
- first_byte_position,
- last_byte_position, instance_length);
+ return HttpUtil::ParseContentRangeHeaderFor206(
+ content_range_spec, first_byte_position, last_byte_position,
+ instance_length);
}
std::unique_ptr<base::Value> HttpResponseHeaders::NetLogCallback(
diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h
index 39404cf..6dc14a9 100644
--- a/net/http/http_response_headers.h
+++ b/net/http/http_response_headers.h
@@ -292,16 +292,16 @@
// such header in the response.
int64_t GetInt64HeaderValue(const std::string& header) const;
- // Extracts the values in a Content-Range header and returns true if they are
- // valid for a 206 response; otherwise returns false.
+ // Extracts the values in a Content-Range header and returns true if all three
+ // values are present and valid for a 206 response; otherwise returns false.
// The following values will be outputted:
// |*first_byte_position| = inclusive position of the first byte of the range
// |*last_byte_position| = inclusive position of the last byte of the range
// |*instance_length| = size in bytes of the object requested
- // If any of the above values is unknown, its value will be -1.
- bool GetContentRange(int64_t* first_byte_position,
- int64_t* last_byte_position,
- int64_t* instance_length) const;
+ // If this method returns false, then all of the outputs will be -1.
+ bool GetContentRangeFor206(int64_t* first_byte_position,
+ int64_t* last_byte_position,
+ int64_t* instance_length) const;
// Returns true if the response is chunk-encoded.
bool IsChunkEncoded() const;
diff --git a/net/http/http_response_headers_unittest.cc b/net/http/http_response_headers_unittest.cc
index 041972b7..2f2f7be 100644
--- a/net/http/http_response_headers_unittest.cc
+++ b/net/http/http_response_headers_unittest.cc
@@ -1338,7 +1338,7 @@
public ::testing::WithParamInterface<ContentRangeTestData> {
};
-TEST_P(ContentRangeTest, GetContentRange) {
+TEST_P(ContentRangeTest, GetContentRangeFor206) {
const ContentRangeTestData test = GetParam();
std::string headers(test.headers);
@@ -1348,9 +1348,8 @@
int64_t first_byte_position;
int64_t last_byte_position;
int64_t instance_size;
- bool return_value = parsed->GetContentRange(&first_byte_position,
- &last_byte_position,
- &instance_size);
+ bool return_value = parsed->GetContentRangeFor206(
+ &first_byte_position, &last_byte_position, &instance_size);
EXPECT_EQ(test.expected_return_value, return_value);
EXPECT_EQ(test.expected_first_byte_position, first_byte_position);
EXPECT_EQ(test.expected_last_byte_position, last_byte_position);
@@ -1363,99 +1362,16 @@
"Content-Range:",
false, -1, -1, -1},
{"HTTP/1.1 206 Partial Content\n"
- "Content-Range: megabytes 0-10/50",
- false, -1, -1, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: 0-10/50",
- false, -1, -1, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: Bytes 0-50/51",
- true, 0, 50, 51},
- {"HTTP/1.1 206 Partial Content\n"
"Content-Range: bytes 0-50/51",
true, 0, 50, 51},
{"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes\t0-50/51",
- false, -1, -1, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-50/51",
- true, 0, 50, 51},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0 - 50 \t / \t51",
- true, 0, 50, 51},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0\t-\t50\t/\t51\t",
- true, 0, 50, 51},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: \tbytes\t\t\t 0\t-\t50\t/\t51\t",
- true, 0, 50, 51},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: \t bytes \t 0 - 50 / 5 1",
- false, 0, 50, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: \t bytes \t 0 - 5 0 / 51",
- false, -1, -1, -1},
- {"HTTP/1.1 206 Partial Content\n"
"Content-Range: bytes 50-0/51",
- false, 50, 0, -1},
- {"HTTP/1.1 416 Requested range not satisfiable\n"
- "Content-Range: bytes * /*",
false, -1, -1, -1},
{"HTTP/1.1 416 Requested range not satisfiable\n"
- "Content-Range: bytes * / * ",
+ "Content-Range: bytes */*",
false, -1, -1, -1},
{"HTTP/1.1 206 Partial Content\n"
"Content-Range: bytes 0-50/*",
- false, 0, 50, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-50 / * ",
- false, 0, 50, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-10000000000/10000000001",
- true, 0, 10000000000ll, 10000000001ll},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-10000000000/10000000000",
- false, 0, 10000000000ll, 10000000000ll},
- // 64 bit wraparound.
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0 - 9223372036854775807 / 100",
- false, 0, std::numeric_limits<int64_t>::max(), 100},
- // 64 bit wraparound.
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0 - 100 / -9223372036854775808",
- false, 0, 100, std::numeric_limits<int64_t>::min()},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes */50",
- false, -1, -1, 50},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-50/10",
- false, 0, 50, 10},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 40-50/45",
- false, 40, 50, 45},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-50/-10",
- false, 0, 50, -10},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-0/1",
- true, 0, 0, 1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-40000000000000000000/40000000000000000001",
- false, -1, -1, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 1-/100",
- false, -1, -1, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes -/100",
- false, -1, -1, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes -1/100",
- false, -1, -1, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes 0-1233/*",
- false, 0, 1233, -1},
- {"HTTP/1.1 206 Partial Content\n"
- "Content-Range: bytes -123 - -1/100",
false, -1, -1, -1},
};
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index 26552e3..40c37ed 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -271,10 +271,11 @@
// byte-range-resp-spec = (first-byte-pos "-" last-byte-pos) | "*"
// instance-length = 1*DIGIT
// bytes-unit = "bytes"
-bool HttpUtil::ParseContentRangeHeader(base::StringPiece content_range_spec,
- int64_t* first_byte_position,
- int64_t* last_byte_position,
- int64_t* instance_length) {
+bool HttpUtil::ParseContentRangeHeaderFor206(
+ base::StringPiece content_range_spec,
+ int64_t* first_byte_position,
+ int64_t* last_byte_position,
+ int64_t* instance_length) {
*first_byte_position = *last_byte_position = *instance_length = -1;
content_range_spec = TrimLWS(content_range_spec);
@@ -288,53 +289,31 @@
return false;
}
- size_t slash_position = content_range_spec.find('/', space_position + 1);
+ size_t minus_position = content_range_spec.find('-', space_position + 1);
+ if (minus_position == base::StringPiece::npos)
+ return false;
+ size_t slash_position = content_range_spec.find('/', minus_position + 1);
if (slash_position == base::StringPiece::npos)
return false;
- // Obtain the part behind the space and before slash.
- base::StringPiece byte_range_resp_spec = TrimLWS(content_range_spec.substr(
- space_position + 1, slash_position - (space_position + 1)));
-
- if (byte_range_resp_spec != "*") {
- size_t minus_position = byte_range_resp_spec.find('-');
- if (minus_position == base::StringPiece::npos)
- return false;
-
- // Obtain first-byte-pos and last-byte-pos.
- if (!base::StringToInt64(
- TrimLWS(byte_range_resp_spec.substr(0, minus_position)),
- first_byte_position) ||
- !base::StringToInt64(
- TrimLWS(byte_range_resp_spec.substr(minus_position + 1)),
- last_byte_position)) {
- *first_byte_position = *last_byte_position = -1;
- return false;
- }
- if (*first_byte_position < 0 || *last_byte_position < 0 ||
- *first_byte_position > *last_byte_position)
- return false;
+ if (base::StringToInt64(
+ TrimLWS(content_range_spec.substr(
+ space_position + 1, minus_position - (space_position + 1))),
+ first_byte_position) &&
+ *first_byte_position >= 0 &&
+ base::StringToInt64(
+ TrimLWS(content_range_spec.substr(
+ minus_position + 1, slash_position - (minus_position + 1))),
+ last_byte_position) &&
+ *last_byte_position >= *first_byte_position &&
+ base::StringToInt64(
+ TrimLWS(content_range_spec.substr(slash_position + 1)),
+ instance_length) &&
+ *instance_length > *last_byte_position) {
+ return true;
}
-
- // Parse the instance-length part.
- base::StringPiece instance_length_spec =
- TrimLWS(content_range_spec.substr(slash_position + 1));
-
- if (base::StartsWith(instance_length_spec, "*",
- base::CompareCase::SENSITIVE)) {
- return false;
- } else if (!base::StringToInt64(instance_length_spec, instance_length)) {
- *instance_length = -1;
- return false;
- }
-
- // We have all the values; let's verify that they make sense for a 206
- // response.
- if (*first_byte_position < 0 || *last_byte_position < 0 ||
- *instance_length < 0 || *instance_length - 1 < *last_byte_position)
- return false;
-
- return true;
+ *first_byte_position = *last_byte_position = *instance_length = -1;
+ return false;
}
// static
diff --git a/net/http/http_util.h b/net/http/http_util.h
index a0ee9bb..de4b1a6 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -60,20 +60,18 @@
static bool ParseRangeHeader(const std::string& range_specifier,
std::vector<HttpByteRange>* ranges);
- // Extracts the values in a Content-Range header and returns true if they are
- // valid for a 206 response; otherwise returns false.
+ // Extracts the values in a Content-Range header and returns true if all three
+ // values are present and valid for a 206 response; otherwise returns false.
// The following values will be outputted:
// |*first_byte_position| = inclusive position of the first byte of the range
// |*last_byte_position| = inclusive position of the last byte of the range
// |*instance_length| = size in bytes of the object requested
- // If any of the above values is unknown, its value will be -1.
- // TODO(sclittle): Change this method to only support Content-Range headers
- // from 206 responses, since right now it only has incomplete support for
- // Content-Range headers from 416 responses. See crbug.com/670913.
- static bool ParseContentRangeHeader(base::StringPiece content_range_spec,
- int64_t* first_byte_position,
- int64_t* last_byte_position,
- int64_t* instance_length);
+ // If this method returns false, then all of the outputs will be -1.
+ static bool ParseContentRangeHeaderFor206(
+ base::StringPiece content_range_spec,
+ int64_t* first_byte_position,
+ int64_t* last_byte_position,
+ int64_t* instance_length);
// Parses a Retry-After header that is either an absolute date/time or a
// number of seconds in the future. Interprets absolute times as relative to
diff --git a/net/http/http_util_unittest.cc b/net/http/http_util_unittest.cc
index ee38dc6..1c41d82 100644
--- a/net/http/http_util_unittest.cc
+++ b/net/http/http_util_unittest.cc
@@ -995,40 +995,37 @@
{" bytes 0 - 50 \t / \t51", true, 0, 50, 51},
{"bytes 0\t-\t50\t/\t51\t", true, 0, 50, 51},
{" \tbytes\t\t\t 0\t-\t50\t/\t51\t", true, 0, 50, 51},
- {"\t bytes \t 0 - 50 / 5 1", false, 0, 50, -1},
+ {"\t bytes \t 0 - 50 / 5 1", false, -1, -1, -1},
{"\t bytes \t 0 - 5 0 / 51", false, -1, -1, -1},
- {"bytes 50-0/51", false, 50, 0, -1},
+ {"bytes 50-0/51", false, -1, -1, -1},
{"bytes * /*", false, -1, -1, -1},
{"bytes * / * ", false, -1, -1, -1},
- {"bytes 0-50/*", false, 0, 50, -1},
- {"bytes 0-50 / * ", false, 0, 50, -1},
+ {"bytes 0-50/*", false, -1, -1, -1},
+ {"bytes 0-50 / * ", false, -1, -1, -1},
{"bytes 0-10000000000/10000000001", true, 0, 10000000000ll,
10000000001ll},
- {"bytes 0-10000000000/10000000000", false, 0, 10000000000ll,
- 10000000000ll},
+ {"bytes 0-10000000000/10000000000", false, -1, -1, -1},
// 64 bit wraparound.
- {"bytes 0 - 9223372036854775807 / 100", false, 0,
- std::numeric_limits<int64_t>::max(), 100},
+ {"bytes 0 - 9223372036854775807 / 100", false, -1, -1, -1},
// 64 bit wraparound.
- {"bytes 0 - 100 / -9223372036854775808", false, 0, 100,
- std::numeric_limits<int64_t>::min()},
- {"bytes */50", false, -1, -1, 50},
- {"bytes 0-50/10", false, 0, 50, 10},
- {"bytes 40-50/45", false, 40, 50, 45},
- {"bytes 0-50/-10", false, 0, 50, -10},
+ {"bytes 0 - 100 / -9223372036854775808", false, -1, -1, -1},
+ {"bytes */50", false, -1, -1, -1},
+ {"bytes 0-50/10", false, -1, -1, -1},
+ {"bytes 40-50/45", false, -1, -1, -1},
+ {"bytes 0-50/-10", false, -1, -1, -1},
{"bytes 0-0/1", true, 0, 0, 1},
{"bytes 0-40000000000000000000/40000000000000000001", false, -1, -1, -1},
{"bytes 1-/100", false, -1, -1, -1},
{"bytes -/100", false, -1, -1, -1},
{"bytes -1/100", false, -1, -1, -1},
- {"bytes 0-1233/*", false, 0, 1233, -1},
+ {"bytes 0-1233/*", false, -1, -1, -1},
{"bytes -123 - -1/100", false, -1, -1, -1},
};
for (const auto& test : tests) {
int64_t first_byte_position, last_byte_position, instance_length;
EXPECT_EQ(test.expected_return_value,
- HttpUtil::ParseContentRangeHeader(
+ HttpUtil::ParseContentRangeHeaderFor206(
test.content_range_header_spec, &first_byte_position,
&last_byte_position, &instance_length))
<< test.content_range_header_spec;
diff --git a/net/http/partial_data.cc b/net/http/partial_data.cc
index 915aaa4..dceea25b 100644
--- a/net/http/partial_data.cc
+++ b/net/http/partial_data.cc
@@ -274,7 +274,7 @@
}
int64_t start, end, total_length;
- if (!headers->GetContentRange(&start, &end, &total_length))
+ if (!headers->GetContentRangeFor206(&start, &end, &total_length))
return false;
if (total_length <= 0)
return false;