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..eb9715b5987f8cc2f1cbb521949f494857f6518a 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"
@@ -50,6 +51,7 @@
 #include "net/url_request/url_request.h"
 #include "ui/base/l10n/l10n_util.h"
 
+using chrome::VersionInfo;
 using content::BrowserMessageFilter;
 using content::BrowserThread;
 using content::ResourceRequestInfo;
@@ -77,6 +79,29 @@ static const char* const kWebRequestEvents[] = {
 
 #define ARRAYEND(array) (array + arraysize(array))
 
+static bool g_channel_cached_ = false;
+static VersionInfo::Channel g_channel_ = VersionInfo::CHANNEL_UNKNOWN;
+static bool g_post_data_enabled_ = false;

[wtc, 2012/08/03 00:57:19]

Nit: these three variables (and also the existing
kWebRequestEvents constant above) are inside an anonymous
namespace, so they don't need to be declared 'static'.

Nit: these three variables are not class members, so they
should not have the trailing underscore '_'.

[vabr (Chromium), 2012/08/05 18:54:46]

Done.

+
+bool PostDataEnabledOnChannel() {

[wtc, 2012/08/03 00:57:19]

Nit: This function operates on the global variable g_channel_.
This is not obvious.  I recommend having this function
take the channel as input.

[vabr (Chromium), 2012/08/05 18:54:46]

The function was removed in the meantime.

+  return g_channel_ == VersionInfo::CHANNEL_UNKNOWN ||
+         g_channel_ == VersionInfo::CHANNEL_CANARY ||
+         g_channel_ == VersionInfo::CHANNEL_DEV;

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

note: the channels go in increasing order of stability so that you can do things
like (channel_ <= DEV).

[vabr (Chromium), 2012/08/05 18:54:46]

Good to know! If that's the convention, then the test is much simpler to write.

+}
+
+// PostData feature (crbug.com/91191/) is currently only enabled in dev
+// and canary channels. This function caches the release channel info from
+// chrome::VersionInfo.
+bool IsWebRequestPostDataEnabled() {
+  if (!g_channel_cached_) {
+    // Find out the correct channel and flip |g_post_data_enabled_| accordingly.

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

This comment is fairly obvious from the code. Take a look at the "Dont's"
section of
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Implementation...
.

[vabr (Chromium), 2012/08/05 18:54:46]

Comment removed.

+    g_channel_ = VersionInfo::GetChannel();
+    g_channel_cached_ = true;
+    g_post_data_enabled_  = PostDataEnabledOnChannel();

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

no need to cache this imo

[wtc, 2012/08/03 00:57:19]

I agree that the result of PostDataEnabledOnChannel() is
not worth caching.

Is VersionInfo::GetChannel() expensive?  If not, we don't
need to cache the result of VersionInfo::GetChannel() either.

[vabr (Chromium), 2012/08/05 18:54:46]

With switching to channel <= DEV instead of the three equalities my last
argument for caching disappeared. In addition, due to absence of caching the
"enabled" flag and the simplicity of the channel <= DEV test, I could get rid of
PostDataEnabledOnChannel completely. :)

+  }
+  return g_post_data_enabled_;
+}
+
 bool IsWebRequestEvent(const std::string& event_name) {
   return std::find(kWebRequestEvents, ARRAYEND(kWebRequestEvents),
                    event_name) != ARRAYEND(kWebRequestEvents);
@@ -157,6 +182,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")
+    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::kPostDataFormKey, form_data.release());
+}
+
 // Converts a HttpHeaders dictionary to a |name|, |value| pair. Returns
 // true if successful.
 bool FromHeaderDictionary(const DictionaryValue* header_value,
@@ -251,6 +302,17 @@ void ClearCacheOnNavigationOnUI() {
 
 }  // namespace
 
+// Sets the fake channel |c| and flips |g_post_data_enabled_| accordingly.
+void SetChannelForTestingWebRequest(VersionInfo::Channel channel) {

[wtc, 2012/08/03 00:57:19]

You name the argument |c| in the header file but name it
|channel| here.  The comment on line 305 uses the name |c|.

[vabr (Chromium), 2012/08/05 18:54:46]

Header synced with implementation. Comment removed in the meantime.

+  g_channel_ = channel;
+  g_channel_cached_ = true;
+  g_post_data_enabled_  = PostDataEnabledOnChannel();
+}
+
+void ResetChannelForTestingWebRequest() {
+  g_channel_cached_ = false;
+}
+
 // Represents a single unique listener to an event, along with whatever filter
 // parameters and extra_info_spec were specified at the time the listener was
 // added.
@@ -409,6 +471,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 |= IsWebRequestPostDataEnabled() ? POST_DATA : 0;
     else
       return false;
 
@@ -498,6 +562,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 |=