Prevent URLFetcher::AppendChunkedData from dereferencing NULL pointers.

net/url_request/url_fetcher_core.h

 // 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.
 
 #ifndef NET_URL_REQUEST_URL_FETCHER_CORE_H_
 #define NET_URL_REQUEST_URL_FETCHER_CORE_H_
 
 #include <stdint.h>
 
 #include <set>
 #include <string>
 
 #include "base/compiler_specific.h"
 #include "base/debug/stack_trace.h"
 #include "base/files/file_path.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/timer/timer.h"
+#include "net/base/chunked_upload_data_stream.h"
 #include "net/base/host_port_pair.h"
 #include "net/http/http_request_headers.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context_getter_observer.h"
 #include "net/url_request/url_request_status.h"
 #include "url/gurl.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ skipping 233 matching lines @@
   base::FilePath upload_file_path_;  // Path to file containing POST payload
   uint64_t upload_range_offset_;     // Offset from the beginning of the file
                                      // to be uploaded.
   uint64_t upload_range_length_;     // The length of the part of file to be
                                      // uploaded.
   URLFetcher::CreateUploadStreamCallback
       upload_stream_factory_;        // Callback to create HTTP POST payload.
   std::string upload_content_type_;  // MIME type of POST payload
   std::string referrer_;             // HTTP Referer header value and policy
   URLRequest::ReferrerPolicy referrer_policy_;
   bool is_chunked_upload_;           // True if using chunked transfer encoding

[mef, 2016/03/03 16:49:32]

do we need this bool or could it be replaced by chunked_stream_writer_?

[mmenke, 2016/03/03 17:04:45]

ChunkedUploadDataStreams are non-thread-safe, and have to be used on the thread
they're created on.  These values are all initialized on the embedder thread,
not the network thread.

We could post a task over to the network thread to set the
chunked_stream_writer_, but then we couldn't DCHECK on it...  Also,
unfortunately (Very unfortunately), URLFetchers can be reused, and they keep
this value between uses, but can't keep chunked_stream_writer_, and I'm
reluctant to change the API.

I do want to get rid of the ability for these to be reused, but that would
probably be a major undertaking.  :(

[mef, 2016/03/08 17:19:22]

So, if URLFetcher is re-used, would we need to create another chunked_stream_ to
go with new UrlRequest?

[mmenke, 2016/03/09 17:35:00]

Oops...Missed this.  Yes, we would.

 
+  // Used to write to |chunked_stream|, even after ownership has been passed to
+  // the URLRequest. Continues to be valid even after the request deletes its
+  // upload data.
+  scoped_ptr<ChunkedUploadDataStream::Writer> chunked_stream_writer_;
+
+  // Temporary storage of ChunkedUploadDataStream, before request is created.
+  scoped_ptr<ChunkedUploadDataStream> chunked_stream_;
+
   // Used to determine how long to wait before making a request or doing a
   // retry.
   //
   // Both of them can only be accessed on the IO thread.
   //
   // To determine the proper backoff timing, throttler entries for
   // both |original_URL| and |url| are needed. For example, consider
   // the case that URL A redirects to URL B, for which the server
   // returns a 500 response. In this case, the exponential back-off
   // release time of URL A won't increase. If only the backoff
@@ skipping 56 matching lines @@
   base::debug::StackTrace stack_trace_;
 
   static base::LazyInstance<Registry> g_registry;
 
   DISALLOW_COPY_AND_ASSIGN(URLFetcherCore);
 };
 
 }  // namespace net
 
 #endif  // NET_URL_REQUEST_URL_FETCHER_CORE_H_