QUIC - chromium server push support.

net/quic/quic_http_stream.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/quic/quic_http_stream.h"
 
 #include "base/callback_helpers.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_util.h"
 #include "net/quic/quic_chromium_client_session.h"
 #include "net/quic/quic_chromium_client_stream.h"
+#include "net/quic/quic_client_promised_info.h"
 #include "net/quic/quic_http_utils.h"
 #include "net/quic/quic_utils.h"
 #include "net/quic/spdy_utils.h"
 #include "net/socket/next_proto.h"
 #include "net/spdy/spdy_frame_builder.h"
 #include "net/spdy/spdy_framer.h"
 #include "net/spdy/spdy_http_utils.h"
 #include "net/ssl/ssl_info.h"
 
 namespace net {
@@ skipping 10 matching lines @@
       priority_(MINIMUM_PRIORITY),
       response_info_(nullptr),
       response_status_(OK),
       response_headers_received_(false),
       headers_bytes_received_(0),
       headers_bytes_sent_(0),
       closed_stream_received_bytes_(0),
       closed_stream_sent_bytes_(0),
       user_buffer_len_(0),
       quic_connection_error_(QUIC_NO_ERROR),
+      found_promise_(false),
+      push_handle_(nullptr),
       weak_factory_(this) {
   DCHECK(session_);
   session_->AddObserver(this);
 }
 
 QuicHttpStream::~QuicHttpStream() {
   Close(false);
   if (session_)
     session_->RemoveObserver(this);
 }
 
+void QuicHttpStream::ConvertHeaderBlockToHttpRequestHeaders(
+    const SpdyHeaderBlock& spdy_headers,
+    HttpRequestHeaders* http_headers) {
+  for (auto it : spdy_headers) {
+    StringPiece key = it.first;
+    if (key[0] == ':') {
+      key.remove_prefix(1);
+    }
+    std::vector<StringPiece> values = base::SplitStringPiece(
+        it.second, "\0", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+    for (auto value : values) {
+      http_headers->SetHeader(key, value);
+    }
+  }
+}
+
+bool QuicHttpStream::CheckVary(const SpdyHeaderBlock& client_request,
+                               const SpdyHeaderBlock& promise_request,
+                               const SpdyHeaderBlock& promise_response) {
+  HttpResponseInfo promise_response_info;
+
+  HttpRequestInfo promise_request_info;
+  ConvertHeaderBlockToHttpRequestHeaders(promise_request,
+                                         &promise_request_info.extra_headers);
+  HttpRequestInfo client_request_info;
+  ConvertHeaderBlockToHttpRequestHeaders(client_request,
+                                         &client_request_info.extra_headers);
+
+  if (!SpdyHeadersToHttpResponse(promise_response, HTTP2,
+                                 &promise_response_info)) {
+    DLOG(WARNING) << "Invalid headers";
+    return false;
+  }
+
+  HttpVaryData vary_data;
+  if (vary_data.Init(promise_request_info,
+                     *promise_response_info.headers.get())) {
+    // Now compare the client request for matching.
+    return vary_data.MatchesRequest(client_request_info,
+                                    *promise_response_info.headers.get());
+  } else {
+    // Promise didn't contain valid vary info, so URL match was sufficient.
+    return true;

[Ryan Hamilton, 2016/02/24 00:34:00]

nit: early return

if (!vary_data.Init(...)) {
  // Promise didn't contain valid vary info, so URL match was sufficient.
  return true;
}

// Now compare the client request for matching.
return vary_data.MatchesRequest

Or you could be really clever and do:
  return !var_data.Init() || vary_data.Match()
but I suspect that won't be terribly readable

[Buck, 2016/02/26 23:54:16]

Done.

+  }
+}
+
+void QuicHttpStream::OnRendezvousResult(QuicSpdyStream* stream) {
+  push_handle_ = nullptr;
+  if (stream) {
+    stream_ = static_cast<QuicChromiumClientStream*>(stream);
+    stream_->SetDelegate(this);
+  }
+  if (!callback_.is_null()) {

[Ryan Hamilton, 2016/02/24 00:34:00]

Can you comment about under what circumstances will the callback be non-null?

[Buck, 2016/02/26 23:54:16]

Done.

+    // Try returned QUIC_PENDING
+    if (stream) {
+      next_state_ = STATE_OPEN;
+      DoCallback(OK);

[Ryan Hamilton, 2016/02/24 00:34:00]

Instead of DoCallback, can you simply do OnIOComplete(OK)? Does that work? The
asymetry between these two paths feel tingly but it might be totally fine.

[Buck, 2016/02/26 23:54:16]

No, because OnIoComplete() calls DoLoop() which toggles next_state_ back from
STATE_OPEN to STATE_NONE, and later code, namely IsResponseComplete(), wants to
see STATE_OPEN.

Unless maybe... change DoLoop to be a no-op on next_state_ == STATE_OPEN?

+      return;
+    }
+    // rendezvous has failed so proceed as with a non-push request.
+    next_state_ = STATE_STREAM_REQUEST;
+    OnIOComplete(OK);
+  }
+}
+
 int QuicHttpStream::InitializeStream(const HttpRequestInfo* request_info,
                                      RequestPriority priority,
                                      const BoundNetLog& stream_net_log,
                                      const CompletionCallback& callback) {
   DCHECK(!stream_);
   if (!session_)
     return was_handshake_confirmed_ ? ERR_CONNECTION_CLOSED
                                     : ERR_QUIC_HANDSHAKE_FAILED;
 
   stream_net_log.AddEvent(
       NetLog::TYPE_HTTP_STREAM_REQUEST_BOUND_TO_QUIC_SESSION,
       session_->net_log().source().ToEventParametersCallback());
 
   stream_net_log_ = stream_net_log;
   request_info_ = request_info;
   request_time_ = base::Time::Now();
   priority_ = priority;
 
   bool success = session_->GetSSLInfo(&ssl_info_);
   DCHECK(success);
   DCHECK(ssl_info_.cert.get());
 
+  if (session_->push_promise_index()->GetPromised(request_info->url.spec())) {
+    found_promise_ = true;
+    return OK;

[Ryan Hamilton, 2016/02/24 00:34:00]

Can you add a comment here which explains that the push will be coming later (or
some such)

[Buck, 2016/02/26 23:54:16]

Done.

+  }
+
+  next_state_ = STATE_STREAM_REQUEST;
+  int rv = DoLoop(OK);
+  if (rv == ERR_IO_PENDING)
+    callback_ = callback;
+
+  return rv;
+}
+
+int QuicHttpStream::DoStreamRequest() {
   int rv = stream_request_.StartRequest(
       session_, &stream_,
       base::Bind(&QuicHttpStream::OnStreamReady, weak_factory_.GetWeakPtr()));
-  if (rv == ERR_IO_PENDING) {
-    callback_ = callback;
-  } else if (rv == OK) {
+  if (rv == OK) {
     stream_->SetDelegate(this);
-  } else if (!was_handshake_confirmed_) {
+    if (response_info_) {
+      EnterStateSendHeaders();

[Ryan Hamilton, 2016/02/24 00:34:00]

I think it would be more common to rename EnterStateSendHeaders to
 DoStreamRequestComplete
and make it an explicit state.

+    }
+  } else if (rv != ERR_IO_PENDING && !was_handshake_confirmed_) {
     rv = ERR_QUIC_HANDSHAKE_FAILED;
   }
+  return rv;
+}
 
-  return rv;
+void QuicHttpStream::EnterStateSendHeaders() {
+  // Only advance to STATE_SEND_HEADERS if the stream is ready and
+  // SendRequest has been called (i.e. if response_info_ is a
+  // non-null value, so priority_ etc. are also set).

[Ryan Hamilton, 2016/02/24 00:34:00]

This comment implies some conditional logic which I don't seem to see. Does that
logic live elsewhere?

[Buck, 2016/02/26 23:54:16]

I updated the comment.   [ The conditional logic *was* here in a pre-review
version, but I later lifted it out to the callsites.  ]

+  DCHECK(stream_);
+  DCHECK(response_info_);
+  SpdyPriority priority = ConvertRequestPriorityToQuicPriority(priority_);
+  stream_->SetPriority(priority);
+  next_state_ = STATE_SEND_HEADERS;
 }
 
 void QuicHttpStream::OnStreamReady(int rv) {
   DCHECK(rv == OK || !stream_);
   if (rv == OK) {
     stream_->SetDelegate(this);
+    if (response_info_) {
+      EnterStateSendHeaders();
+      rv = DoLoop(OK);
+    }

[Ryan Hamilton, 2016/02/24 00:34:00]

My brain is working slowly. What circumstances lead to this new path?

[Buck, 2016/02/26 23:54:16]

Added a comment to explain.

   } else if (!was_handshake_confirmed_) {
     rv = ERR_QUIC_HANDSHAKE_FAILED;
   }
+  if (rv != ERR_IO_PENDING) {
+    DoCallback(rv);
+  }
+}
 
-  base::ResetAndReturn(&callback_).Run(rv);
+int QuicHttpStream::HandlePromise() {
+  QuicAsyncStatus push_status = session_->push_promise_index()->Try(
+      request_headers_, this, &this->push_handle_);
+  if (push_status == QUIC_FAILURE) {

[Ryan Hamilton, 2016/02/24 00:34:00]

nit: consider a switch statement here.

[Buck, 2016/02/26 23:54:16]

Done.

+    // Push rendezvous failed.
+    next_state_ = STATE_STREAM_REQUEST;
+  } else if (request_body_stream_) {
+    // Method type or request with body ineligble for push.
+    this->push_handle_->Cancel();
+    this->push_handle_ = nullptr;
+    next_state_ = STATE_STREAM_REQUEST;
+  } else if (push_status == QUIC_SUCCESS) {
+    next_state_ = STATE_OPEN;
+  } else {
+    DCHECK(push_status == QUIC_PENDING);
+    // Have a promise but the promised stream doesn't exist yet.
+    // Still have to do validation before accepting the promised
+    // stream for sure.
+    return ERR_IO_PENDING;
+  }
+  if (next_state_ != STATE_OPEN) {

[Ryan Hamilton, 2016/02/24 00:34:00]

How 'bout avoiding this conditional by doing a return; in the QUIC_SUCCESS case?
(And add a comment to explain why)

[Buck, 2016/02/26 23:54:16]

Done.

+    return DoLoop(OK);
+  }
+  return OK;
 }
 
 int QuicHttpStream::SendRequest(const HttpRequestHeaders& request_headers,
                                 HttpResponseInfo* response,
                                 const CompletionCallback& callback) {
   CHECK(!request_body_stream_);
   CHECK(!response_info_);
   CHECK(!callback.is_null());
   CHECK(response);
 
   // TODO(rch): remove this once we figure out why channel ID is not being
   // sent when it should be.
   HostPortPair origin = HostPortPair::FromURL(request_info_->url);
   if (origin.Equals(HostPortPair("accounts.google.com", 443)) &&
       request_headers.HasHeader(HttpRequestHeaders::kCookie)) {
     UMA_HISTOGRAM_BOOLEAN("Net.QuicSession.CookieSentToAccountsOverChannelId",
                           ssl_info_.channel_id_sent);
   }
-  if (!stream_) {
-    return ERR_CONNECTION_CLOSED;
+  if (!found_promise_) {
+    if (!stream_)
+      return ERR_CONNECTION_CLOSED;

[Ryan Hamilton, 2016/02/24 00:34:00]

I wonder if this should have the same logic as below?

if (!session_ || (!found_promise! && !stream_)) {
  return was_handshake_confirmed_ ? ERR_CONNECTION_CLOSED
                                                             :
ERR_QUIC_HANDSHAKE_FAILED;

but I'm not sure.

[Buck, 2016/02/26 23:54:16]

Done.

+  } else if (!session_) {
+    return was_handshake_confirmed_ ? ERR_CONNECTION_CLOSED
+                                    : ERR_QUIC_HANDSHAKE_FAILED;
   }
 
-  SpdyPriority priority = ConvertRequestPriorityToQuicPriority(priority_);
-  stream_->SetPriority(priority);
   // Store the serialized request headers.
   CreateSpdyHeadersFromHttpRequest(*request_info_, request_headers, HTTP2,
                                    /*direct=*/true, &request_headers_);
 
   // Store the request body.
   request_body_stream_ = request_info_->upload_data_stream;
   if (request_body_stream_) {
     // TODO(rch): Can we be more precise about when to allocate
     // raw_request_body_buf_. Removed the following check. DoReadRequestBody()
     // was being called even if we didn't yet allocate raw_request_body_buf_.
     //   && (request_body_stream_->size() ||
     //       request_body_stream_->is_chunked()))
     // Use 10 packets as the body buffer size to give enough space to
     // help ensure we don't often send out partial packets.
     raw_request_body_buf_ =
         new IOBufferWithSize(static_cast<size_t>(10 * kMaxPacketSize));
     // The request body buffer is empty at first.
     request_body_buf_ = new DrainableIOBuffer(raw_request_body_buf_.get(), 0);
   }
 
   // Store the response info.
   response_info_ = response;
 
-  next_state_ = STATE_SEND_HEADERS;
-  int rv = DoLoop(OK);
+  int rv;
+
+  if (found_promise_) {
+    rv = HandlePromise();
+  } else {
+    EnterStateSendHeaders();
+    rv = DoLoop(OK);
+  }
+
   if (rv == ERR_IO_PENDING)
     callback_ = callback;
 
   return rv > 0 ? OK : rv;
 }
 
 UploadProgress QuicHttpStream::GetUploadProgress() const {
   if (!request_body_stream_)
     return UploadProgress();
 
@@ skipping 222 matching lines @@
   // The client callback can do anything, including destroying this class,
   // so any pending callback must be issued after everything else is done.
   base::ResetAndReturn(&callback_).Run(rv);
 }
 
 int QuicHttpStream::DoLoop(int rv) {
   do {
     State state = next_state_;
     next_state_ = STATE_NONE;
     switch (state) {
+      case STATE_STREAM_REQUEST:
+        rv = DoStreamRequest();
+        break;
       case STATE_SEND_HEADERS:
         CHECK_EQ(OK, rv);
         rv = DoSendHeaders();
         break;
       case STATE_SEND_HEADERS_COMPLETE:
         rv = DoSendHeadersComplete(rv);
         break;
       case STATE_READ_REQUEST_BODY:
         CHECK_EQ(OK, rv);
         rv = DoReadRequestBody();
@@ skipping 175 matching lines @@
   }
   return rv;
 }
 
 void QuicHttpStream::ResetStream() {
   // TODO(rtenneti): Temporary until crbug.com/585591 is solved.
   if (read_in_progress_) {
     // |stream_| is going away when Read is called. Should never happen??
     CHECK(false);
   }
+  if (push_handle_) {
+    push_handle_->Cancel();
+    push_handle_ = nullptr;
+  }
   if (!stream_)
     return;
   closed_stream_received_bytes_ = stream_->stream_bytes_read();
   closed_stream_sent_bytes_ = stream_->stream_bytes_written();
   stream_ = nullptr;
 
   // If |request_body_stream_| is non-NULL, Reset it, to abort any in progress
   // read.
   if (request_body_stream_)
     request_body_stream_->Reset();
 }
 
 }  // namespace net