Add read-only access to POST data for webRequest's onBeforeRequest

chrome/browser/extensions/api/web_request/web_request_api_unittest.cc

 // Copyright (c) 2012 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 <queue>
 #include <map>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/file_util.h"
+#include "base/json/json_reader.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
+#include "base/string_piece.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api_constants.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api_helpers.h"
 #include "chrome/browser/extensions/event_router_forwarder.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
 #include "chrome/browser/prefs/pref_member.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "chrome/common/pref_names.h"
@@ skipping 40 matching lines @@
 }
 
 // Searches |key| in |collection| by iterating over its elements and returns
 // true if found.
 template <typename Collection, typename Key>
 bool Contains(const Collection& collection, const Key& key) {
   return std::find(collection.begin(), collection.end(), key) !=
       collection.end();
 }
 
+// Parses |json_string| as a Value. Returns NULL on failure. The caller owns
+// the returned object.
+const Value* GetValueFromJson(const base::StringPiece& json_string) {
+  const Value* parsed = base::JSONReader::Read(json_string);
+  return parsed;
+}
+
+// Parses |json_string| as a DictionaryValue. Returns NULL on failure. The
+// caller owns the returned object.
+const DictionaryValue* GetDictionaryFromJson(
+    const base::StringPiece& json_string) {
+  const Value* value = GetValueFromJson(json_string);
+  if (value == NULL)
+    return NULL;
+  // Now we own the object pointed to by |value|.
+  const DictionaryValue* dictionary = NULL;
+  if (value->GetAsDictionary(&dictionary)) {
+    // We pass the ownership of the object |value|==|dictionary| to the caller.
+    return dictionary;
+  } else {
+    // The object at |value| is no longer needed.
+    delete value;

[Matt Perry, 2012/08/01 09:46:31]

Prefer using scoped_ptr<Value> for stuff like this. Then the ownership semantics
are conveyed in the code, and you don't need to add comments. i.e.
  return value.release();  (means ownership is passed)
  return NULL;  (means value will be deleted)
  Foo(value.get())  (means ownership stays with us, not Foo)

[vabr (Chromium), 2012/08/01 18:03:22]

Done.

+    return NULL;
+  }
+}
+
+// Parses the JSON data attached to the |message| and tries to return it.
+// Returns NULL on failure.
+scoped_ptr<const DictionaryValue> GetPartOfMessageArguments(
+    IPC::Message* message) {
+  CHECK(message->type() == ExtensionMsg_MessageInvoke::ID);

[Matt Perry, 2012/08/01 09:46:31]

Don't use CHECK in tests. If it fails, you bring down the whole test harness.

Use ASSERT_TRUE, EXPECT_TRUE, or maybe if (!foo) ADD_FAILURE.

[vabr (Chromium), 2012/08/01 18:03:22]

Needed to use EXPECT_*, because ASSERT_* does not work in non-void functions,
and the failures are fatal.
Dominic, earlier we discussed the error reporting here (right after patch set
10). Are you happy with the current changes?

+  ExtensionMsg_MessageInvoke::Param param;
+  Value* temp_value = NULL;
+  CHECK(ExtensionMsg_MessageInvoke::Read(message, &param));
+  CHECK(param.c.GetSize() >= 2);
+  CHECK(param.c.Get(1, &temp_value));
+  std::string args;
+  CHECK(temp_value->GetAsString(&args));
+  const Value* value = GetValueFromJson(args);
+  if (value == NULL) {
+    DLOG(INFO) << "Failed to parse JSON in the message arguments.";

[Matt Perry, 2012/08/01 09:46:31]

Do these messages represent test failures? If so, make them failures with
ADD_FAILURE. If not, don't spam the logs.

[vabr (Chromium), 2012/08/01 18:03:22]

Unified with the redone CHECKs above.

+    return scoped_ptr<const DictionaryValue>();
+  }
+  const ListValue* list = NULL;
+  if (!value->GetAsList(&list)) {
+    DLOG(INFO) << "Parsed message argument is not a ListValue.";
+    return scoped_ptr<const DictionaryValue>();
+  }
+  scoped_ptr<const ListValue> list_scoped(list);  // To ensure clean-up.

[Matt Perry, 2012/08/01 09:46:31]

comment is unnecessary

[vabr (Chromium), 2012/08/01 18:03:22]

Done.

+  if (list->GetSize() != 1) {
+    DLOG(INFO) << "The Listvalue in message arguments has not size 1.";
+    return scoped_ptr<const DictionaryValue>();
+  }
+  // TODO(vabr): Add const once ListValue getters get corrected.

[Matt Perry, 2012/08/01 09:46:31]

Realistically, this will never happen :). Just remove this TODO.

