Pass raw response with Context-Encoding sdch if support is not configured.

net/url_request/url_request_http_job_unittest.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/url_request/url_request_http_job.h"
 
 #include <stdint.h>
 
 #include <cstddef>
 #include <memory>
@@ skipping 144 matching lines @@
   test_job_interceptor_->set_main_intercept_job(std::move(job));
   request->Start();
 
   base::RunLoop().Run();
   EXPECT_EQ(OK, delegate_.request_status());
   EXPECT_EQ("Test Content", delegate_.data_received());
 }
 
 // Received a malformed SDCH encoded response when there is no SdchManager.
 TEST_F(URLRequestHttpJobSetUpSourceTest,
        SdchNotAdvertisedGotMalformedSdchResponse) {

[xunjieli, 2016/11/29 23:30:42]

Could you change the name of this test to something like:

SdchNotAdvertisedGotSdchResponse ?

[mef, 2016/11/30 15:51:07]

Done.

   MockWrite writes[] = {MockWrite(kSimpleGetMockWrite)};
   MockRead reads[] = {MockRead("HTTP/1.1 200 OK\r\n"
                                "Content-Encoding: sdch\r\n"
                                "Content-Length: 12\r\n\r\n"),
                       MockRead("Test Content")};
 
   StaticSocketDataProvider socket_data(reads, arraysize(reads), writes,
                                        arraysize(writes));
   socket_factory_.AddSocketDataProvider(&socket_data);
 
   // This test expects TestURLRequestContexts to have no SdchManager.
   DCHECK(!context_.sdch_manager());
 
   std::unique_ptr<URLRequest> request = context_.CreateRequest(
       GURL("http://www.example.com"), DEFAULT_PRIORITY, &delegate_);
   std::unique_ptr<TestURLRequestHttpJob> job(
       new TestURLRequestHttpJob(request.get()));
   test_job_interceptor_->set_main_intercept_job(std::move(job));
   request->Start();
 
   base::RunLoop().Run();
-  EXPECT_EQ(ERR_CONTENT_DECODING_INIT_FAILED, delegate_.request_status());
+  // Pass through the raw response tje same way as if received unknown encoding.

[xunjieli, 2016/11/29 23:30:42]

nit: typo in "the"

[mef, 2016/11/30 15:51:07]

Done.

+  EXPECT_EQ(OK, delegate_.request_status());
+  EXPECT_EQ("Test Content", delegate_.data_received());
 }
 
 class URLRequestHttpJobTest : public ::testing::Test {
  protected:
   URLRequestHttpJobTest() : context_(true) {
     context_.set_http_transaction_factory(&network_layer_);
 
     // The |test_job_factory_| takes ownership of the interceptor.
     test_job_interceptor_ = new TestJobInterceptor();
     EXPECT_TRUE(test_job_factory_.SetProtocolHandler(
@@ skipping 571 matching lines @@
         new HttpNetworkSession::Params);
     context_.set_http_network_session_params(std::move(params));
     context_.set_client_socket_factory(&socket_factory_);
     context_.Init();
   }
 
   MockClientSocketFactory socket_factory_;
   TestURLRequestContext context_;
 };
 
+// Received a malformed SDCH encoded response when there is no SdchManager.

[xunjieli, 2016/11/29 23:30:42]

Could you adjust this comment to something like:

// Received a malformed SDCH encoded response that has no valid dictionary id. 

[mef, 2016/11/30 15:51:07]

Done.

+TEST_F(URLRequestHttpJobWithSdchSupportTest,
+       SdchAdvertisedGotMalformedSdchResponse) {
+  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: gzip, deflate, sdch\r\n"
+                "Accept-Language: en-us,fr\r\n\r\n")};
+  MockRead reads[] = {MockRead("HTTP/1.1 200 OK\r\n"
+                               "Content-Encoding: sdch\r\n"
+                               "Content-Length: 12\r\n\r\n"),
+                      MockRead("Test Content")};
+
+  StaticSocketDataProvider socket_data(reads, arraysize(reads), writes,
+                                       arraysize(writes));
+  socket_factory_.AddSocketDataProvider(&socket_data);
+
+  MockSdchObserver sdch_observer;
+  SdchManager sdch_manager;
+  sdch_manager.AddObserver(&sdch_observer);
+  context_.set_sdch_manager(&sdch_manager);
+  TestDelegate delegate;
+  std::unique_ptr<URLRequest> request = context_.CreateRequest(
+      GURL("http://www.example.com"), DEFAULT_PRIORITY, &delegate);
+  request->Start();
+
+  base::RunLoop().Run();
+  // SDCH encoded content is silently passed through if it cannot be decoded.
+  // EXPECT_EQ(ERR_CONTENT_DECODING_INIT_FAILED, delegate.request_status());

[mef, 2016/11/29 22:51:51]

I would've expected an error if SDCH manager is present, but server response
doesn't specify valid dictionary.

Currently it passes through raw response data with or without my change to
URLRequestHttpJob::SetUpSourceStream(), so I don't think that this CL masks any
errors.

[xunjieli, 2016/11/29 23:30:42]

This is expected. SdchPolicyDelegate::OnDictionaryIdError() once detects that
the response is malformed, it will issue a pass-through, so we end up receiving
the raw response.

As you can see here, we already masking quite a few errors in sdch, so this
won't be a precedent. The one that I mentioned in my previous CL is about Cronet
clients changing Accept-Encodings, which I think can be problematic depending on
what exactly Cronet clients do. 

[mef, 2016/11/30 15:51:07]

Acknowledged. Thanks for explanation!

+  // Pass through the raw response tje same way as if received unknown encoding.

[xunjieli, 2016/11/29 23:30:42]

Could you remove this line and the line above?
Maybe mention the fact that the malformed response is passed through undecoded.

[mef, 2016/11/30 15:51:07]

Done.

+  EXPECT_EQ(OK, delegate.request_status());
+  EXPECT_EQ("Test Content", delegate.data_received());
+  // Cleanup manager.
+  sdch_manager.RemoveObserver(&sdch_observer);
+}
+
 TEST_F(URLRequestHttpJobWithSdchSupportTest, GetDictionary) {
   MockWrite writes[] = {
       MockWrite("GET / HTTP/1.1\r\n"
                 "Host: example.com\r\n"
                 "Connection: keep-alive\r\n"
                 "User-Agent:\r\n"
                 "Accept-Encoding: gzip, deflate, sdch\r\n"
                 "Accept-Language: en-us,fr\r\n\r\n")};
 
   MockRead reads[] = {MockRead("HTTP/1.1 200 OK\r\n"
@@ skipping 268 matching lines @@
   base::RunLoop().RunUntilIdle();
   EXPECT_THAT(delegate_.request_status(), IsError(ERR_IO_PENDING));
   EXPECT_TRUE(fake_handshake_stream->initialize_stream_was_called());
 }
 
 #endif  // BUILDFLAG(ENABLE_WEBSOCKETS)
 
 }  // namespace
 
 }  // namespace net