Set Token-Binding HTTP header

net/http/http_network_transaction_unittest.cc

Index: net/http/http_network_transaction_unittest.cc
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index ed7470b3a618ffba8be29837496992d3e2f5ca21..71d71196c7f290984f13df1898645ebabc0770a1 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -21,6 +21,8 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
+#include "base/stl_util.h"
+#include "base/strings/string_number_conversions.h"

[Bence, 2016/01/23 22:31:17]

Sorry, I cannot seem to find what these two includes are for.

[nharper, 2016/01/25 18:54:54]

I don't remember what they were for and it compiles fine without them, so
they're gone now.

 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_file_util.h"
@@ -75,6 +77,7 @@
 #include "net/spdy/spdy_session.h"
 #include "net/spdy/spdy_session_pool.h"
 #include "net/spdy/spdy_test_util_common.h"
+#include "net/ssl/default_channel_id_store.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_config_service.h"
 #include "net/ssl/ssl_config_service_defaults.h"
@@ -14413,6 +14416,12 @@ class FakeStream : public HttpStream,
 
   bool GetRemoteEndpoint(IPEndPoint* endpoint) override { return false; }
 
+  Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
+                                    std::vector<uint8_t>* out) override {
+    ADD_FAILURE();
+    return ERR_NOT_IMPLEMENTED;
+  }
+
   void Drain(HttpNetworkSession* session) override { ADD_FAILURE(); }
 
   void PopulateNetErrorDetails(NetErrorDetails* details) override { return; }
@@ -14644,6 +14653,12 @@ class FakeWebSocketBasicHandshakeStream : public WebSocketHandshakeStreamBase {
 
   bool GetRemoteEndpoint(IPEndPoint* endpoint) override { return false; }
 
+  Error GetSignedEKMForTokenBinding(crypto::ECPrivateKey* key,
+                                    std::vector<uint8_t>* out) override {
+    ADD_FAILURE();
+    return ERR_NOT_IMPLEMENTED;
+  }
+
   void Drain(HttpNetworkSession* session) override { NOTREACHED(); }
 
   void PopulateNetErrorDetails(NetErrorDetails* details) override { return; }
@@ -15802,4 +15817,38 @@ TEST_P(HttpNetworkTransactionTest, DisableNPN) {
   EXPECT_TRUE(trans.server_ssl_config_.npn_protos.empty());
 }
 
+#if !defined(OS_IOS)
+TEST_P(HttpNetworkTransactionTest, TokenBindingSpdy) {
+  const std::string https_url = "https://www.example.com";
+  HttpRequestInfo request;
+  request.url = GURL(https_url);
+  request.method = "GET";
+
+  SSLSocketDataProvider ssl(ASYNC, OK);
+  ssl.token_binding_negotiated = true;
+  ssl.token_binding_key_param = TB_PARAM_ECDSAP256;
+  ssl.SetNextProto(GetProtocol());
+  session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl);
+  scoped_ptr<SpdyFrame> resp(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 1));

[Bence, 2016/01/23 22:31:17]

Optional: add an empty line above this one.

[Bence, 2016/01/23 22:31:17]

Style guide recommends nullptr instead of NULL for new code, see
https://chromium-cpp.appspot.com.  Here and twice below.

[nharper, 2016/01/25 18:54:54]

Done.

[nharper, 2016/01/25 18:54:54]

Done.

+  scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, true));
+  MockRead reads[] = {CreateMockRead(*resp, 1), CreateMockRead(*body, 2),

[Bence, 2016/01/23 22:31:17]

Don't you have to count mock data from 0?  I'd be surprised if this test passed,
I would expect a "index 0 not found" kind of error.

[nharper, 2016/01/25 18:54:54]

I don't know what the sequence number is supposed to be. From looking at a few
of the other tests, I see numbers (starting at 0) being used for both reads and
writes - generally 0 is the first write (e.g. a request). I tried various other
possibilities for the numbers (including using the same sequence number for all
of them, or having the numbers in reverse order), and the test still passed.

[Bence, 2016/01/25 21:45:19]

Well, if the sequence numbers do not seem to matter, could you please omit them?
 I believe both CreateMockRead() and MockRead() has a variant without a sequence
number argument.  Calling them would better reflect the empirical fact that
sequence numbers are irrelevant.

[nharper, 2016/01/25 22:06:17]

Done.

+                      MockRead(ASYNC, ERR_IO_PENDING, 3)};
+  StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0);
+  session_deps_.socket_factory->AddSocketDataProvider(&data);
+  session_deps_.channel_id_service.reset(new ChannelIDService(
+      new DefaultChannelIDStore(NULL), base::ThreadTaskRunnerHandle::Get()));
+  scoped_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));

[Bence, 2016/01/23 22:31:18]

Optional: add an empty line above this one.

[nharper, 2016/01/25 18:54:54]

Done.

+  HttpNetworkTransaction trans(DEFAULT_PRIORITY, session.get());
+  TestCompletionCallback callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            trans.Start(&request, callback.callback(), BoundNetLog()));
+  base::MessageLoop::current()->RunUntilIdle();
+
+  EXPECT_TRUE(trans.GetResponseInfo()->was_fetched_via_spdy);
+  HttpRequestHeaders headers;
+  ASSERT_TRUE(trans.GetFullRequestHeaders(&headers));
+  EXPECT_TRUE(headers.HasHeader(HttpRequestHeaders::kTokenBinding));
+}
+#endif  // !defined(OS_IOS)
+
 }  // namespace net