Add a histogram for Vary response header in pushed HTTP/2 streams.

This is to assess if pushed responses with Vary header could be rejected
(if proportion is very-very low).  See public discussion at
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/5_aP_stqndw.

Also add unittests.

Bug: 554220
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I8c5c95fc12da056fed7f837c79419e969f991a35
Reviewed-on: https://chromium-review.googlesource.com/755093Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516094}

net/spdy/chromium/spdy_session.cc

@@ -18,6 +18,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -77,6 +78,56 @@ const uint32_t kDefaultInitialMaxFrameSize = 16384;
 // The maximum size of header list that the server is allowed to send.
 const uint32_t kSpdyMaxHeaderListSize = 256 * 1024;
 
+// Values of Vary response header on pushed streams.  This is logged to
+// Net.PushedStreamVaryResponseHeader, entries must not be changed.
+enum PushedStreamVaryResponseHeaderValues {
+  // There is no Vary header.
+  kNoVaryHeader = 0,
+  // The value of Vary is empty.
+  kVaryIsEmpty = 1,
+  // The value of Vary is "*".
+  kVaryIsStar = 2,
+  // The value of Vary is "accept-encoding" (case insensitive).
+  kVaryIsAcceptEncoding = 3,
+  // The value of Vary contains "accept-encoding" (case insensitive) and some
+  // other field names as well.
+  kVaryHasAcceptEncoding = 4,
+  // The value of Vary does not contain "accept-encoding", is not empty, and is
+  // not "*".
+  kVaryHasNoAcceptEncoding = 5,
+  // The number of entries above.
+  kNumberOfVaryEntries = 6
+};
+
+// String literals for parsing the Vary header in a pushed response.
+const char kVary[] = "vary";
+const char kStar[] = "*";
+const char kAcceptEncoding[] = "accept-encoding";
+
+enum PushedStreamVaryResponseHeaderValues ParseVaryInPushedResponse(
+    const SpdyHeaderBlock& headers) {
+  SpdyHeaderBlock::iterator it = headers.find(kVary);
+  if (it == headers.end())
+    return kNoVaryHeader;
+  base::StringPiece value(it->second);
+  if (value.empty())
+    return kVaryIsEmpty;
+  if (value == kStar)
+    return kVaryIsStar;
+  std::string lowercase_value = ToLowerASCII(value);
+  if (lowercase_value == kAcceptEncoding)
+    return kVaryIsAcceptEncoding;
+  // Both comma and newline delimiters occur in the wild.
+  for (const auto& substr :
+       SplitString(lowercase_value, ",\n", base::TRIM_WHITESPACE,
+                   base::SPLIT_WANT_NONEMPTY)) {
+    if (substr == kAcceptEncoding)
+      return kVaryHasAcceptEncoding;
+  }
+
+  return kVaryHasNoAcceptEncoding;
+}
+
 bool IsSpdySettingAtDefaultInitialValue(SpdySettingsIds setting_id,
                                         uint32_t value) {
   switch (setting_id) {
@@ -2429,6 +2480,14 @@ void SpdySession::RecordProtocolErrorHistogram(
   }
 }
 
+// static
+void SpdySession::RecordPushedStreamVaryResponseHeaderHistogram(
+    const SpdyHeaderBlock& headers) {
+  UMA_HISTOGRAM_ENUMERATION("Net.PushedStreamVaryResponseHeader",
+                            ParseVaryInPushedResponse(headers),
+                            kNumberOfVaryEntries);
+}
+
 void SpdySession::DcheckGoingAway() const {
 #if DCHECK_IS_ON()
   DCHECK_GE(availability_state_, STATE_GOING_AWAY);
@@ -2909,6 +2968,9 @@ void SpdySession::OnHeaders(SpdyStreamId stream_id,
   SpdyStream* stream = it->second;
   CHECK_EQ(stream->stream_id(), stream_id);
 
+  if (stream->type() == SPDY_PUSH_STREAM)
+    RecordPushedStreamVaryResponseHeaderHistogram(headers);
+
   stream->AddRawReceivedBytes(last_compressed_frame_len_);
   last_compressed_frame_len_ = 0;
 

net/spdy/chromium/spdy_session.h

@@ -553,6 +553,7 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
                            ServerPushValidCrossOrigin);
   FRIEND_TEST_ALL_PREFIXES(SpdyNetworkTransactionTest,
                            ServerPushValidCrossOriginWithOpenSession);
+  FRIEND_TEST_ALL_PREFIXES(RecordPushedStreamHistogramTest, VaryResponseHeader);
 
   using PendingStreamRequestQueue =
       base::circular_deque<base::WeakPtr<SpdyStreamRequest>>;
@@ -809,6 +810,8 @@ class NET_EXPORT SpdySession : public BufferedSpdyFramerVisitorInterface,
   void RecordPingRTTHistogram(base::TimeDelta duration);
   void RecordHistograms();
   void RecordProtocolErrorHistogram(SpdyProtocolErrorDetails details);
+  static void RecordPushedStreamVaryResponseHeaderHistogram(
+      const SpdyHeaderBlock& headers);
 
   // DCHECKs that |availability_state_| >= STATE_GOING_AWAY, that
   // there are no pending stream creation requests, and that there are

net/spdy/chromium/spdy_session_unittest.cc

@@ -6264,4 +6264,40 @@ TEST(CanPoolTest, CanPoolWithAcceptablePins) {
       &tss, ssl_info, "www.example.org", "mail.example.org"));
 }
 
