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

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

Index: chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
diff --git a/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc b/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
index bae78a47da0b66a748cb0200ae8d57948550962d..85287cd860ffeec1e9536743a7807173840dc721 100644
--- a/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
@@ -31,6 +31,7 @@
 #include "net/base/capturing_net_log.h"
 #include "net/base/mock_host_resolver.h"
 #include "net/base/net_util.h"
+#include "net/base/upload_data.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -73,6 +74,33 @@ bool Contains(const Collection& collection, const Key& key) {
       collection.end();
 }
 
+// Tests whether |message| is a valid ExtensionMsg_MessageInvoke, and tries
+// to extract a "postData" section to be passed to onBeforeRequest listeners.
+enum TestMessageResult {
+  kPostDataFound,
+  kNoPostData,
+  kError
+};

[battre, 2012/07/11 12:28:43]

nit: newline

[vabr (Chromium), 2012/07/12 15:13:11]

Done.

+TestMessageResult TestMessage(IPC::Message* message, std::string* post_data) {

[battre, 2012/07/11 12:28:43]

Can you rename this to something more descriptive? E.g. GetPostData()?

[vabr (Chromium), 2012/07/12 15:13:11]

Done.

+  if (message->type() != ExtensionMsg_MessageInvoke::ID) return kError;

[battre, 2012/07/11 12:28:43]

can you move the return statements into the next line?

[battre, 2012/07/11 12:28:43]

I think all cases where you return kError are cases where the input is invalid.
How about using assertions instead? I.e.

ASSERT_TRUE(ExtensionMsg_MessageInvoke::Read(message, &param));
ASSERT_EQ(2, param.c.GetSize());
ASSERT_TRUE(param.c.Get(1, &temp_value));

Then it would be sufficient to return true and false.

[vabr (Chromium), 2012/07/12 15:13:11]

Done. Had to move the return bool to (output) arguments, because ASSERT_* cannot
be used in non-void functions.

[vabr (Chromium), 2012/07/12 15:13:11]

Done.

+  ExtensionMsg_MessageInvoke::Param param;
+  Value* temp_value = NULL;
+  if (!ExtensionMsg_MessageInvoke::Read(message, &param)) return kError;
+  if (param.c.GetSize() != 2) return kError;
+  if (!param.c.Get(1,&temp_value)) return kError;

[battre, 2012/07/11 12:28:43]

nit: space before &

[vabr (Chromium), 2012/07/12 15:13:11]

Done.

+  std::string args;
+  if (!temp_value->GetAsString(&args)) return kError;
+  const char kPostDataHead[] = "\"postData\":{";
+  size_t post_data_start = args.find(kPostDataHead);
+  if (post_data_start == std::string::npos) return kNoPostData;
+  post_data_start += sizeof(kPostDataHead) - 1; //-1 for trailing '\0'

[battre, 2012/07/11 12:28:43]

nit: strlen? 2 spaces before //, 1 space after //

[vabr (Chromium), 2012/07/12 15:13:11]

Done.

+  const size_t post_data_end = args.find("}", post_data_start);
+  if (post_data_end == std::string::npos) return kError;
+  const size_t post_data_length = (post_data_end - 1) - post_data_start;
+  *post_data = std::string(args, post_data_start, post_data_length);
+  return kPostDataFound;
+}
+
 }  // namespace
 
 // A mock event router that responds to events with a pre-arranged queue of
@@ -398,7 +426,169 @@ TEST_F(ExtensionWebRequestTest, SimulateChancelWhileBlocked) {
   ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
       &profile_, extension_id, kEventName + "/1");
   ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