[vabr (Chromium), 2012/08/01 18:03:22]

Oh it will happen, that CL (10837044) is done, review pending. I already did the
same for DictionaryValue last week ;-).

+  DictionaryValue* dictionary = NULL;
+  if (!list->GetDictionary(0, &dictionary)) {
+    DLOG(INFO) << "The only Value in the list in message arguments"
+                  "is not a DictionaryiValue.";
+    return scoped_ptr<const DictionaryValue>();
+  }
+  return scoped_ptr<const DictionaryValue>(dictionary->DeepCopy());
+}
+
 }  // namespace
 
 // A mock event router that responds to events with a pre-arranged queue of
 // Tasks.
 class TestIPCSender : public IPC::Sender {
  public:
   typedef std::list<linked_ptr<IPC::Message> > SentMessages;
 
   // Adds a Task to the queue. We will fire these in order as events are
   // dispatched.
@@ skipping 41 matching lines @@
         prefs::kEnableReferrers, profile_.GetTestingPrefService(), NULL);
     network_delegate_.reset(new ChromeNetworkDelegate(
         event_router_.get(), NULL, NULL, &profile_,
         CookieSettings::Factory::GetForProfile(&profile_), &enable_referrers_,
         NULL));
     context_.reset(new TestURLRequestContext(true));
     context_->set_network_delegate(network_delegate_.get());
     context_->Init();
   }
 
+  // Fires a URLRequest with the specified |content_type|. Method will be "POST"
+  // and the data will be |bytes|.
+  void FireURLRequestWithPostData(const char* content_type,
+                                  const char* bytes, size_t bytes_length);
+
   MessageLoopForIO message_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread io_thread_;
   TestingProfile profile_;
   TestDelegate delegate_;
   BooleanPrefMember enable_referrers_;
   TestIPCSender ipc_sender_;
   scoped_refptr<extensions::EventRouterForwarder> event_router_;
   scoped_refptr<ExtensionInfoMap> extension_info_map_;
   scoped_ptr<ChromeNetworkDelegate> network_delegate_;
@@ skipping 245 matching lines @@
   EXPECT_TRUE(!request.is_pending());
   EXPECT_EQ(net::URLRequestStatus::CANCELED, request.status().status());
   EXPECT_EQ(net::ERR_ABORTED, request.status().error());
   EXPECT_EQ(request_url, request.url());
   EXPECT_EQ(1U, request.url_chain().size());
   EXPECT_EQ(0U, ipc_sender_.GetNumTasks());
 
   ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
       &profile_, extension_id, kEventName + "/1");
   ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
