Adding support for POST requests.

atom/browser/api/atom_api_url_request.cc

@@ -41,11 +41,45 @@ struct Converter<scoped_refptr<const net::HttpResponseHeaders>> {
 
 template<>
 struct Converter<scoped_refptr<const net::IOBufferWithSize>> {
+
   static v8::Local<v8::Value> ToV8(
     v8::Isolate* isolate,
     scoped_refptr<const net::IOBufferWithSize> buffer) {
       return node::Buffer::Copy(isolate, buffer->data(), buffer->size()).ToLocalChecked();
   }
+
+  static bool FromV8(v8::Isolate* isolate, v8::Local<v8::Value> val,
+    scoped_refptr<const net::IOBufferWithSize>* out) {
+
+    auto size = node::Buffer::Length(val);
+    
+    if (size == 0) {
+      // Support conversoin from empty buffer. A use case is 
+      // a GET request without body. 
+      // Since zero-sized IOBuffer(s) are not supported, we set the
+      // out pointer to null.
+      *out = nullptr;
+      return true;
+    }
+    
+    auto data = node::Buffer::Data(val);
+    if (!data) {
+      // This is an error as size is positif but data is null.
+      return false;
+    }
+
+    auto io_buffer = new net::IOBufferWithSize(size);
+    if (!io_buffer) {
+      // Assuming allocation failed.
+      return false;
+    }
+
+    // We do a deep copy. We could have used Buffer's internal memory
+    // but that is much more complicated to be properly handled.
+    memcpy(io_buffer->data(), data, size);
+    *out = io_buffer;
+    return true;
+  }
 };
 
 }
