Add UMAs for checking header values against RFC 7230 in //net

net/url_request/url_request_http_job.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 "net/url_request/url_request_http_job.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 465 matching lines @@
 
 void URLRequestHttpJob::StartTransactionInternal() {
   // This should only be called while the request's status is IO_PENDING.
   DCHECK_EQ(URLRequestStatus::IO_PENDING, request_->status().status());
 
   // NOTE: This method assumes that request_info_ is already setup properly.
 
   // If we already have a transaction, then we should restart the transaction
   // with auth provided by auth_credentials_.
 
+  int invalid_header_values_in_RFC7230 = 0;

[davidben, 2015/12/15 21:16:36]

This should probably be a bool (and then you can break inside the loop when you
set it to true).

[hiroshige, 2015/12/17 06:36:02]

Done.

+  for (net::HttpRequestHeaders::Iterator it(request_info_.extra_headers);

[davidben, 2015/12/15 21:16:36]

Nit: No net:: prefix.

[hiroshige, 2015/12/17 06:36:02]

Done.

+       it.GetNext();) {
+    if (!HttpUtil::IsValidHeaderValueRFC7230(it.value()))
+      ++invalid_header_values_in_RFC7230;
+  }
+  UMA_HISTOGRAM_BOOLEAN("Net.HttpRequestContainsInvalidHeaderValuesInRFC7230",
+                        invalid_header_values_in_RFC7230 > 0);
+
   int rv;
 
   if (network_delegate()) {
     network_delegate()->NotifySendHeaders(
         request_, request_info_.extra_headers);
   }
 
   if (transaction_.get()) {
     rv = transaction_->RestartWithAuth(auth_credentials_, start_callback_);
     auth_credentials_ = AuthCredentials();
@@ skipping 422 matching lines @@
   // If the job is done (due to cancellation), can just ignore this
   // notification.
   if (done_)
     return;
 
   receive_headers_end_ = base::TimeTicks::Now();
 
   // Clear the IO_PENDING status
   SetStatus(URLRequestStatus());
 
+  if (scoped_refptr<HttpResponseHeaders> headers = GetResponseHeaders()) {

[davidben, 2015/12/15 21:16:36]

I don't think we usually declare variables inside conditionals like that.

[hiroshige, 2015/12/17 06:36:02]

Done. Also I moved this to exclude error cases.

+    void* iter = NULL;
+    std::string name;
+    std::string value;
+    int invalid_header_values_in_RFC7230 = 0;

[davidben, 2015/12/15 21:16:36]

Ditto about making this a bool.

[hiroshige, 2015/12/17 06:36:02]

Done.

+    while (headers->EnumerateHeaderLines(&iter, &name, &value)) {
+      if (!HttpUtil::IsValidHeaderValueRFC7230(value))
+        ++invalid_header_values_in_RFC7230;
+    }
+    UMA_HISTOGRAM_BOOLEAN(
+        "Net.HttpResponseContainsInvalidHeaderValuesInRFC7230",
+        invalid_header_values_in_RFC7230 > 0);
+  }
+
   const URLRequestContext* context = request_->context();
 
   if (result == OK) {
     if (transaction_ && transaction_->GetResponseInfo()) {
       SetProxyServer(transaction_->GetResponseInfo()->proxy_server);
     }
     scoped_refptr<HttpResponseHeaders> headers = GetResponseHeaders();
     if (network_delegate()) {
       // Note that |this| may not be deleted until
       // |on_headers_received_callback_| or
@@ skipping 640 matching lines @@
   return override_response_headers_.get() ?
              override_response_headers_.get() :
              transaction_->GetResponseInfo()->headers.get();
 }
 
 void URLRequestHttpJob::NotifyURLRequestDestroyed() {
   awaiting_callback_ = false;
 }
 
 }  // namespace net