-        &profile_, extension_id, kEventName2 + "/1");
+      &profile_, extension_id, kEventName2 + "/1");
+}
+
+namespace {
+
+// Based on the string with extraInfoSpec |values|, create the numerical
+// representation. Returns true on success, otherwise false.
+bool GenerateInfoSpec(const std::string& values, int* result) {
+  // Create a ListValue of strings.
+  std::vector<std::string> split_values;
+  scoped_ptr<base::ListValue> list_value(new base::ListValue());
+  size_t num_values = Tokenize(values, ",", &split_values);
+  for (size_t i = 0; i < num_values ; ++i)
+    list_value->Append(new base::StringValue(split_values[i]));
+  return ExtensionWebRequestEventRouter::ExtraInfoSpec::InitFromValue(
+      *list_value, result);
+}
+
+}  // namespace
+
+void ExtensionWebRequestTest::FireURLRequestWithPostData(
+    const char* content_type,
+    const char* bytes, size_t bytes_length) {
+  // The request URL can be arbitrary but must have an HTTP or HTTPS scheme.
+  GURL request_url("http://www.example.com");
+  net::URLRequest request(request_url, &delegate_, context_.get());
+  request.set_method("POST");
+  request.SetExtraRequestHeaderByName(
+      net::HttpRequestHeaders::kContentType, content_type, true);
+  request.AppendBytesToUpload(bytes, bytes_length);
+  ipc_sender_.PushTask(base::Bind(&base::DoNothing));
+  request.Start();
+}
+
+TEST_F(ExtensionWebRequestTest, AccessPostData) {
+  // We verify that POST data are accessible to OnBeforeRequest listeners.
+  // Three testing steps:
+  // 1. Register an extension requesting ExtraInfoSpec::POST_DATA and file a
+  //    URLRequest with a multipart-encoded form. See it getting parsed.
+  // 2. Do the same, but without requesting ExtraInfoSpec::POST_DATA. Nothing
+  //    should be parsed.
+  // 3. With ExtraInfoSpec::POST_DATA, fire a URLRequest with a chunked
+  //    upload. Get the error message.
+  // This test is done also for channels other than dev or canary, when
+  // PostData access is not enabled. In that case steps 1 and 3 should give the
+  // same result as step 2.
+#define kBoundary "THIS_IS_A_BOUNDARY"
+#define kBlock "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"text\"\r\n" \
+    "\r\n" \
+    "test text\r\n" \
+    "--" kBoundary "--"
+  // POST data input.
+  const char kMultipartBytes[] = kBlock;
+  // Expected POST data output.
+  const std::string kPostDataPath(keys::kPostDataKey);
+  const std::string kFormDataPath(kPostDataPath + "." + keys::kFormDataKey);
+  const std::string kErrorPath(kPostDataPath + "." + keys::kPostDataErrorKey);
+  const std::string* kPath[] = {&kFormDataPath, &kPostDataPath, &kErrorPath};
+  // formData
+  const char kFormDataString[] = "{\"text\":[\"test text\"]}";
+  scoped_ptr<const DictionaryValue> form_data(
+      GetDictionaryFromJson(kFormDataString));
+  ASSERT_TRUE(form_data.get() != NULL);
+  // error
+  const StringValue error("chunked_encoding");
+  // Summary.
+  const Value* kExpected[] = {form_data.get(), NULL, &error};
+  // Header.
+  const char kMultipart[] = "multipart/form-data; boundary=" kBoundary;
+#undef kBlock
+#undef kBoundary
+
+  // Set up a dummy extension name.
+  ExtensionWebRequestEventRouter::RequestFilter filter;
+  std::string extension_id("1");
+  const std::string string_spec_post("blocking,postData");
+  const std::string string_spec_no_post("blocking");
+  int extra_info_spec_no_post;
+  int extra_info_spec_post;
+  ASSERT_TRUE(GenerateInfoSpec(string_spec_post, &extra_info_spec_post));
+  ASSERT_TRUE(GenerateInfoSpec(string_spec_no_post, &extra_info_spec_no_post));
+
+  // Part 1.
+  // Subscribe to OnBeforeRequest with POST data requirement.
+  const std::string kEventName(keys::kOnBeforeRequest);
+  base::WeakPtrFactory<TestIPCSender> ipc_sender_factory(&ipc_sender_);
+  ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
+      &profile_, extension_id, extension_id, kEventName, kEventName + "/1",
+      filter, extra_info_spec_post, ipc_sender_factory.GetWeakPtr());
+
+  FireURLRequestWithPostData(
+      kMultipart, kMultipartBytes, strlen(kMultipartBytes));
+
+  MessageLoop::current()->RunAllPending();
+
+  // Part 2.
+  // Now remove the requirement of POST data.
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+      &profile_, extension_id, kEventName + "/1");
+  ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
+      &profile_, extension_id, extension_id, kEventName, kEventName + "/1",
+      filter, extra_info_spec_no_post, ipc_sender_factory.GetWeakPtr());
+
+  FireURLRequestWithPostData(
+      kMultipart, kMultipartBytes, strlen(kMultipartBytes));
+
+  // Part 3.
+  // Get back the requirement of POST data.
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+      &profile_, extension_id, kEventName + "/1");
+  ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
+      &profile_, extension_id, extension_id, kEventName, kEventName + "/1",
+      filter, extra_info_spec_post, ipc_sender_factory.GetWeakPtr());
+  {
+    GURL request_url("http://www.example.com");
+    net::URLRequest request(request_url, &delegate_, context_.get());
+    request.set_method("POST");
+    request.EnableChunkedUpload();
+    const char kDummyBytes[] = "dummy";
+    request.AppendChunkToUpload(kDummyBytes, strlen(kDummyBytes), true);
+    net::HttpRequestHeaders headers(request.extra_request_headers());
+    headers.SetHeader(net::HttpRequestHeaders::kTransferEncoding, "chunked");
+    request.SetExtraRequestHeaders(headers);
+    ipc_sender_.PushTask(base::Bind(&base::DoNothing));
+    request.Start();
+  }
+
+  MessageLoop::current()->RunAllPending();
+
+  IPC::Message* message = NULL;
+  TestIPCSender::SentMessages::const_iterator i = ipc_sender_.sent_begin();
+  for (size_t test = 0; test < arraysize(kExpected); ++test) {
+    EXPECT_NE(i, ipc_sender_.sent_end());
+    message = (i++)->get();
+    scoped_ptr<const DictionaryValue>
+        details(GetPartOfMessageArguments(message).Pass());
+    ASSERT_TRUE(details.get() != NULL);
+    const Value* result = NULL;
+    const bool anything_expected = IsPostDataEnabled() &&
+                                   kExpected[test] != NULL;
+    EXPECT_EQ(anything_expected, details->Get(*(kPath[test]), &result));
+    if (anything_expected) {
+      EXPECT_TRUE(kExpected[test]->Equals(result));
+    }
+  }
+
+  EXPECT_EQ(i, ipc_sender_.sent_end());
+
+  // Clean-up.
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+      &profile_, extension_id, kEventName + "/1");
 }
 
 struct HeaderModificationTest_Header {
   const char* name;
   const char* value;
 };
 
 struct HeaderModificationTest_Modification {
   enum Type {
     SET,
@@ skipping 209 matching lines @@
   ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
       &profile_, extension2_id, kEventName + "/2");
   ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
       &profile_, extension3_id, std::string(keys::kOnSendHeaders) + "/3");
 };
 
 namespace {
 
 void TestInitFromValue(const std::string& values, bool expected_return_code,
                        int expected_extra_info_spec) {
-  // Create a ListValue of strings.
-  std::vector<std::string> split_values;
-  scoped_ptr<base::ListValue> list_value(new base::ListValue());
-  size_t num_values = Tokenize(values, ",", &split_values);
-  for (size_t i = 0; i < num_values ; ++i)
-    list_value->Append(new base::StringValue(split_values[i]));
   int actual_info_spec;
-  bool actual_return_code =
-      ExtensionWebRequestEventRouter::ExtraInfoSpec::InitFromValue(
-          *list_value, &actual_info_spec);
+  bool actual_return_code = GenerateInfoSpec(values, &actual_info_spec);
   EXPECT_EQ(expected_return_code, actual_return_code);
   if (expected_return_code)
     EXPECT_EQ(expected_extra_info_spec, actual_info_spec);
 }
 
 }
 TEST_F(ExtensionWebRequestTest, InitFromValue) {
   TestInitFromValue("", true, 0);
 
   // Single valid values.
   TestInitFromValue(
       "requestHeaders",
       true,
       ExtensionWebRequestEventRouter::ExtraInfoSpec::REQUEST_HEADERS);
   TestInitFromValue(
       "responseHeaders",
       true,
       ExtensionWebRequestEventRouter::ExtraInfoSpec::RESPONSE_HEADERS);
   TestInitFromValue(
       "blocking",
       true,
       ExtensionWebRequestEventRouter::ExtraInfoSpec::BLOCKING);
   TestInitFromValue(
       "asyncBlocking",
       true,
       ExtensionWebRequestEventRouter::ExtraInfoSpec::ASYNC_BLOCKING);
+  TestInitFromValue(
+      "postData",
+      IsPostDataEnabled(),
+      ExtensionWebRequestEventRouter::ExtraInfoSpec::POST_DATA);
 
   // Multiple valid values are bitwise-or'ed.
   TestInitFromValue(
       "requestHeaders,blocking",
       true,
       ExtensionWebRequestEventRouter::ExtraInfoSpec::REQUEST_HEADERS |
       ExtensionWebRequestEventRouter::ExtraInfoSpec::BLOCKING);
 
   // Any invalid values lead to a bad parse.
   TestInitFromValue("invalidValue", false, 0);
@@ skipping 803 matching lines @@
   credentials_set = MergeOnAuthRequiredResponses(
         deltas, &auth3, &conflicting_extensions, &net_log);
   EXPECT_TRUE(credentials_set);
   EXPECT_FALSE(auth3.Empty());
   EXPECT_EQ(username, auth1.username());
   EXPECT_EQ(password, auth1.password());
   EXPECT_EQ(1u, conflicting_extensions.size());
   EXPECT_TRUE(ContainsKey(conflicting_extensions, "extid2"));
   EXPECT_EQ(3u, capturing_net_log.GetSize());
 }
-