Small cleanups to ported QuicServer and dependencies

net/quic/quic_spdy_server_stream.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/quic/quic_spdy_server_stream.h"
 
 #include "base/memory/singleton.h"
 #include "base/strings/string_number_conversions.h"
 #include "net/quic/quic_in_memory_cache.h"
 #include "net/quic/quic_session.h"
@@ skipping 14 matching lines @@
     : QuicDataStream(id, session),
       read_buf_(new GrowableIOBuffer()),
       request_headers_received_(false) {
   read_buf_->SetCapacity(kHeaderBufInitialSize);
 }
 
 QuicSpdyServerStream::~QuicSpdyServerStream() {
 }
 
 uint32 QuicSpdyServerStream::ProcessData(const char* data, uint32 data_len) {
+  if (data_len > INT_MAX) {

[Ryan Hamilton, 2014/07/02 19:31:31]

Is INT_MAX the right type here, since int *might* be 64 bits? (What does the
internal code do?)

[wtc, 2014/07/02 19:39:08]

This protects the cast of 'data_len' to 'int' on line 42.

+    LOG(DFATAL) << "Data length too long: " << data_len;
+    return 0;
+  }
   // Are we still reading the request headers.
   if (!request_headers_received_) {
     // Grow the read buffer if necessary.
     while (read_buf_->RemainingCapacity() < (int)data_len) {

[wtc, 2014/07/02 19:39:08]

Please take the opportunity to change this to static_cast<int>(data_len)

[dmz, 2014/07/02 20:58:39]

Done.

       read_buf_->SetCapacity(read_buf_->capacity() * 2);
     }
     memcpy(read_buf_->data(), data, data_len);
     read_buf_->set_offset(read_buf_->offset() + data_len);
     // Try parsing the request headers. This will set request_headers_received_
     // if successful; if not, it will be tried again with more data.
     ParseRequestHeaders();
   } else {
     body_.append(data, data_len);
   }
@@ skipping 21 matching lines @@
   }
 
   SendResponse();
 }
 
 // Try parsing the request headers. If successful, sets
 // request_headers_received_. If not successful, it can just be tried again once
 // there's more data.
 void QuicSpdyServerStream::ParseRequestHeaders() {
   SpdyFramer framer((kDefaultSpdyMajorVersion));
-  char* data = read_buf_->StartOfBuffer();
+  const char* data = read_buf_->StartOfBuffer();
   size_t read_buf_len = static_cast<size_t>(read_buf_->offset());
   size_t len = framer.ParseHeaderBlockInBuffer(data, read_buf_len, &headers_);
   if (len == 0) {
     // Not enough data yet, presumably. (If we still don't succeed by the end of
-    // the stream, then we'll error above.)
+    // the stream, then we'll error in OnFinRead().)
     return;
   }
 
   // Headers received and parsed: extract the request URL.
   request_url_ = GetUrlFromHeaderBlock(headers_,
                                        kDefaultSpdyMajorVersion,
                                        false);
   if (!request_url_.is_valid()) {
     SendErrorResponse();
     return;
@@ skipping 22 matching lines @@
     CloseConnection(QUIC_NO_ERROR);
     return;
   }
 
   if (response->response_type() == QuicInMemoryCache::IGNORE_REQUEST) {
     DVLOG(1) << "Special response: ignoring request.";
     return;
   }
 
   DVLOG(1) << "Sending response for stream " << id();
-  SendHeadersAndBody(response->headers(), response->body());
+  SendHeadersAndBody(*response->headers(), response->body());

[wtc, 2014/07/02 19:39:08]

Hmm... If we are still dereferencing response->headers() without checking for a
null pointer, then the change of response->headers() from a reference to a
pointer isn't useful.

[dmz, 2014/07/02 20:58:39]

Alright, I've undone the change.

 }
 
 void QuicSpdyServerStream::SendErrorResponse() {
   DVLOG(1) << "Sending error response for stream " << id();
   scoped_refptr<HttpResponseHeaders> headers
       = new HttpResponseHeaders(string("HTTP/1.1 500 Server Error") + '\0' +
                                 "content-length: 3" + '\0' + '\0');
   SendHeadersAndBody(*headers, "bad");
 }
 
@@ skipping 11 matching lines @@
                                     &header_block);
 
   WriteHeaders(header_block, body.empty(), NULL);
 
   if (!body.empty()) {
     WriteOrBufferData(body, true, NULL);
   }
 }
 
 }  // namespace net