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

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

Index: chrome/browser/extensions/api/web_request/web_request_api.cc
diff --git a/chrome/browser/extensions/api/web_request/web_request_api.cc b/chrome/browser/extensions/api/web_request/web_request_api.cc
index 7c2f278321dc8eb4887b0322a8fb6847a315f6e0..e3b3995c751fc1df9afcce0a92a6ed1b34dcc1c5 100644
--- a/chrome/browser/extensions/api/web_request/web_request_api.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_api.cc
@@ -19,6 +19,7 @@
 #include "chrome/browser/extensions/api/declarative_webrequest/webrequest_rule.h"
 #include "chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.h"
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.h"
+#include "chrome/browser/extensions/api/web_request/post_data_parser.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/api/web_request/web_request_time_tracker.h"
@@ -31,6 +32,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/renderer_host/chrome_render_message_filter.h"
 #include "chrome/browser/renderer_host/web_cache_manager.h"
+#include "chrome/common/chrome_version_info.h"
 #include "chrome/common/extensions/event_filtering_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -157,6 +159,32 @@ void ExtractRequestInfo(net::URLRequest* request, DictionaryValue* out) {
   out->SetDouble(keys::kTimeStampKey, base::Time::Now().ToDoubleT() * 1000);
 }
 
+// Extracts the POST data from |request| and writes the data into |out|.
+// This can be expensive, so it's separated from ExtractRequestInfo().
+void ExtractRequestInfoPost(const net::URLRequest* request,
+                            DictionaryValue* out) {
+  if (request->method() != "POST")

[wtc, 2012/08/01 17:24:41]

If you want to support the PUT method, add
  && request->method() != "PUT"
here.

+    return;  // Need to exit without "out->Set(keys::kPostDataKey, ...);" .
+
+  DictionaryValue* post_data = new DictionaryValue();
+  out->Set(keys::kPostDataKey, post_data);
+
+  // Chunked encoding (defined in RFC 2616) is currently not supported.
+  std::string transfer_encoding;
+  if (request->extra_request_headers().GetHeader(
+      net::HttpRequestHeaders::kTransferEncoding, &transfer_encoding)) {
+    if (base::strcasecmp(transfer_encoding.c_str(), "chunked") == 0) {
+      post_data->SetString(keys::kPostDataErrorKey, "chunked_encoding");
+      return;
+    }
+  }
+
+  scoped_ptr<DictionaryValue> form_data =
+      extensions::PostDataParser::ParseURLRequestData(request);
+  if (form_data.get() != NULL)
+    post_data->Set(keys::kFormDataKey, form_data.release());
+}
+
 // Converts a HttpHeaders dictionary to a |name|, |value| pair. Returns
 // true if successful.
 bool FromHeaderDictionary(const DictionaryValue* header_value,
@@ -409,6 +437,8 @@ bool ExtensionWebRequestEventRouter::ExtraInfoSpec::InitFromValue(
       *extra_info_spec |= BLOCKING;
     else if (str == "asyncBlocking")
       *extra_info_spec |= ASYNC_BLOCKING;
+    else if (str == "postData")
+      *extra_info_spec |= (IsPostDataEnabled()) ? POST_DATA : 0;

[wtc, 2012/08/01 17:24:41]

Nit: you can also write this as

  else if (str == "postData" && IsPostDataEnabled())
    *extra_info_spec |= POST_DATA;

[vabr (Chromium), 2012/08/02 09:15:15]

Actually, that would be different. The current code means that "postData" is
silently ignored in beta/stable, because InitFromValue returns true. If I did it
as suggested, asking for postData in beta/stable would cause InitFromValue to
return false, and result in an error during registering the listener.

Dominic prefers the silent ignoring, so that extensions experimenting with
postData on dev are not broken on beta/stable (of course they will still not be
able to use POST data, but at least their listeners will get registered for
doing other stuff).

Matt, do you have an opinion on this?

[Matt Perry, 2012/08/02 10:11:03]

I'm torn, so let's just go with what you have.

[wtc, 2012/08/02 21:58:49]

You're right.  My suggested change was wrong.  Please ignore it.

     else
       return false;
 
@@ -498,6 +528,8 @@ int ExtensionWebRequestEventRouter::OnBeforeRequest(
     ListValue args;
     DictionaryValue* dict = new DictionaryValue();
     ExtractRequestInfo(request, dict);
+    if (extra_info_spec & ExtraInfoSpec::POST_DATA)
+      ExtractRequestInfoPost(request, dict);
     args.Append(dict);
 
     initialize_blocked_requests |=
@@ -1827,3 +1859,16 @@ void SendExtensionWebRequestStatusToHost(content::RenderProcessHost* host) {
 
   host->Send(new ExtensionMsg_UsingWebRequestAPI(adblock, adblock_plus, other));
 }
+
+bool IsPostDataEnabled() {
+  static bool cached = false;
+  static bool enabled = false;
+  if (!cached) {
+    chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();

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

channel is a POD (it's just an enum), so you can make it a static variable.
Doing

bool IsEnabled() {
  static chrome::VersionInfo::Channel channel =
chrome::VersionInfo::GetChannel();
  return channel <= Dev;
}

works fine, because GetChannel() will only be invoked the first time this method
is called.

[wtc, 2012/08/01 17:24:41]

mpcomplete's suggestion is correct.  Note that neither form
is thread-safe, so you need to document that IsPostDataEnabled()
can only be called on a single thread, unless the single-thread
requirement is obvious to people using this function.

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

Thanks, I learned something new! :)

However, due to adding the channel switcher, those static variables had to go
out of the function, where non-constant initialisation is forbidden by the style
guide even by POD, so this is no more relevant.

+    enabled = channel == chrome::VersionInfo::CHANNEL_UNKNOWN ||
+              channel == chrome::VersionInfo::CHANNEL_CANARY ||
+              channel == chrome::VersionInfo::CHANNEL_DEV;
+    cached = true;
+  }
+  return enabled;
+}