Network traffic annotation added to SDCH Dictionary Fetcher.

net/url_request/sdch_dictionary_fetcher.cc

 // Copyright 2014 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/sdch_dictionary_fetcher.h"
 
 #include <stdint.h>
 #include <queue>
 #include <set>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
 #include "net/base/sdch_net_log_params.h"
 #include "net/http/http_response_headers.h"
 #include "net/log/net_log_event_type.h"
 #include "net/log/net_log_with_source.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/redirect_info.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_throttler_manager.h"
 
 namespace net {
 
 namespace {
 
 const int kBufferSize = 4096;
@@ skipping 239 matching lines @@
   if (fetch_queue_->IsEmpty() || current_request_.get()) {
     next_state_ = STATE_NONE;
     return OK;
   }
 
   next_state_ = STATE_SEND_REQUEST_PENDING;
 
   FetchInfo info;
   bool success = fetch_queue_->Pop(&info);
   DCHECK(success);
-  current_request_ = context_->CreateRequest(info.url, IDLE, this);
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("...", R"(

[Randy Smith (Not in Mondays), 2017/05/02 15:37:36]

I believe from reading the proto and definitions that this is the unique id, and
hence should be something recognizable and related to where the traffic comes
form, and has no other constraints.  So I'd suggest "sdch_dictionary_fetch".

[Ramin Halavati, 2017/05/03 05:55:40]

Done.

+        semantics {
+          sender: "..."

[Randy Smith (Not in Mondays), 2017/05/02 15:37:35]

So this is tricky and I'm not sure what to do here.  A bit of background: SDCH
fetches are triggered when a "Get-Dictionary" request is received from a server
in the response to *any* URLRequest.  That URLRequest may be renderer initiated,
or initiated from the browser, or something else.  The dictionary is gotten from
the server and stored in memory, advertised on some URLRequests following, and
then used to decode the results of those URLRequest responses.  So that means:
* Any server that is being communicated with by Chrome can cause a dictionary
fetch to be initiated.

* Follow on requests that that dictionary advertises itself as being used for
will be affected by the contents of that dictionary.  

From a security POV, this means that dictionary fetches can be initiated by
arbitrary URLRequests and can affect arbitrary (not really, but it's hard to
reason about) future URLRequests.  On the other hand, I don't *think* there's a
way for them to leak information from the browser; the only information in
making a dictionary request is "some earlier URLRequest asked us to fetch this
dictionary".  So maybe this is ok, or maybe you want to disable SDCH in
environments that have concerns about these issues.

Pulling back to just this line, I suspect you want the "sender" to reflect the
sender of the original URLRequest that triggered this fetch, which would require
a lot of plumbing.  I can guide you in doing that plumbing if you want.

Do note that we recently disabled SDCH in Chrome; it's only enabled for
non-chromium embedders now.  This may mean that you can put something really
scary here and assume it'll never show up in traces; if you want to go that way,
let me know.

[Ramin Halavati, 2017/05/03 05:55:40]

Thank you very much for description, but sender here refers to this module that
is actually making the network request. So what do you think about just 'SDCH'?

[Randy Smith (Not in Mondays), 2017/05/03 12:58:42]

That's fine.

[Ramin Halavati, 2017/05/04 04:36:28]

Done.

+          description: "..."

[Randy Smith (Not in Mondays), 2017/05/02 15:37:35]

The Chrome Network Stack can use less bandwidth and reduce request latency if a
dictionary shared between client and server is used to compress content on the
server before sending and decompress content on the client after reception. 
This request is fetching such a dictionary; it is dispatched when the response
to a previous URL request indicated that there was a dictionary the client did
not currently have in memory that could be used for compression.

[Ramin Halavati, 2017/05/03 05:55:40]

Done.

+          trigger: "..."

[Randy Smith (Not in Mondays), 2017/05/02 15:37:36]

A response to a previous URL request indicated that this URL contained a
dictionary that could be used to compress other URL responses.  

[Ramin Halavati, 2017/05/03 05:55:40]

Done.

+          data: "..."

[Randy Smith (Not in Mondays), 2017/05/02 15:37:36]

The server is sending a dictionary of common 'phrases' (may not be textual),
references to which can be used in future URL request responses, shrinking
response size.

[Ramin Halavati, 2017/05/03 05:55:40]

This field should refer to data that this request sends.
Should it be request for website's dictionary?

[Randy Smith (Not in Mondays), 2017/05/03 12:58:42]

Ah, sorry, data sent to the server.  Yes, the URL of the dictionary, as
specified in a previous response from a server.

+          destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER/LOCAL

[Randy Smith (Not in Mondays), 2017/05/02 15:37:36]

I think WEBSITE is the right answer here, but not that it really depends on what
that nature of the response that triggered this request is.  It'll mostly be
websites, but it might be a google owned service as well.

[Ramin Halavati, 2017/05/03 05:55:40]

So my understanding is Google has no role in compressing data on server side and
the websites creates the dictionary themselves, am I correct?

[Randy Smith (Not in Mondays), 2017/05/03 12:58:42]

Google is one of the websites that has used SDCH, but not the only one (and
we're turning it down, I believe).

[Ramin Halavati, 2017/05/04 04:36:28]

Thank you.

+        }
+        policy {
+          cookies_allowed: false
+          setting: "..."

[Randy Smith (Not in Mondays), 2017/05/02 15:37:36]

Disabled in chrome.  Can only be enabled in non-Chromium embedders.

[Ramin Halavati, 2017/05/03 05:55:40]

Done.

+          chrome_policy {
+            [POLICY_NAME] {
+              policy_options {mode: MANDATORY/RECOMMENDED/UNSET}
+              [POLICY_NAME]: ... //(value to disable it)
+            }
+          }
+          policy_exception_justification: "..."
+        })");
+  current_request_ =
+      context_->CreateRequest(info.url, IDLE, this, traffic_annotation);
   int load_flags = LOAD_DO_NOT_SEND_COOKIES | LOAD_DO_NOT_SAVE_COOKIES;
   if (info.cache_only)
     load_flags |= LOAD_ONLY_FROM_CACHE | LOAD_SKIP_CACHE_VALIDATION;
   current_request_->SetLoadFlags(load_flags);
 
   buffer_ = new IOBuffer(kBufferSize);
   dictionary_.reset(new std::string());
   current_callback_ = info.callback;
 
   current_request_->Start();
@@ skipping 76 matching lines @@
                           current_request_->net_log(),
                           current_request_->was_cached());
   }
 
   ResetRequest();
   next_state_ = STATE_SEND_REQUEST;
   return OK;
 }
 
 }  // namespace net