From 87a7c4f94b8efd06b7c35058d8f8bf9c1d594b6a Mon Sep 17 00:00:00 2001 From: Jengerer Date: Mon, 20 Apr 2020 13:19:42 -0400 Subject: [PATCH] Fix handling of partial range request (fixes issue #2873) --- .../net_service/stream_reader_url_loader.cc | 9 ++-- libcef/common/net_service/net_service_util.cc | 5 +- tests/ceftests/test_util.cc | 6 ++- tests/ceftests/urlrequest_unittest.cc | 52 +++++++++++++++++-- 4 files changed, 61 insertions(+), 11 deletions(-) diff --git a/libcef/browser/net_service/stream_reader_url_loader.cc b/libcef/browser/net_service/stream_reader_url_loader.cc index e8be7609a..362956a2e 100644 --- a/libcef/browser/net_service/stream_reader_url_loader.cc +++ b/libcef/browser/net_service/stream_reader_url_loader.cc @@ -610,9 +610,12 @@ void StreamReaderURLLoader::OnReaderSkipCompleted(int64_t bytes_skipped) { HeadersComplete(net::HTTP_OK, -1); } else if (bytes_skipped == byte_range_.first_byte_position()) { // We skipped the expected number of bytes. - int64_t expected_content_length = byte_range_.last_byte_position() - - byte_range_.first_byte_position() + 1; - DCHECK_GE(expected_content_length, 0); + int64_t expected_content_length = -1; + if (byte_range_.HasLastBytePosition()) { + expected_content_length = byte_range_.last_byte_position() - + byte_range_.first_byte_position() + 1; + DCHECK_GE(expected_content_length, 0); + } HeadersComplete(net::HTTP_OK, expected_content_length); } else { RequestComplete(bytes_skipped < 0 ? bytes_skipped : net::ERR_FAILED); diff --git a/libcef/common/net_service/net_service_util.cc b/libcef/common/net_service/net_service_util.cc index 6e2e513b3..68a04a307 100644 --- a/libcef/common/net_service/net_service_util.cc +++ b/libcef/common/net_service/net_service_util.cc @@ -107,8 +107,9 @@ scoped_refptr MakeResponseHeaders( // Track the headers that have already been set. Perform all comparisons in // lowercase. std::set set_headers_lowercase; - - if (status_code == net::HTTP_OK) { + if ((status_code >= 200 && status_code < 300) && + status_code != net::HTTP_NO_CONTENT && + status_code != net::HTTP_RESET_CONTENT) { if (!mime_type.empty()) { headers->AddHeader(MakeHeader(net::HttpRequestHeaders::kContentType, MakeContentTypeValue(mime_type, charset))); diff --git a/tests/ceftests/test_util.cc b/tests/ceftests/test_util.cc index d8e6c9dfa..5058b1f60 100644 --- a/tests/ceftests/test_util.cc +++ b/tests/ceftests/test_util.cc @@ -21,8 +21,12 @@ void TestMapEqual(const CefRequest::HeaderMap& map1, for (it1 = map1.begin(); it1 != map1.end(); ++it1) { bool found = false; + std::string name1 = it1->first; + std::transform(name1.begin(), name1.end(), name1.begin(), ::tolower); for (it2 = map2.begin(); it2 != map2.end(); ++it2) { - if (it1->first == it2->first && it1->second == it2->second) { + std::string name2 = it2->first; + std::transform(name2.begin(), name2.end(), name2.begin(), ::tolower); + if (name1 == name2 && it1->second == it2->second) { found = true; break; } diff --git a/tests/ceftests/urlrequest_unittest.cc b/tests/ceftests/urlrequest_unittest.cc index 508f70e77..0e1cebf26 100644 --- a/tests/ceftests/urlrequest_unittest.cc +++ b/tests/ceftests/urlrequest_unittest.cc @@ -60,6 +60,7 @@ const char kIncompleteDoNotSendData[] = "DO NOT SEND"; enum RequestTestMode { REQTEST_GET = 0, REQTEST_GET_NODATA, + REQTEST_GET_PARTIAL_CONTENT, REQTEST_GET_ALLOWCOOKIES, REQTEST_GET_REDIRECT, REQTEST_GET_REDIRECT_STOP, @@ -133,6 +134,9 @@ struct RequestRunSettings { // If true download data will be expected. bool expect_download_data = true; + // The offset from what we passed that we expect to receive. + size_t expected_download_offset = 0; + // Expected status value. CefURLRequest::Status expected_status = UR_SUCCESS; @@ -659,7 +663,22 @@ class RequestSchemeHandler : public CefResourceHandler { CefString& redirectUrl) override { EXPECT_IO_THREAD(); GetNormalResponse(settings_, response); - response_length = response_data_.length(); + response_length = response_data_.length() - offset_; + } + + bool Skip(int64 bytes_to_skip, + int64& bytes_skipped, + CefRefPtr callback) override { + size_t size = response_data_.length(); + if (offset_ < size) { + bytes_skipped = + std::min(bytes_to_skip, static_cast(size - offset_)); + offset_ += bytes_skipped; + } else { + bytes_skipped = ERR_FAILED; + } + + return bytes_skipped > 0; } bool Read(void* data_out, @@ -1413,8 +1432,10 @@ class RequestServerHandler : public CefServerHandler { // HEAD requests are identical to GET requests except no response data is // sent. std::string response_data; - if (request->GetMethod() != "HEAD") - response_data = settings->response_data; + if (request->GetMethod() != "HEAD") { + size_t expected_offset = settings->expected_download_offset; + response_data = settings->response_data.substr(expected_offset); + } SendResponse(server, connection_id, response, response_data); } @@ -1644,6 +1665,8 @@ class RequestTestRunner : public base::RefCountedThreadSafe { // Register the test callbacks. REGISTER_TEST(REQTEST_GET, SetupGetTest, SingleRunTest); REGISTER_TEST(REQTEST_GET_NODATA, SetupGetNoDataTest, SingleRunTest); + REGISTER_TEST(REQTEST_GET_PARTIAL_CONTENT, SetupGetPartialContentTest, + SingleRunTest); REGISTER_TEST(REQTEST_GET_ALLOWCOOKIES, SetupGetAllowCookiesTest, SingleRunTest); REGISTER_TEST(REQTEST_GET_REDIRECT, SetupGetRedirectTest, SingleRunTest); @@ -1819,6 +1842,20 @@ class RequestTestRunner : public base::RefCountedThreadSafe { complete_callback.Run(); } + void SetupGetPartialContentTest(const base::Closure& complete_callback) { + // Start with the normal get test. + SetupGetTestShared(); + + // Skip first 4 bytes of content and expect to receive the rest + settings_.request->SetHeaderByName("Range", "bytes=4-", true); + settings_.response->SetHeaderByName("Content-Range", "bytes 4-8/8", true); + settings_.response->SetStatus(206); + settings_.response->SetStatusText("Partial Content"); + settings_.expected_download_offset = 4; + + complete_callback.Run(); + } + void SetupGetAllowCookiesTest(const base::Closure& complete_callback) { // Start with the normal get test. SetupGetTestShared(); @@ -2541,15 +2578,18 @@ class RequestTestRunner : public base::RefCountedThreadSafe { if (settings_.expect_download_progress) { EXPECT_LE(1, client->download_progress_ct_); - EXPECT_EQ((int64)settings_.response_data.size(), client->download_total_); + EXPECT_EQ((int64)(settings_.response_data.size() - + settings_.expected_download_offset), + client->download_total_); } else { EXPECT_EQ(0, client->download_progress_ct_); EXPECT_EQ(0, client->download_total_); } if (settings_.expect_download_data) { + size_t expected_offset = settings_.expected_download_offset; EXPECT_LE(1, client->download_data_ct_); - EXPECT_STREQ(settings_.response_data.c_str(), + EXPECT_STREQ(settings_.response_data.substr(expected_offset).c_str(), client->download_data_.c_str()); } else { EXPECT_EQ(0, client->download_data_ct_); @@ -3367,6 +3407,8 @@ void RegisterURLRequestCustomSchemes( test_server_backend, test_frame_method) \ REQ_TEST(BrowserGETNoData##suffix, REQTEST_GET_NODATA, context_mode, true, \ test_server_backend, test_frame_method) \ + REQ_TEST(BrowserGETPartialContent##suffix, REQTEST_GET_PARTIAL_CONTENT, \ + context_mode, true, test_server_backend, test_frame_method) \ REQ_TEST(BrowserGETAllowCookies##suffix, REQTEST_GET_ALLOWCOOKIES, \ context_mode, true, test_server_backend, test_frame_method) \ REQ_TEST(BrowserGETRedirect##suffix, REQTEST_GET_REDIRECT, context_mode, \