fix: match net module headers & http.IncomingMessage headers (#17517)

* fix: match net module headers & http.IncomingMessage headers

* update net doc for cleanliness

* address feedback from review

* Update spec/api-net-spec.js

Co-Authored-By: codebytere <codebytere@github.com>

* add special cookie case

docs/api/net.md

@@ -21,13 +21,10 @@ module instead of the native Node.js modules:
 * Support for traffic monitoring proxies: Fiddler-like proxies used for access
   control and monitoring.
 
-The `net` module API has been specifically designed to mimic, as closely as
-possible, the familiar Node.js API. The API components including classes,
-methods, properties and event names are similar to those commonly used in
+The API components (including classes, methods, properties and event names) are similar to those used in
 Node.js.
 
-For instance, the following example quickly shows how the `net` API might be
-used:
+Example usage:
 
 ```javascript
 const { app } = require('electron')
@@ -48,10 +45,6 @@ app.on('ready', () => {
 })
 ```
 
-By the way, it is almost identical to how you would normally use the
-[HTTP](https://nodejs.org/api/http.html)/[HTTPS](https://nodejs.org/api/https.html)
-modules of Node.js
-
 The `net` API can be used only after the application emits the `ready` event.
 Trying to use the module before the `ready` event will throw an error.
 

lib/browser/api/net.js

@@ -16,6 +16,29 @@ Object.setPrototypeOf(URLRequest.prototype, EventEmitter.prototype)
 
 const kSupportedProtocols = new Set(['http:', 'https:'])
 
+// set of headers that Node.js discards duplicates for
+// see https://nodejs.org/api/http.html#http_message_headers
+const discardableDuplicateHeaders = new Set([
+  'content-type',
+  'content-length',
+  'user-agent',
+  'referer',
+  'host',
+  'authorization',
+  'proxy-authorization',
+  'if-modified-since',
+  'if-unmodified-since',
+  'from',
+  'location',
+  'max-forwards',
+  'retry-after',
+  'etag',
+  'last-modified',
+  'server',
+  'age',
+  'expires'
+])
+
 class IncomingMessage extends Readable {
   constructor (urlRequest) {
     super()
@@ -41,7 +64,23 @@ class IncomingMessage extends Readable {
   }
 
   get headers () {
-    return this.urlRequest.rawResponseHeaders
+    const filteredHeaders = {}
+    const rawHeaders = this.urlRequest.rawResponseHeaders
+    Object.keys(rawHeaders).forEach(header => {
+      if (header in filteredHeaders && discardableDuplicateHeaders.has(header)) {
+        // do nothing with discardable duplicate headers
+      } else {
+        if (header === '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]
+        } else {
+          // for non-cookie headers, the values are joined together with ', '
+          filteredHeaders[header] = rawHeaders[header].join(', ')
+        }
+      }
+    })
+    return filteredHeaders
   }
 
   get httpVersion () {

spec/api-net-spec.js

@@ -1,4 +1,5 @@
 const assert = require('assert')
+const { expect } = require('chai')
 const { remote } = require('electron')
 const { ipcRenderer } = require('electron')
 const http = require('http')
@@ -1363,6 +1364,7 @@ describe('net module', () => {
       const requestUrl = '/requestUrl'
       const customHeaderName = 'Some-Custom-Header-Name'
       const customHeaderValue = 'Some-Customer-Header-Value'
+
       server.on('request', (request, response) => {
         switch (request.url) {
           case requestUrl:
@@ -1375,36 +1377,117 @@ describe('net module', () => {
             handleUnexpectedURL(request, response)
         }
       })
+
       const urlRequest = net.request({
         method: 'GET',
         url: `${server.url}${requestUrl}`
       })
+
       urlRequest.on('response', (response) => {
-        const statusCode = response.statusCode
-        assert(typeof statusCode === 'number')
-        assert.strictEqual(statusCode, 200)
-        const statusMessage = response.statusMessage
-        assert(typeof statusMessage === 'string')
-        assert.strictEqual(statusMessage, 'OK')
+        expect(response.statusCode).to.equal(200)
+        expect(response.statusMessage).to.equal('OK')
+
         const headers = response.headers
-        assert(typeof headers === 'object')
-        assert.deepStrictEqual(headers[customHeaderName.toLowerCase()],
-          [customHeaderValue])
+        expect(headers).to.be.an('object')
+        const headerValue = headers[customHeaderName.toLowerCase()]
+        expect(headerValue).to.equal(customHeaderValue)
+
         const httpVersion = response.httpVersion
-        assert(typeof httpVersion === 'string')
-        assert(httpVersion.length > 0)
+        expect(httpVersion).to.be.a('string').and.to.have.lengthOf.at.least(1)
+
         const httpVersionMajor = response.httpVersionMajor
-        assert(typeof httpVersionMajor === 'number')
-        assert(httpVersionMajor >= 1)
+        expect(httpVersionMajor).to.be.a('number').and.to.be.at.least(1)
+
         const httpVersionMinor = response.httpVersionMinor
-        assert(typeof httpVersionMinor === 'number')
-        assert(httpVersionMinor >= 0)
+        expect(httpVersionMinor).to.be.a('number').and.to.be.at.least(0)
+
         response.pause()
-        response.on('data', (chunk) => {
-        })
-        response.on('end', () => {
-          done()
-        })
+        response.on('data', chunk => {})
+        response.on('end', () => { done() })
+        response.resume()
+      })
+      urlRequest.end()
+    })
+
+    it('should discard duplicate headers', (done) => {
+      const requestUrl = '/duplicateRequestUrl'
+      const includedHeader = 'max-forwards'
+      const discardableHeader = 'Max-Forwards'
+
+      const includedHeaderValue = 'max-fwds-val'
+      const discardableHeaderValue = 'max-fwds-val-two'
+
+      server.on('request', (request, response) => {
+        switch (request.url) {
+          case requestUrl:
+            response.statusCode = 200
+            response.statusMessage = 'OK'
+            response.setHeader(discardableHeader, discardableHeaderValue)
+            response.setHeader(includedHeader, includedHeaderValue)
+            response.end()
+            break
+          default:
+            handleUnexpectedURL(request, response)
+        }
+      })
+
+      const urlRequest = net.request({
+        method: 'GET',
+        url: `${server.url}${requestUrl}`
+      })
+
+      urlRequest.on('response', response => {
+        expect(response.statusCode).to.equal(200)
+        expect(response.statusMessage).to.equal('OK')
+
+        const headers = response.headers
+        expect(headers).to.be.an('object')
+
+        expect(headers).to.have.property(includedHeader)
+        expect(headers).to.not.have.property(discardableHeader)
+        expect(headers[includedHeader]).to.equal(includedHeaderValue)
+
+        response.pause()
+        response.on('data', chunk => {})
+        response.on('end', () => { done() })
+        response.resume()
+      })
+      urlRequest.end()
+    })
+
+    it('should join repeated non-discardable value with ,', (done) => {
+      const requestUrl = '/requestUrl'
+
+      server.on('request', (request, response) => {
+        switch (request.url) {
+          case requestUrl:
+            response.statusCode = 200
+            response.statusMessage = 'OK'
+            response.setHeader('referrer-policy', ['first-text', 'second-text'])
+            response.end()
+            break
+          default:
+            handleUnexpectedURL(request, response)
+        }
+      })
+
+      const urlRequest = net.request({
+        method: 'GET',
+        url: `${server.url}${requestUrl}`
+      })
+
+      urlRequest.on('response', response => {
+        expect(response.statusCode).to.equal(200)
+        expect(response.statusMessage).to.equal('OK')
+
+        const headers = response.headers
+        expect(headers).to.be.an('object')
+        expect(headers).to.have.a.property('referrer-policy')
+        expect(headers['referrer-policy']).to.equal('first-text, second-text')
+
+        response.pause()
+        response.on('data', chunk => {})
+        response.on('end', () => { done() })
         response.resume()
       })
       urlRequest.end()