-        &profile_, extension_id, kEventName2 + "/1");
+      &profile_, extension_id, kEventName2 + "/1");
+}
+
+TEST_F(ExtensionWebRequestTest, AccessPostData) {
+  // We verify that POST data are accessible to OnBeforeRequest listeners.
+  // Construct the test data.
+#define kBoundary "THIS_IS_A_BOUNDARY"
+#define kMultipartBytesBlock1 "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"text\"\r\n" \
+    "\r\n" \
+    "test text\r\n" \
+    "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"file\"; filename=\"test\"\r\n" \
+    "Content-Type: application/octet-stream\r\n" \
+    "\r\n"
+#define kMultipartBytesBlock2 "\r\n" \
+    "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"password\"\r\n" \
+    "\r\n" \
+    "test password\r\n" \
+    "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"radio\"\r\n" \
+    "\r\n" \
+    "Yes\r\n" \
+    "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"check\"\r\n" \
+    "\r\n" \
+    "option A\r\n" \
+    "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"check\"\r\n" \
+    "\r\n" \
+    "option B\r\n" \
+    "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"txtarea\"\r\n" \
+    "\r\n" \
+    "Some text.\r\n" \
+    "Other.\r\n" \
+    "\r\n" \
+    "--" kBoundary "\r\n" \
+    "Content-Disposition: form-data; name=\"select\"\r\n" \
+    "\r\n" \
+    "one\r\n" \
+    "--" kBoundary "--"
+  // POST data input.
+  const char kMultipartBytes[] = kMultipartBytesBlock1 kMultipartBytesBlock2;
+  const char kMultipartBytesSplit1[] = kMultipartBytesBlock1;
+  const char kMultipartBytesSplit2[] = kMultipartBytesBlock2;
+  const char kUrlEncodedBytes[] = "text=test+text&file=test-file"
+      "&password=test+password&radio=Yes&check=option+A&check=option+B"
+      "&txtarea=Some+text.%0D%0AOther.%0D%0A&select=one";
+  const char kTextPlainBytes[] = "dummy text";
+  // POST data output.
+  const char kResultMultipart[] = "\"check\":[\"option A\",\"option B\"]," \
+      "\"file\":[\"test\"],\"password\":[\"test password\"]," \
+      "\"radio\":[\"Yes\"],\"select\":[\"one\"],\"text\":[\"test text\"]," \
+      "\"txtarea\":[\"Some text.\\r\\nOther.\\r\\n\"";
+  const char kResultUrlEncoded[] = "\"check\":[\"option+A\",\"option+B\"]," \
+      "\"file\":[\"test-file\"],\"password\":[\"test+password\"]," \
+      "\"radio\":[\"Yes\"],\"select\":[\"one\"],\"text\":[\"test+text\"]," \
+      "\"txtarea\":[\"Some+text.%0D%0AOther.%0D%0A\"";
+  const char* kResults[] =
+      {kResultMultipart, kResultMultipart, kResultUrlEncoded, NULL, NULL};

[battre, 2012/07/11 12:28:43]

Rename this to kExpectedResults?

[vabr (Chromium), 2012/07/12 15:13:11]

Done.

+  // Headers.
+  const char kUrlEncoded[] = "application/x-www-form-urlencoded";
+  const char kTextPlain[] = "text/plain";
+  const char kMultipart[] = "multipart/form-data; boundary=" kBoundary;
+#undef kMultipartBytesBlock2
+#undef kMultipartBytesBlock1
+#undef kBoundary
+
+  // Set up a dummy extension name.
+  ExtensionWebRequestEventRouter::RequestFilter filter;
+  std::string extension_id("1");
+  int extra_info_spec_post =
+      ExtensionWebRequestEventRouter::ExtraInfoSpec::BLOCKING |
+      ExtensionWebRequestEventRouter::ExtraInfoSpec::REQUEST_POST_DATA;
+  int extra_info_spec_no_post =
+      ExtensionWebRequestEventRouter::ExtraInfoSpec::BLOCKING;
+
+  // 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());
+
+  // The request URL can be arbitrary but must have a HTTP or HTTPS scheme.
+  GURL request_url("http://www.example.com");
+
+  // First test: multipart POST data in one lump.
+  net::URLRequest request1(request_url, &delegate_, context_.get());
+  request1.set_method("POST");
+  request1.SetExtraRequestHeaderByName("Content-Type", kMultipart, true);
+  request1.AppendBytesToUpload(kMultipartBytes, strlen(kMultipartBytes));
+  ipc_sender_.PushTask(base::Bind(&base::DoNothing));
+  request1.Start();
+
+  // Second test: multipart POST data in several lumps.
+  net::URLRequest request2(request_url, &delegate_, context_.get());
+  request2.set_method("POST");
+  request2.SetExtraRequestHeaderByName("Content-Type", kMultipart, true);
+  request2.AppendBytesToUpload(kMultipartBytesSplit1,
+                               strlen(kMultipartBytesSplit1));
+  request2.AppendBytesToUpload(kMultipartBytesSplit2,
+                               strlen(kMultipartBytesSplit2));
+  ipc_sender_.PushTask(base::Bind(&base::DoNothing));
+  request2.Start();
+
+  // Third test: URL-encoded POST data.
+  net::URLRequest request3(request_url, &delegate_, context_.get());
+  request3.set_method("POST");
+  request3.SetExtraRequestHeaderByName("Content-Type", kUrlEncoded, true);
+  request3.AppendBytesToUpload(kUrlEncodedBytes, strlen(kUrlEncodedBytes));
+  ipc_sender_.PushTask(base::Bind(&base::DoNothing));
+  request3.Start();
+
+  // Fourth test: text/plain POST data in one lump.
+  net::URLRequest request4(request_url, &delegate_, context_.get());
+  request4.set_method("POST");
+  request4.SetExtraRequestHeaderByName("Content-Type", kTextPlain, true);
+  request4.AppendBytesToUpload(kTextPlainBytes, strlen(kTextPlainBytes));
+  ipc_sender_.PushTask(base::Bind(&base::DoNothing));
+  request4.Start();
+
+  MessageLoop::current()->RunAllPending();
+
+  // 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());
+
+  // Fifth test is the same as the first. Nobody asked for POST data,
+  // so in this case, none should be reported.
+  net::URLRequest request5(request_url, &delegate_, context_.get());
+  request5.set_method("POST");
+  request5.SetExtraRequestHeaderByName("Content-Type", kMultipart, true);
+  request5.AppendBytesToUpload(kMultipartBytes, strlen(kMultipartBytes));
+  ipc_sender_.PushTask(base::Bind(&base::DoNothing));
+  request5.Start();
+
+  MessageLoop::current()->RunAllPending();
+
+  IPC::Message* message = NULL;
+  std::string post_data;
+  TestIPCSender::SentMessages::const_iterator i = ipc_sender_.sent_begin();
+  for (size_t test = 0; test < arraysize(kResults); ++test) {
+    EXPECT_FALSE(i == ipc_sender_.sent_end());

[battre, 2012/07/11 12:28:43]

does EXPECT_NE work? Otherwise, I think it is simpler to understand
EXPECT_TRUE(i != ipc_Sender_.send_end());

[vabr (Chromium), 2012/07/12 15:13:11]

Done.

+    message = (i++)->get();
+    if (kResults[test] == NULL) {
+      EXPECT_EQ(kNoPostData, TestMessage(message, &post_data));
+    } else {
+      EXPECT_EQ(kPostDataFound, TestMessage(message, &post_data));
+      EXPECT_EQ(kResults[test], post_data);
+    }
+  }
+
+  EXPECT_TRUE(i == ipc_sender_.sent_end());
+
+  // Clean-up.
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+      &profile_, extension_id, kEventName + "/1");
 }
 
 struct HeaderModificationTest_Header {
@@ -663,6 +853,10 @@ TEST_F(ExtensionWebRequestTest, InitFromValue) {
       "asyncBlocking",
       true,
       ExtensionWebRequestEventRouter::ExtraInfoSpec::ASYNC_BLOCKING);
+  TestInitFromValue(
+      "requestPostData",
+      true,
+      ExtensionWebRequestEventRouter::ExtraInfoSpec::REQUEST_POST_DATA);
 
   // Multiple valid values are bitwise-or'ed.
   TestInitFromValue(