fix: pass full response headers in net module (#21770)

* fix: pass full response headers in net module * chore: put helper classes in annoymouse namespace * fix: use hasOwnProperty to test key * chore: shorter class name * fix: 7.x has different type for raw headers Co-authored-by: Cheng Zhao <zcbenz@github.com>
electron · Jan 15, 2020 · 7ddfffb · 7ddfffb
1 parent f8e601a
commit 7ddfffb
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 13 deletions.
diff --git a/lib/browser/api/net.js b/lib/browser/api/net.js
@@ -53,18 +53,27 @@ class IncomingMessage extends Readable {
 
   get headers () {
     const filteredHeaders = {}
-    const rawHeaders = this._responseHead.headers
-    Object.keys(rawHeaders).forEach(header => {
-      if (header in filteredHeaders && discardableDuplicateHeaders.has(header)) {
+    const { rawHeaders } = this._responseHead
+    rawHeaders.forEach(header => {
+      if (Object.prototype.hasOwnProperty.call(filteredHeaders, header.key) &&
+          discardableDuplicateHeaders.has(header.key)) {
         // do nothing with discardable duplicate headers
       } else {
-        if (header === 'set-cookie') {
+        if (header.key === 'set-cookie') {
           // keep set-cookie as an array per Node.js rules
           // see https://nodejs.org/api/http.html#http_message_headers
-          filteredHeaders[header] = rawHeaders[header]
+          if (Object.prototype.hasOwnProperty.call(filteredHeaders, header.key)) {
+            filteredHeaders[header.key].push(header.value)
+          } else {
+            filteredHeaders[header.key] = [header.value]
+          }
         } else {
           // for non-cookie headers, the values are joined together with ', '
-          filteredHeaders[header] = rawHeaders[header].join(', ')
+          if (Object.prototype.hasOwnProperty.call(filteredHeaders, header.key)) {
+            filteredHeaders[header.key] += `, ${header.value}`
+          } else {
+            filteredHeaders[header.key] = header.value
+          }
         }
       }
     })

diff --git a/shell/browser/api/atom_api_url_loader.cc b/shell/browser/api/atom_api_url_loader.cc
@@ -33,6 +33,28 @@
 #include "shell/common/node_includes.h"
 #include "shell/common/promise_util.h"
 
+namespace mate {
+
+template <>
+struct Converter<std::pair<std::string, std::string>> {
+  static v8::Local<v8::Value> ToV8(
+      v8::Isolate* isolate,
+      const std::pair<std::string, std::string>& pair) {
+    mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate);
+    dict.Set("key", base::ToLowerASCII(pair.first));
+    dict.Set("value", pair.second);
+    return dict.GetHandle();
+  }
+};
+
+}  // namespace mate
+
+namespace electron {
+
+namespace api {
+
+namespace {
+
 class BufferDataSource : public mojo::DataPipeProducer::DataSource {
  public:
   explicit BufferDataSource(base::span<char> buffer) {
@@ -200,12 +222,6 @@ class JSChunkedDataPipeGetter : public gin::Wrappable<JSChunkedDataPipeGetter>,
 gin::WrapperInfo JSChunkedDataPipeGetter::kWrapperInfo = {
     gin::kEmbedderNativeGin};
 
-namespace electron {
-
-namespace api {
-
-namespace {
-
 const net::NetworkTrafficAnnotationTag kTrafficAnnotation =
     net::DefineNetworkTrafficAnnotation("electron_net_module", R"(
         semantics {
@@ -343,6 +359,10 @@ mate::WrappableBase* SimpleURLLoaderWrapper::New(mate::Arguments* args) {
     }
   }
 
+  // Chromium filters headers using browser rules, while for net module we have
+  // every header passed.
+  request->report_raw_headers = true;
+
   v8::Local<v8::Value> body;
   v8::Local<v8::Value> chunk_pipe_getter;
   if (opts.Get("body", &body)) {
@@ -419,8 +439,12 @@ void SimpleURLLoaderWrapper::OnResponseStarted(
   mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate());
   dict.Set("statusCode", response_head.headers->response_code());
   dict.Set("statusMessage", response_head.headers->GetStatusText());
-  dict.Set("headers", response_head.headers.get());
   dict.Set("httpVersion", response_head.headers->GetHttpVersion());
+  // Note that |response_head.headers| are filtered by Chromium and should not
+  // be used here.
+  DCHECK(response_head.raw_request_response_info);
+  dict.Set("rawHeaders",
+           response_head.raw_request_response_info->response_headers);
   Emit("response-started", final_url, dict);
 }
 

diff --git a/spec-main/api-net-spec.ts b/spec-main/api-net-spec.ts
@@ -616,6 +616,23 @@ describe('net module', () => {
       })
     })
 
+    it('should be able to receive cookies', (done) => {
+      const cookie = ['cookie1', 'cookie2']
+      respondOnce.toSingleURL((request, response) => {
+        response.statusCode = 200
+        response.statusMessage = 'OK'
+        response.setHeader('set-cookie', cookie)
+        response.end()
+      }).then(serverUrl => {
+        const urlRequest = net.request(serverUrl)
+        urlRequest.on('response', (response) => {
+          expect(response.headers['set-cookie']).to.have.same.members(cookie)
+          done()
+        })
+        urlRequest.end()
+      })
+    })
+
     it('should be able to abort an HTTP request before first write', async () => {
       const serverUrl = await respondOnce.toSingleURL((request, response) => {
         response.end()