@@ -102,8 +136,7 @@ void URLRequest::BuildPrototype(v8::Isolate* isolate,
   mate::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
     // Request API
     .MakeDestroyable()
-    .SetMethod("_write", &URLRequest::Write)
+    .SetMethod("_writeBuffer", &URLRequest::WriteBuffer)
-    .SetMethod("_end", &URLRequest::End)
     .SetMethod("abort", &URLRequest::Abort)
     .SetMethod("_setHeader", &URLRequest::SetHeader)
     .SetMethod("_getHeader", &URLRequest::GetHeader)
@@ -118,21 +151,27 @@ void URLRequest::BuildPrototype(v8::Isolate* isolate,
     
 }
 
-void URLRequest::Write() {
+bool URLRequest::WriteBuffer(scoped_refptr<const net::IOBufferWithSize> buffer, bool is_last) {
-  atom_request_->Write();
+  atom_request_->WriteBuffer(buffer, is_last);
+  return true;
 }
 
-void URLRequest::End() {
-  pin();
-  atom_request_->End();
-}
 
 void URLRequest::Abort() {
   atom_request_->Abort();
 }
 
-void URLRequest::SetHeader(const std::string& name, const std::string& value) {
+bool URLRequest::SetHeader(const std::string& name, const std::string& value) {
+  if (!net::HttpUtil::IsValidHeaderName(name)) {
+    return false;
+  }
+
+  if (!net::HttpUtil::IsValidHeaderValue(value)) {
+    return false;
+  }
+
   atom_request_->SetHeader(name, value);
+  return true;
 }
 std::string URLRequest::GetHeader(const std::string& name) {
   return atom_request_->GetHeader(name);
@@ -165,6 +204,8 @@ void URLRequest::OnResponseData(
 
 void URLRequest::OnResponseCompleted() {
   EmitResponseEvent("end");
+  unpin();
+  atom_request_ = nullptr;
 }
 
 

atom/browser/api/atom_api_url_request.h

@@ -32,10 +32,9 @@ class URLRequest : public mate::EventEmitter<URLRequest> {
 
 
 private:
-  void Write();
+  bool WriteBuffer(scoped_refptr<const net::IOBufferWithSize> buffer, bool is_last);
-  void End();
   void Abort();
-  void SetHeader(const std::string& name, const std::string& value);
+  bool SetHeader(const std::string& name, const std::string& value);
   std::string GetHeader(const std::string& name);
   void RemoveHeader(const std::string& name);
 
@@ -66,7 +65,7 @@ private:
   void pin();
   void unpin();
 
-  scoped_refptr<const AtomURLRequest> atom_request_;
+  scoped_refptr<AtomURLRequest> atom_request_;
   v8::Global<v8::Object> wrapper_;
   base::WeakPtrFactory<URLRequest> weak_ptr_factory_;
   

atom/browser/net/atom_url_request.cc

@@ -9,6 +9,8 @@
 #include "base/callback.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/io_buffer.h"
+#include "net/base/elements_upload_data_stream.h"
+#include "net/base/upload_bytes_element_reader.h"
 
 
 namespace {
@@ -19,9 +21,37 @@ const int kBufferSize = 4096;
 
 namespace atom {
 
+namespace internal {
+
+
+class UploadOwnedIOBufferElementReader : public net::UploadBytesElementReader {
+public:
+  explicit UploadOwnedIOBufferElementReader(
+    scoped_refptr<const net::IOBufferWithSize> buffer)
+    : net::UploadBytesElementReader(buffer->data(), buffer->size())
+    , buffer_(buffer) {
+  }
+  ~UploadOwnedIOBufferElementReader() override {
+  }
+
+
+  static UploadOwnedIOBufferElementReader* CreateWithBuffer(
+    scoped_refptr<const net::IOBufferWithSize> buffer) {
+    return new UploadOwnedIOBufferElementReader(std::move(buffer));
+  }
+
+private:
+  scoped_refptr<const net::IOBuffer> buffer_;
+
+  DISALLOW_COPY_AND_ASSIGN(UploadOwnedIOBufferElementReader);
+};
+
+}
+
 AtomURLRequest::AtomURLRequest(base::WeakPtr<api::URLRequest> delegate)
   : delegate_(delegate)
-  , buffer_(new net::IOBuffer(kBufferSize)) {
+  , response_read_buffer_(new net::IOBuffer(kBufferSize))
+  , is_chunked_upload_(is_chunked_upload_) {
 }
 
 AtomURLRequest::~AtomURLRequest() {
@@ -55,17 +85,25 @@ scoped_refptr<AtomURLRequest> AtomURLRequest::Create(
 
 }
 
-void AtomURLRequest::Write() const {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-}
 
-void AtomURLRequest::End() const {
+void AtomURLRequest::WriteBuffer(scoped_refptr<const net::IOBufferWithSize> buffer,
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+                           bool is_last) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI); 
   content::BrowserThread::PostTask(
     content::BrowserThread::IO, FROM_HERE,
-    base::Bind(&AtomURLRequest::DoStart, this));
+    base::Bind(&AtomURLRequest::DoWriteBuffer, this, buffer, is_last));
 }
 
+void AtomURLRequest::SetChunkedUpload() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // The method can be called only before switching to multi-threaded mode,
+  // i.e. before the first call to write.
+  // So it is safe to change the object in the UI thread.
+  is_chunked_upload_ = true;
+}
+
+
 void AtomURLRequest::Abort() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 }
@@ -119,10 +157,58 @@ void AtomURLRequest::PassLoginInformation(const base::string16& username,
 }
 
 
-void AtomURLRequest::DoStart() const {
+void AtomURLRequest::DoWriteBuffer(scoped_refptr<const net::IOBufferWithSize> buffer,
+                             bool is_last) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  request_->Start();
+  if (is_chunked_upload_) {
+
+    // Chunked encoding case.
+
+    bool first_call = false;
+    if (!chunked_stream_writer_) {
+      std::unique_ptr<net::ChunkedUploadDataStream> chunked_stream(
+        new net::ChunkedUploadDataStream(0));
+      chunked_stream_writer_ = chunked_stream->CreateWriter();
+      request_->set_upload(std::move(chunked_stream));
+      first_call = true;
+    }
+
+    if (buffer) {
+      // Non-empty buffer.
+      auto write_result = chunked_stream_writer_->AppendData(
+        buffer->data(),
+        buffer->size(),
+        is_last);
+    }
+    else if (is_last) {
+      // Empty buffer and last chunck, i.e. request.end().
+      auto write_result = chunked_stream_writer_->AppendData(
+        nullptr,
+        0,
+        true);
+    }
+
+    if (first_call) {
+      request_->Start();
+    }
+  }
+  else {
+
+    if (buffer) {
+      // Handling potential empty buffers.
+      std::unique_ptr<net::UploadElementReader> element_reader(internal::UploadOwnedIOBufferElementReader
+        ::CreateWithBuffer(std::move(buffer)));
+      upload_element_readers_.push_back(std::move(element_reader));
+    }
+
+    if (is_last) {
+      std::unique_ptr<net::UploadDataStream> elements_upload_data_stream(
+        new net::ElementsUploadDataStream(std::move(upload_element_readers_), 0));
+      request_->set_upload(std::move(elements_upload_data_stream));
+      request_->Start();
+    }
+  }
 }
 
 void AtomURLRequest::DoSetAuth(const base::string16& username,
@@ -175,7 +261,7 @@ void AtomURLRequest::ReadResponse() {
   // about is the response code and headers, which we already have).
   int bytes_read = 0;
   if (request_->status().is_success() /* TODO && (request_type_ != URLFetcher::HEAD)*/) {
-    if (!request_->Read(buffer_.get(), kBufferSize, &bytes_read))
+    if (!request_->Read(response_read_buffer_.get(), kBufferSize, &bytes_read))
       bytes_read = -1; 
   }
   OnReadCompleted(request_.get(), bytes_read);
@@ -197,7 +283,7 @@ void AtomURLRequest::OnReadCompleted(net::URLRequest* request, int bytes_read) {
       // Failed to transfer data to UI thread.
       return;
     }
-  } while (request_->Read(buffer_.get(), kBufferSize, &bytes_read));
+  } while (request_->Read(response_read_buffer_.get(), kBufferSize, &bytes_read));
     
   const auto status = request_->status();
 
@@ -213,10 +299,10 @@ void AtomURLRequest::OnReadCompleted(net::URLRequest* request, int bytes_read) {
 bool AtomURLRequest::CopyAndPostBuffer(int bytes_read) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  // data is only a wrapper for the async buffer_.
+  // data is only a wrapper for the async response_read_buffer_.
   // Make a deep copy of payload and transfer ownership to the UI thread.
   scoped_refptr<net::IOBufferWithSize> buffer_copy(new net::IOBufferWithSize(bytes_read));
-  memcpy(buffer_copy->data(), buffer_->data(), bytes_read);
+  memcpy(buffer_copy->data(), response_read_buffer_->data(), bytes_read);
 
   return content::BrowserThread::PostTask(
     content::BrowserThread::UI, FROM_HERE,

atom/browser/net/atom_url_request.h

@@ -8,20 +8,21 @@
 
 #include "base/memory/ref_counted.h"
 #include "net/url_request/url_request.h"
+#include "net/base/chunked_upload_data_stream.h"
 
 
 namespace net {
 class IOBuffer;
 class IOBufferWithSize;
 class DrainableIOBuffer;
-}
+};
 
 namespace atom {
 
 class AtomBrowserContext;
 
 namespace api {
-  class URLRequest;
+class URLRequest;
 }
 
 class AtomURLRequest : public base::RefCountedThreadSafe<AtomURLRequest>,
@@ -33,8 +34,8 @@ public:
     const std::string& url,
     base::WeakPtr<api::URLRequest> delegate);
 
-  void Write() const;
+  void WriteBuffer(scoped_refptr<const net::IOBufferWithSize> buffer, bool is_last);
-  void End() const;
+  void SetChunkedUpload();
   void Abort() const;
   void SetHeader(const std::string& name, const std::string& value) const;
   std::string GetHeader(const std::string& name) const;
@@ -53,7 +54,7 @@ protected:
 
 private:
   friend class base::RefCountedThreadSafe<AtomURLRequest>;
-  void DoStart() const;
+  void DoWriteBuffer(scoped_refptr<const net::IOBufferWithSize> buffer, bool is_last);
   void DoSetAuth(const base::string16& username,
     const base::string16& password) const;
   void DoCancelAuth() const;
@@ -73,7 +74,14 @@ private:
 
   base::WeakPtr<api::URLRequest> delegate_;
   std::unique_ptr<net::URLRequest> request_;
-  scoped_refptr<net::IOBuffer> buffer_;
+
+  bool is_chunked_upload_;
+  std::unique_ptr<net::ChunkedUploadDataStream> chunked_stream_;
+  std::unique_ptr<net::ChunkedUploadDataStream::Writer> chunked_stream_writer_;
+
+  std::vector<std::unique_ptr<net::UploadElementReader>>upload_element_readers_;
+
+  scoped_refptr<net::IOBuffer> response_read_buffer_;
   scoped_refptr<net::HttpResponseHeaders> response_headers_;
 
    DISALLOW_COPY_AND_ASSIGN(AtomURLRequest);

lib/browser/api/net.js

@@ -56,10 +56,16 @@ Net.prototype.request = function(options, callback) {
 }
 
 URLRequest.prototype._init = function() {
-  this._finished = false;
+  // Flag to prevent writings after end.
-  this._hasBody = true;
+  this._finished = false; 
+
+  // Set when the request uses chuned encoding. Can be switched
+  // to true only once and never set back to false.
   this._chunkedEncoding = false;
-  this._headersSent = false;
+
+  // Flag to prevent request's headers modifications after 
+  // headers flush.
+  this._headersSent = false; 
 }
 
 
@@ -111,80 +117,73 @@ URLRequest.prototype._emitResponseEvent = function() {
 }
 
 
-URLRequest.prototype.write = function(chunk, encoding, callback) {
+URLRequest.prototype._write = function(chunk, encoding, callback, is_last) {
-   
-  if (this.finished) {
-    var err = new Error('write after end');
-    process.nextTick(writeAfterEndNT, this, err, callback);
-
-    return true;
-  }
-
- /* TODO
-  if (!this._header) {
-    this._implicitHeader();
-  }
-  */
-
-  if (!this._hasBody) {
-    //debug('This type of response MUST NOT have a body. ' +
-    //      'Ignoring write() calls.');
-    return true;
-  }
   
-
+  let chunk_is_string = typeof chunk === 'string';
-  if (typeof chunk !== 'string' && !(chunk instanceof Buffer)) {
+  let chunk_is_buffer = chunk instanceof Buffer;
-    throw new TypeError('First argument must be a string or Buffer');
+  if (!chunk_is_string && !chunk_is_buffer) {
+    throw new TypeError('First argument must be a string or Buffer.');
   }
 
-
+  if (chunk_is_string) {
-  // If we get an empty string or buffer, then just do nothing, and
+    // We convert all strings into binary buffers.
-  // signal the user to keep writing.
+    chunk = Buffer.from(chunk, encoding);
-  if (chunk.length === 0) return true;
-
-  var len, ret;
-  if (this.chunkedEncoding) {
-    if (typeof chunk === 'string' &&
-        encoding !== 'hex' &&
-        encoding !== 'base64' &&
-        encoding !== 'binary') {
-      len = Buffer.byteLength(chunk, encoding);
-      chunk = len.toString(16) + CRLF + chunk + CRLF;
-      ret = this._send(chunk, encoding, callback);
-    } else {
-      // buffer, or a non-toString-friendly encoding
-      if (typeof chunk === 'string')
-        len = Buffer.byteLength(chunk, encoding);
-      else
-        len = chunk.length;
-
-      if (this.connection && !this.connection.corked) {
-        this.connection.cork();
-        process.nextTick(connectionCorkNT, this.connection);
-      }
-      this._send(len.toString(16), 'binary', null);
-      this._send(crlf_buf, null, null);
-      this._send(chunk, encoding, null);
-      ret = this._send(crlf_buf, null, callback);
-    }
-  } else {
-    ret = this._send(chunk, encoding, callback);
   }
 
-  //debug('write ret = ' + ret);
+  // Headers are assumed to be sent on first call to _writeBuffer,
-  return ret;
+  // i.e. after the first call to write or end.
+  let result = this._writeBuffer(chunk, is_last);
+
+  // Since writing to the network is asynchronous, we conservatively 
+  // assume that request headers are written after delivering the first
+  // buffer to the network IO thread.
+  this._headersSent = true;
+
+  // The write callback is fired asynchronously to mimic Node.js.
+  if (callback) {
+    process.nextTick(callback);
+  }
+
+  return result;
+}
+
+URLRequest.prototype.write = function(data, encoding, callback) {
+   
+  if (this._finished) {
+    let error = new Error('Write after end.');
+    process.nextTick(writeAfterEndNT, this, error, callback);
+    return true;
+  }
+
+  return this._write(data, encoding, callback, false);
 };
 
 
+URLRequest.prototype.end = function(data, encoding, callback) {
+  if (this._finished) {
+    return false;
+  }
+   
+  this._finished = true;
 
-URLRequest.prototype.end = function() {
+  if (typeof data === 'function') {
-  this._end();
+    callback = data;
+    encoding = null;
+    data = null;
+  } else if (typeof encoding === 'function') {
+    callback = encoding;
+    encoding = null;
+  }
+
+  data = data || '';
+
+  return this._write(data, encoding, callback, true);
 };
 
 
-function writeAfterEndNT(self, err, callback) {
+function writeAfterEndNT(self, error, callback) {
-  self.emit('error', err);
+  self.emit('error', error);
-  if (callback) callback(err);
+  if (callback) callback(error);
 }