fix: allow stream protocols to return headers with multiple values (#…

…14887)

* fix: allow stream protocols to return headers with multiple values

This allows stream protocols to return headers with multiple values as
an array of values.

Fixes #14778

* Prefer ConvertFromV8

* Cleanup header conversion

1. Deduplicate the code by using a lambda
2. Remove duplicate calls to headers->Get(key)

* Fix broken test

Headers with multiple values are now being converted correctly, this
test asserted the wrong behavior.

atom/common/native_mate_converters/net_converter.cc

@@ -181,24 +181,45 @@ bool Converter<net::HttpResponseHeaders*>::FromV8(
   if (!val->IsObject()) {
     return false;
   }
+
+  auto addHeaderFromValue = [&isolate, &out](
+                                const std::string& key,
+                                const v8::Local<v8::Value>& localVal) {
+    auto context = isolate->GetCurrentContext();
+    v8::Local<v8::String> localStrVal;
+    if (!localVal->ToString(context).ToLocal(&localStrVal)) {
+      return false;
+    }
+    std::string value;
+    mate::ConvertFromV8(isolate, localStrVal, &value);
+    out->AddHeader(key + ": " + value);
+    return true;
+  };
+
   auto context = isolate->GetCurrentContext();
   auto headers = v8::Local<v8::Object>::Cast(val);
   auto keys = headers->GetOwnPropertyNames();
   for (uint32_t i = 0; i < keys->Length(); i++) {
-    v8::Local<v8::String> key, value;
-    if (!keys->Get(i)->ToString(context).ToLocal(&key)) {
+    v8::Local<v8::String> keyVal;
+    if (!keys->Get(i)->ToString(context).ToLocal(&keyVal)) {
       return false;
     }
-    if (!headers->Get(key)->ToString(context).ToLocal(&value)) {
-      return false;
+    std::string key;
+    mate::ConvertFromV8(isolate, keyVal, &key);
+
+    auto localVal = headers->Get(keyVal);
+    if (localVal->IsArray()) {
+      auto values = v8::Local<v8::Array>::Cast(localVal);
+      for (uint32_t j = 0; j < values->Length(); j++) {
+        if (!addHeaderFromValue(key, values->Get(j))) {
+          return false;
+        }
+      }
+    } else {
+      if (!addHeaderFromValue(key, localVal)) {
+        return false;
+      }
     }
-    v8::String::Utf8Value key_utf8(key);
-    v8::String::Utf8Value value_utf8(value);
-    std::string k(*key_utf8, key_utf8.length());
-    std::string v(*value_utf8, value_utf8.length());
-    std::ostringstream tmp;
-    tmp << k << ": " << v;
-    out->AddHeader(tmp.str());
   }
   return true;
 }

spec/api-protocol-spec.js

@@ -529,7 +529,7 @@ describe('protocol module', () => {
           cache: false,
           success: (data, _, request) => {
             assert.strictEqual(request.status, 200)
-            assert.strictEqual(request.getResponseHeader('x-electron'), 'a,b')
+            assert.strictEqual(request.getResponseHeader('x-electron'), 'a, b')
             assert.strictEqual(data, text)
             done()
           },
@@ -589,6 +589,36 @@ describe('protocol module', () => {
         })
       })
     })
+
+    it('returns response multiple response headers with the same name', (done) => {
+      const handler = (request, callback) => {
+        callback({
+          headers: {
+            'header1': ['value1', 'value2'],
+            'header2': 'value3'
+          },
+          data: getStream()
+        })
+      }
+
+      protocol.registerStreamProtocol(protocolName, handler, (error) => {
+        if (error) return done(error)
+        $.ajax({
+          url: protocolName + '://fake-host',
+          cache: false,
+          success: (data, status, request) => {
+            // SUBTLE: when the response headers have multiple values it
+            // separates values by ", ". When the response headers are incorrectly
+            // converting an array to a string it separates values by ",".
+            assert.strictEqual(request.getAllResponseHeaders(), 'header1: value1, value2\r\nheader2: value3\r\n')
+            done()
+          },
+          error: (xhr, errorType, error) => {
+            done(error || new Error(`Request failed: ${xhr.status}`))
+          }
+        })
+      })
+    })
   })
 
   describe('protocol.isProtocolHandled', () => {