Return false when trusted SPDY proxy advertises HTTPS URL

net/spdy/spdy_session_unittest.cc

Index: net/spdy/spdy_session_unittest.cc
diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc
index ea8ae113a0cb735a561a57041c5a8ac44d5b8825..ab53d11663256c4a8081aa34ec9087d4eba23c7e 100644
--- a/net/spdy/spdy_session_unittest.cc
+++ b/net/spdy/spdy_session_unittest.cc
@@ -4842,6 +4842,156 @@ TEST_P(SpdySessionTest, RejectPushedStreamExceedingConcurrencyLimit) {
   EXPECT_FALSE(session_);
 }
 
+// Tests that HTTP SPDY push streams that advertise an origin different from the
+// associated stream are accepted from a trusted SPDY proxy.
+TEST_P(SpdySessionTest, TrustedSpdyProxy) {
+  const std::string kAnotherURL = "example2.org";
+  // kDefaultURL should not contain kAnotherURL.
+  ASSERT_TRUE(GURL(kDefaultURL).host().find(kAnotherURL) == std::string::npos);
+
+  // Push resources for kAnotherURL which is different from kDefaultURL.

[bengr, 2016/01/08 18:57:27]

This doesn't make sense. Maybe call kAnotherURL kAnotherOrigin if you are going
to use constants.

[tbansal1, 2016/01/08 22:41:34]

Done.

+  // cross_origin_push is HTTP, and should be accepted. cross_origin_https_push
+  // is HTTPS, and should not be accepted.
+  scoped_ptr<SpdyFrame> cross_origin_push(spdy_util_.ConstructSpdyPush(
+      nullptr, 0, 2, 1, ("http://www." + kAnotherURL + "/a.dat").c_str()));

[bengr, 2016/01/08 18:57:27]

My earlier comment was mainly about the example1 URL and these example2 URLs
being defined in different places. It might be cleanest to use string literals
for all URLs and hosts in these tests, instead of string constants that are
defined all over the place.

[bengr, 2016/01/08 19:06:35]

Actually, if kDefaultURL is required by the test harness, I'd just add:

const char[] kHttpURLFromAnotherOrigin = "http://www.example2.com/a.dat";
const char[] kHttpsURLFromAnotherOrigin = ...

and extract the origin from one of these as needed.

[tbansal1, 2016/01/08 22:41:34]

Done.

+  // cross_origin_https_push is HTTPS, and should be refused.
+  scoped_ptr<SpdyFrame> cross_origin_https_push(spdy_util_.ConstructSpdyPush(
+      nullptr, 0, 4, 1, ("https://www." + kAnotherURL + "/b.dat").c_str()));
+  MockRead reads[] = {
+      MockRead(ASYNC, ERR_IO_PENDING, 1),
+      CreateMockRead(*cross_origin_push, 2),
+      MockRead(ASYNC, ERR_IO_PENDING, 3),
+      CreateMockRead(*cross_origin_https_push, 4),
+      MockRead(ASYNC, ERR_IO_PENDING, 6),
+      MockRead(ASYNC, 0, 7),
+  };
+
+  scoped_ptr<SpdyFrame> request(
+      spdy_util_.ConstructSpdyGet(nullptr, 0, false, 1, LOWEST, true));
+  scoped_ptr<SpdyFrame> reset(
+      spdy_util_.ConstructSpdyRstStream(4, RST_STREAM_REFUSED_STREAM));
+  MockWrite writes[] = {
+      CreateMockWrite(*request, 0), CreateMockWrite(*reset, 5),
+  };
+
+  SequencedSocketData data(reads, arraysize(reads), writes, arraysize(writes));
+  session_deps_.socket_factory->AddSocketDataProvider(&data);
+  session_deps_.trusted_spdy_proxy =
+      HostPortPair::FromURL(GURL(kDefaultURL)).ToString();
+
+  CreateNetworkSession();
+  CreateInsecureSpdySession();
+
+  base::WeakPtr<SpdyStream> spdy_stream = CreateStreamSynchronously(
+      SPDY_REQUEST_RESPONSE_STREAM, session_, test_url_, LOWEST, BoundNetLog());
+  ASSERT_TRUE(spdy_stream.get() != nullptr);
+  EXPECT_EQ(0u, spdy_stream->stream_id());
+  test::StreamDelegateDoNothing delegate(spdy_stream);
+  spdy_stream->SetDelegate(&delegate);
+
+  EXPECT_EQ(0u, session_->num_active_streams());
+  EXPECT_EQ(1u, session_->num_created_streams());
+  EXPECT_EQ(0u, session_->num_pushed_streams());
+  EXPECT_EQ(0u, session_->num_active_pushed_streams());
+
+  scoped_ptr<SpdyHeaderBlock> headers(
+      spdy_util_.ConstructGetHeaderBlock(kDefaultURL));
+  spdy_stream->SendRequestHeaders(std::move(headers), NO_MORE_DATA_TO_SEND);
+  EXPECT_TRUE(spdy_stream->HasUrlFromHeaders());
+
+  // Run until 1st stream is activated.
+  EXPECT_EQ(0u, delegate.stream_id());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1u, delegate.stream_id());
+  EXPECT_EQ(1u, session_->num_active_streams());
+  EXPECT_EQ(0u, session_->num_created_streams());
+  EXPECT_EQ(0u, session_->num_pushed_streams());
+  EXPECT_EQ(0u, session_->num_active_pushed_streams());
+
+  // Run until pushed stream is created.
+  data.Resume();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(2u, session_->num_active_streams());
+  EXPECT_EQ(0u, session_->num_created_streams());
+  EXPECT_EQ(1u, session_->num_pushed_streams());
+  EXPECT_EQ(1u, session_->num_active_pushed_streams());
+
+  // Reset incoming pushed stream.
+  data.Resume();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(2u, session_->num_active_streams());
+  EXPECT_EQ(0u, session_->num_created_streams());
+  EXPECT_EQ(1u, session_->num_pushed_streams());
+  EXPECT_EQ(1u, session_->num_active_pushed_streams());
+
+  // Read EOF.
+  data.Resume();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(session_);
+}
+
+// Tests that if the SPDY trusted proxy is not set, then push streams that
+// advertise an origin different from the associated stream are refused.
+TEST_P(SpdySessionTest, TrustedSpdyProxyNotSet) {
+  const std::string kAnotherURL = "example2.org";
+  // kDefaultURL should not contain kAnotherURL.
+  ASSERT_TRUE(GURL(kDefaultURL).host().find(kAnotherURL) == std::string::npos);
+
+  // cross_origin_push contains resource for an origin different from
+  // kDefaultURL, and should be refused.
+  scoped_ptr<SpdyFrame> cross_origin_push(spdy_util_.ConstructSpdyPush(
+      nullptr, 0, 2, 1, ("http://www." + kAnotherURL + "/a.dat").c_str()));
+  MockRead reads[] = {
+      MockRead(ASYNC, ERR_IO_PENDING, 1), CreateMockRead(*cross_origin_push, 2),
+      MockRead(ASYNC, 0, 4),
+  };
+
+  scoped_ptr<SpdyFrame> req(
+      spdy_util_.ConstructSpdyGet(nullptr, 0, false, 1, LOWEST, true));
+  scoped_ptr<SpdyFrame> rst(
+      spdy_util_.ConstructSpdyRstStream(2, RST_STREAM_REFUSED_STREAM));
+  MockWrite writes[] = {
+      CreateMockWrite(*req, 0), CreateMockWrite(*rst, 3),
+  };
+
+  SequencedSocketData data(reads, arraysize(reads), writes, arraysize(writes));
+  session_deps_.socket_factory->AddSocketDataProvider(&data);
+
+  CreateNetworkSession();
+  CreateInsecureSpdySession();
+
+  base::WeakPtr<SpdyStream> spdy_stream = CreateStreamSynchronously(
+      SPDY_REQUEST_RESPONSE_STREAM, session_, test_url_, LOWEST, BoundNetLog());
+  ASSERT_TRUE(spdy_stream.get() != nullptr);
+  EXPECT_EQ(0u, spdy_stream->stream_id());
+  test::StreamDelegateDoNothing delegate(spdy_stream);
+  spdy_stream->SetDelegate(&delegate);
+
+  EXPECT_EQ(0u, session_->num_active_streams());
+  EXPECT_EQ(1u, session_->num_created_streams());
+  EXPECT_EQ(0u, session_->num_pushed_streams());
+  EXPECT_EQ(0u, session_->num_active_pushed_streams());
+
+  scoped_ptr<SpdyHeaderBlock> headers(
+      spdy_util_.ConstructGetHeaderBlock(kDefaultURL));
+  spdy_stream->SendRequestHeaders(std::move(headers), NO_MORE_DATA_TO_SEND);
+  EXPECT_TRUE(spdy_stream->HasUrlFromHeaders());
+
+  // Run until 1st stream is activated.
+  EXPECT_EQ(0u, delegate.stream_id());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1u, delegate.stream_id());
+  EXPECT_EQ(1u, session_->num_active_streams());
+  EXPECT_EQ(0u, session_->num_created_streams());
+  EXPECT_EQ(0u, session_->num_pushed_streams());
+  EXPECT_EQ(0u, session_->num_active_pushed_streams());
+
+  // Read EOF.
+  data.Resume();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(session_);
+}
+
 TEST_P(SpdySessionTest, IgnoreReservedRemoteStreamsCount) {
   // Streams in reserved remote state exist only in HTTP/2.
   if (spdy_util_.spdy_version() < HTTP2)