+TEST(RecordPushedStreamHistogramTest, VaryResponseHeader) {
+  struct {
+    size_t num_headers;
+    const char* headers[2];
+    int expected_bucket;
+  } test_cases[] = {{0, {}, 0},
+                    {1, {"foo", "bar"}, 0},
+                    {1, {"vary", ""}, 1},
+                    {1, {"vary", "*"}, 2},
+                    {1, {"vary", "accept-encoding"}, 3},
+                    {1, {"vary", "foo , accept-encoding ,bar"}, 4},
+                    {1, {"vary", "\taccept-encoding, foo"}, 4},
+                    {1, {"vary", "foo"}, 5},
+                    {1, {"vary", "fooaccept-encoding"}, 5},
+                    {1, {"vary", "foo, accept-encodingbar"}, 5}};
+
+  for (size_t i = 0; i < arraysize(test_cases); ++i) {
+    SpdyHeaderBlock headers;
+    for (size_t j = 0; j < test_cases[i].num_headers; ++j) {
+      headers[test_cases[i].headers[2 * j]] = test_cases[i].headers[2 * j + 1];
+    }
+    base::HistogramTester histograms;
+    histograms.ExpectTotalCount("Net.PushedStreamVaryResponseHeader", 0);
+    SpdySession::RecordPushedStreamVaryResponseHeaderHistogram(headers);
+    histograms.ExpectTotalCount("Net.PushedStreamVaryResponseHeader", 1);
+    histograms.ExpectBucketCount("Net.PushedStreamVaryResponseHeader",
+                                 test_cases[i].expected_bucket, 1);
+    // Adding an unrelated header field should not change how Vary is parsed.
+    headers["foo"] = "bar";
+    SpdySession::RecordPushedStreamVaryResponseHeaderHistogram(headers);
+    histograms.ExpectTotalCount("Net.PushedStreamVaryResponseHeader", 2);
+    histograms.ExpectBucketCount("Net.PushedStreamVaryResponseHeader",
+                                 test_cases[i].expected_bucket, 2);
+  }
+}
+
 }  // namespace net

tools/metrics/histograms/enums.xml

@@ -33817,6 +33817,21 @@ Called by update_net_error_codes.py.-->
   <int value="7" label="Service Worker timeout while processing event"/>
 </enum>
 
+<enum name="PushedStreamVaryResponseHeaderValues">
+  <int value="0" label="There is no Vary header."/>
+  <int value="1" label="The value of Vary is empty."/>
+  <int value="2" label="The value of Vary is &quot;*&quot;."/>
+  <int value="3"
+      label="The value of Vary is &quot;accept-encoding&quot; (case
+             insensitive)."/>
+  <int value="4"
+      label="The value of Vary contains &quot;accept-encoding&quot; (case
+             insensitive) and some other field names as well."/>
+  <int value="5"
+      label="The value of Vary does not contain &quot;accept-encoding&quot;,
+             is not empty, and is not &quot;*&quot;."/>
+</enum>
+
 <enum name="PushGetRegistrationStatus">
   <int value="0" label="Successful"/>
   <int value="1" label="No push service"/>

tools/metrics/histograms/histograms.xml

@@ -43656,6 +43656,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Net.PushedStreamVaryResponseHeader"
+    enum="PushedStreamVaryResponseHeaderValues">
+  <owner>bnc@chromium.org</owner>
+  <summary>
+    Information about the value of the Vary response header in HTTP/2 pushed
+    streams.
+  </summary>
+</histogram>
+
 <histogram name="Net.QuicActiveSessions">
   <owner>rch@chromium.org</owner>
   <summary>