Repository: node-fetch/node-fetch (js)

Issue:
doesn't correctly handle multiple `content-type` values
I found a peculiar issue of incompatibility of this library comparing to native browser fetch version. Affected **version 2.6.0**

Steps to reproduce:

**1.) Create simple node server and run it**
```js
const http = require('http');

const requestListener = function (req, res) {
  res.setHeader('Cookie', ['one=1', 'two=2']);
  res.setHeader('Content-Type', ['text/plain', 'application/json']);
  res.writeHead(200);
  res.end('Hello, World!');
}

const server = http.createServer(requestListener);
server.listen(8080);
```

**2.) Fetch the server**

`In browser`
```js
fetch('http://localhost:8080').then(r => r.headers).then(h => h.get('Content-Type')).then(console.dir);
// text/plain, application/json
```

`node-fetch`
```js
fetch('http://localhost:8080').then(r => r.headers).then(h => h.get('Content-Type')).then(console.dir);
// text/plain
```
As you can see, `node-fetch` doesn't correctly handle multiple headers with the same name.

Pull request:
revamp Headers module
**What is the purpose of this pull request?**

- [X] Other, please explain:

This is an almost complete rewrite of the `Headers` module (so, diff is looking crazy - it's better just to read new code completely). 

The current implementation of `Headers` is somewhere painful to read (particularly iterator implementation). It's nowhere close to being lightweight too with inventing some wheels and avoiding modern language features.

**What changes did you make? (provide an overview)**

The only change to test file is the replacement of originally broken test for `createHeadersLenient` - it was testing for falseness properties on `Headers` object that was always false.
Change for other files is renaming of internal methods `createHeadersLenient` (to `fromRawHeaders`) and `exportNodeCompatibleHeaders` (to build-in symbol).

The approach used in this refactoring is to take advantage of the fact, that there is _almost_ completely identical by shape built-in object in Node >= 10.0 - `URLSearchParams` (I do believe it was copied from `Headers` when introduced browser-side, so, now we can reuse it back). We extend here `URLSearchParams` attending minor differences in functionality (case insensitivity, parameters validation, and keys iteration) via lightweight Proxy, and major functionality differences (`get` and values iteration) as overloaded methods.

I'd moved `exportNodeCompatibleHeaders` export into Node.JS built-in symbol, so, it also now serves for _everyone secret debug powerhouses_ - `console.log(response.headers)`

I've replaced `createHeadersLenient` and creating `Headers` from `rawHeaders`, that fixes #783.

With the use of Proxy, constructor return value and generator functions in the source is also a fun weekend reading for everybody who loves modern JavaScript 😄 

**Is there anything you'd like reviewers to know?**

This PR purposely doesn't touch any tests to showcase that module is in equal functionality, however, there are some points to discuss:

1. Headers now, by side effect, exposing `getAll` method (as it exists on `URLSearchParams`) that was removed from client-side spec due to security concern and then removed from `node-fetch` too, then `raw` was introduced when we realized than on the server-side, indeed, we need to be able to get all values. 
It's easy to hide this method completely (or make non-enumerable), or leave it undocumented (all other modules as Request and Response are exposing some non-spec methods and we have `raw` here anyway) or to document it. I do believe it's an extremely useful server-side and better fit for Cookies than `raw`. So @node-fetch/core probably should decide here.
See: https://github.com/whatwg/fetch/issues/973#issuecomment-560136041

2. There is no current test for `new Headers(null)` and it passes in both new and original versions of Headers. However, it throws at Chrome. Should behavior be adjusted and test added?

~3. There is a subtle difference in how headers are returned concatenated - `node-fetch` is using `, ` to join them while browsers use just coma. Should it be adjusted?~

4. Proxies are fast in v8: https://v8.dev/blog/optimizing-proxies

Patch:
diff --git a/src/headers.js b/src/headers.js
index f767ef1..8ee47a6 100644
--- a/src/headers.js
+++ b/src/headers.js
@@ -1,76 +1,59 @@
-
 /**
  * Headers.js
  *
  * Headers class offers convenient helpers
  */
 
+import {types} from 'util';
+
 const invalidTokenRegex = /[^`\-\w!#$%&'*+.|~]/;
 const invalidHeaderCharRegex = /[^\t\u0020-\u007E\u0080-\u00FF]/;
 
 function validateName(name) {
-	name = `${name}`;
+	name = String(name);
 	if (invalidTokenRegex.test(name) || name === '') {
-		throw new TypeError(`${name} is not a legal HTTP header name`);
+		throw new TypeError(`'${name}' is not a legal HTTP header name`);
 	}
 }
 
 function validateValue(value) {
-	value = `${value}`;
+	value = String(value);
 	if (invalidHeaderCharRegex.test(value)) {
-		throw new TypeError(`${value} is not a legal HTTP header value`);
+		throw new TypeError(`'${value}' is not a legal HTTP header value`);
 	}
 }
 
 /**
- * Find the key in the map object given a header name.
- *
- * Returns undefined if not found.
- *
- * @param   String  name  Header name
- * @return  String|Undefined
+ * @typedef {Headers | Record<string, string> | Iterable<readonly [string, string]> | Iterable<string>[]} HeadersInit
  */
-function find(map, name) {
-	name = name.toLowerCase();
-	for (const key in map) {
-		if (key.toLowerCase() === name) {
-			return key;
-		}
-	}
 
-	return undefined;
-}
-
-const MAP = Symbol('map');
-export default class Headers {
+/**
+ * This Fetch API interface allows you to perform various actions on HTTP request and response headers.
+ * These actions include retrieving, setting, adding to, and removing.
+ * A Headers object has an associated header list, which is initially empty and consists of zero or more name and value pairs.
+ * You can add to this using methods like append() (see Examples.)
+ * In all methods of this interface, header names are matched by case-insensitive byte sequence.
+ *
+ */
+export default class Headers extends URLSearchParams {
 	/**
 	 * Headers class
 	 *
-	 * @param   Object  headers  Response headers
-	 * @return  Void
+	 * @constructor
+	 * @param {HeadersInit} [init] - Response headers
 	 */
-	constructor(init = undefined) {
-		this[MAP] = Object.create(null);
-
+	constructor(init) {
+		// Validate and normalize init object in [name, value(s)][]
+		/** @type {string[][]} */
+		let result = [];
 		if (init instanceof Headers) {
-			const rawHeaders = init.raw();
-			const headerNames = Object.keys(rawHeaders);
-
-			for (const headerName of headerNames) {
-				for (const value of rawHeaders[headerName]) {
-					this.append(headerName, value);
-				}
+			const raw = init.raw();
+			for (const [name, values] of Object.entries(raw)) {
+				result.push(...values.map(value => [name, value]));
 			}
-
-			return;
-		}
-
-		// We don't worry about converting prop to ByteString here as append()
-		// will handle it.
-		// eslint-disable-next-line no-eq-null, eqeqeq
-		if (init == null) {
+		} else if (init == null) { // eslint-disable-line no-eq-null, eqeqeq
 			// No op
-		} else if (typeof init === 'object') {
+		} else if (typeof init === 'object' && !types.isBoxedPrimitive(init)) {
 			const method = init[Symbol.iterator];
 			// eslint-disable-next-line no-eq-null, eqeqeq
 			if (method != null) {
@@ -80,312 +63,190 @@ export default class Headers {
 
 				// Sequence<sequence<ByteString>>
 				// Note: per spec we have to first exhaust the lists then process them
-				const pairs = [];
-				for (const pair of init) {
-					if (typeof pair !== 'object' || typeof pair[Symbol.iterator] !== 'function') {
-						throw new TypeError('Each header pair must be iterable');
-					}
-
-					pairs.push([...pair]);
-				}
-
-				for (const pair of pairs) {
-					if (pair.length !== 2) {
-						throw new TypeError('Each header pair must be a name/value tuple');
-					}
-
-					this.append(pair[0], pair[1]);
-				}
+				result = [...init]
+					.map(pair => {
+						if (
+							typeof pair !== 'object' || types.isBoxedPrimitive(pair)
+						) {
+							throw new TypeError('Each header pair must be an iterable object');
+						}
+
+						return [...pair];
+					}).map(pair => {
+						if (pair.length !== 2) {
+							throw new TypeError('Each header pair must be a name/value tuple');
+						}
+
+						return [...pair];
+					});
 			} else {
 				// Record<ByteString, ByteString>
-				for (const key of Object.keys(init)) {
-					const value = init[key];
-					this.append(key, value);
-				}
+				result.push(...Object.entries(init));
 			}
 		} else {
-			throw new TypeError('Provided initializer must be an object');
+			throw new TypeError('Failed to construct \'Headers\': The provided value is not of type \'(sequence<sequence<ByteString>> or record<ByteString, ByteString>)');
 		}
+
+		// Validate and lowercase
+		result =
+			result.length > 0 ?
+				result.map(([name, value]) => {
+					validateName(name);
+					validateValue(value);
+					return [String(name).toLowerCase(), value];
+				}) :
+				undefined;
+
+		super(result);
+
+		// Returning a Proxy that will lowercase key names, validate parameters and sort keys
+		// eslint-disable-next-line no-constructor-return
+		return new Proxy(this, {
+			get(target, p, receiver) {
+				switch (p) {
+					case 'append':
+					case 'set':
+						return (name, value) => {
+							validateName(name);
+							validateValue(value);
+							return URLSearchParams.prototype[p].call(
+								receiver,
+								String(name).toLowerCase(),
+								value
+							);
+						};
+
+					case 'delete':
+					case 'has':
+					case 'getAll':
+						return name => {
+							validateName(name);
+							return URLSearchParams.prototype[p].call(
+								receiver,
+								String(name).toLowerCase()
+							);
+						};
+
+					case 'keys':
+						return () => {
+							target.sort();
+							return new Set(URLSearchParams.prototype.keys.call(target)).keys();
+						};
+
+					default:
+						return Reflect.get(target, p, receiver);
+				}
+			}
+		});
+	}
+
+	get [Symbol.toStringTag]() {
+		return 'Headers';
+	}
+
+	toString() {
+		return Object.prototype.toString.call(this);
 	}
 
-	/**
-	 * Return combined header value given name
-	 *
-	 * @param   String  name  Header name
-	 * @return  Mixed
-	 */
 	get(name) {
-		name = `${name}`;
-		validateName(name);
-		const key = find(this[MAP], name);
-		if (key === undefined) {
+		const values = this.getAll(name);
+		if (values.length === 0) {
 			return null;
 		}
 
-		let value = this[MAP][key].join(', ');
-		if (name.toLowerCase() === 'content-encoding') {
+		let value = values.join(', ');
+		if (/^content-encoding$/i.test(name)) {
 			value = value.toLowerCase();
 		}
 
 		return value;
 	}
 
-	/**
-	 * Iterate over all headers
-	 *
-	 * @param   Function  callback  Executed for each item with parameters (value, name, thisArg)
-	 * @param   Boolean   thisArg   `this` context for callback function
-	 * @return  Void
-	 */
-	forEach(callback, thisArg = undefined) {
-		let pairs = getHeaders(this);
-		let i = 0;
-		while (i < pairs.length) {
-			const [name, value] = pairs[i];
-			callback.call(thisArg, value, name, this);
-			pairs = getHeaders(this);
-			i++;
+	forEach(callback) {
+		for (const name of this.keys()) {
+			callback(this.get(name), name);
 		}
 	}
 
-	/**
-	 * Overwrite header values given name
-	 *
-	 * @param   String  name   Header name
-	 * @param   String  value  Header value
-	 * @return  Void
-	 */
-	set(name, value) {
-		name = `${name}`;
-		value = `${value}`;
-		validateName(name);
-		validateValue(value);
-		const key = find(this[MAP], name);
-		this[MAP][key !== undefined ? key : name] = [value];
-	}
-
-	/**
-	 * Append a value onto existing header
-	 *
-	 * @param   String  name   Header name
-	 * @param   String  value  Header value
-	 * @return  Void
-	 */
-	append(name, value) {
-		name = `${name}`;
-		value = `${value}`;
-		validateName(name);
-		validateValue(value);
-		const key = find(this[MAP], name);
-		if (key !== undefined) {
-			this[MAP][key].push(value);
-		} else {
-			this[MAP][name] = [value];
+	* values() {
+		for (const name of this.keys()) {
+			yield this.get(name);
 		}
 	}
 
 	/**
-	 * Check for header name existence
-	 *
-	 * @param   String   name  Header name
-	 * @return  Boolean
+	 * @type {() => IterableIterator<[string, string]>}
 	 */
-	has(name) {
-		name = `${name}`;
-		validateName(name);
-		return find(this[MAP], name) !== undefined;
-	}
-
-	/**
-	 * Delete all header values given name
-	 *
-	 * @param   String  name  Header name
-	 * @return  Void
-	 */
-	delete(name) {
-		name = `${name}`;
-		validateName(name);
-		const key = find(this[MAP], name);
-		if (key !== undefined) {
-			delete this[MAP][key];
+	* entries() {
+		for (const name of this.keys()) {
+			yield [name, this.get(name)];
 		}
 	}
 
-	/**
-	 * Return raw headers (non-spec api)
-	 *
-	 * @return  Object
-	 */
-	raw() {
-		return this[MAP];
-	}
-
-	/**
-	 * Get an iterator on keys.
-	 *
-	 * @return  Iterator
-	 */
-	keys() {
-		return createHeadersIterator(this, 'key');
+	[Symbol.iterator]() {
+		return this.entries();
 	}
 
 	/**
-	 * Get an iterator on values.
-	 *
-	 * @return  Iterator
+	 * Node-fetch non-spec method
+	 * returning all headers and their values as array
+	 * @returns {Record<string, string[]>}
 	 */
-	values() {
-		return createHeadersIterator(this, 'value');
+	raw() {
+		return [...this.keys()].reduce((res, key) => {
+			res[key] = this.getAll(key);
+			return res;
+		}, {});
 	}
 
 	/**
-	 * Get an iterator on entries.
-	 *
-	 * This is the default iterator of the Headers object.
-	 *
-	 * @return  Iterator
+	 * For better console.log(headers) and also to convert Headers into Node.js Request compatible format
 	 */
-	[Symbol.iterator]() {
-		return createHeadersIterator(this, 'key+value');
-	}
-}
-Headers.prototype.entries = Headers.prototype[Symbol.iterator];
-
-Object.defineProperty(Headers.prototype, Symbol.toStringTag, {
-	value: 'Headers',
-	writable: false,
-	enumerable: false,
-	configurable: true
-});
-
-Object.defineProperties(Headers.prototype, {
-	get: {enumerable: true},
-	forEach: {enumerable: true},
-	set: {enumerable: true},
-	append: {enumerable: true},
-	has: {enumerable: true},
-	delete: {enumerable: true},
-	keys: {enumerable: true},
-	values: {enumerable: true},
-	entries: {enumerable: true}
-});
-
-function getHeaders(headers, kind = 'key+value') {
-	const keys = Object.keys(headers[MAP]).sort();
+	[Symbol.for('nodejs.util.inspect.custom')]() {
+		return [...this.keys()].reduce((res, key) => {
+			const values = this.getAll(key);
+			// Http.request() only supports string as Host header.
+			// This hack makes specifying custom Host header possible.
+			if (key === 'host') {
+				res[key] = values[0];
+			} else {
+				res[key] = values.length > 1 ? values : values[0];
+			}
 
-	let iterator;
-	if (kind === 'key') {
-		iterator = header => header.toLowerCase();
-	} else if (kind === 'value') {
-		iterator = header => headers[MAP][header].join(', ');
-	} else {
-		iterator = header => [header.toLowerCase(), headers[MAP][header].join(', ')];
+			return res;
+		}, {});
 	}
-
-	return keys.map(header => iterator(header));
-}
-
-const INTERNAL = Symbol('internal');
-
-function createHeadersIterator(target, kind) {
-	const iterator = Object.create(HeadersIteratorPrototype);
-	iterator[INTERNAL] = {
-		target,
-		kind,
-		index: 0
-	};
-	return iterator;
 }
 
-const HeadersIteratorPrototype = Object.setPrototypeOf({
-	next() {
-		// istanbul ignore if
-		if (!this ||
-			Object.getPrototypeOf(this) !== HeadersIteratorPrototype) {
-			throw new TypeError('Value of `this` is not a HeadersIterator');
-		}
-
-		const {
-			target,
-			kind,
-			index
-		} = this[INTERNAL];
-		const values = getHeaders(target, kind);
-		const length_ = values.length;
-		if (index >= length_) {
-			return {
-				value: undefined,
-				done: true
-			};
-		}
-
-		this[INTERNAL].index = index + 1;
-
-		return {
-			value: values[index],
-			done: false
-		};
-	}
-}, Object.getPrototypeOf(
-	Object.getPrototypeOf([][Symbol.iterator]())
-));
-
-Object.defineProperty(HeadersIteratorPrototype, Symbol.toStringTag, {
-	value: 'HeadersIterator',
-	writable: false,
-	enumerable: false,
-	configurable: true
-});
-
 /**
- * Export the Headers object in a form that Node.js can consume.
- *
- * @param   Headers  headers
- * @return  Object
+ * Re-shaping object for Web IDL tests
+ * Only need to do it for overridden methods
  */
-export function exportNodeCompatibleHeaders(headers) {
-	const object = {__proto__: null, ...headers[MAP]};
-
-	// Http.request() only supports string as Host header. This hack makes
-	// specifying custom Host header possible.
-	const hostHeaderKey = find(headers[MAP], 'Host');
-	if (hostHeaderKey !== undefined) {
-		object[hostHeaderKey] = object[hostHeaderKey][0];
-	}
-
-	return object;
-}
+Object.defineProperties(
+	Headers.prototype,
+	['get', 'entries', 'forEach', 'values'].reduce((res, property) => {
+		res[property] = {enumerable: true};
+		return res;
+	}, {})
+);
 
 /**
- * Create a Headers object from an object of headers, ignoring those that do
+ * Create a Headers object from an http.IncomingMessage.rawHeaders, ignoring those that do
  * not conform to HTTP grammar productions.
- *
- * @param   Object  obj  Object of headers
- * @return  Headers
+ * @param {import('http').IncomingMessage['rawHeaders']} headers
  */
-export function createHeadersLenient(object) {
-	const headers = new Headers();
-	for (const name of Object.keys(object)) {
-		if (invalidTokenRegex.test(name)) {
-			continue;
-		}
-
-		if (Array.isArray(object[name])) {
-			for (const value of object[name]) {
-				if (invalidHeaderCharRegex.test(value)) {
-					continue;
+export function fromRawHeaders(headers = []) {
+	return new Headers(
+		headers
+			// Split into pairs
+			.reduce((result, value, index, array) => {
+				if (index % 2 === 0) {
+					result.push(array.slice(index, index + 2));
 				}
 
-				if (headers[MAP][name] === undefined) {
-					headers[MAP][name] = [value];
-				} else {
-					headers[MAP][name].push(value);
-				}
-			}
-		} else if (!invalidHeaderCharRegex.test(object[name])) {
-			headers[MAP][name] = [object[name]];
-		}
-	}
+				return result;
+			}, [])
+			.filter(([name, value]) => !(invalidTokenRegex.test(name) || invalidHeaderCharRegex.test(value)))
 
-	return headers;
+	);
 }
diff --git a/src/index.js b/src/index.js
index 71c502c..e36434d 100644
--- a/src/index.js
+++ b/src/index.js
@@ -14,7 +14,7 @@ import dataURIToBuffer from 'data-uri-to-buffer';
 
 import Body, {writeToStream, getTotalBytes} from './body.js';
 import Response from './response.js';
-import Headers, {createHeadersLenient} from './headers.js';
+import Headers, {fromRawHeaders} from './headers.js';
 import Request, {getNodeRequestOptions} from './request.js';
 import FetchError from './errors/fetch-error.js';
 import AbortError from './errors/abort-error.js';
@@ -108,7 +108,7 @@ export default function fetch(url, options_) {
 
 		request_.on('response', res => {
 			request_.setTimeout(0);
-			const headers = createHeadersLenient(res.headers);
+			const headers = fromRawHeaders(res.rawHeaders);
 
 			// HTTP fetch step 5
 			if (isRedirect(res.statusCode)) {
diff --git a/src/request.js b/src/request.js
index fc23e14..4a32161 100644
--- a/src/request.js
+++ b/src/request.js
@@ -8,7 +8,7 @@
  */
 
 import {format as formatUrl} from 'url';
-import Headers, {exportNodeCompatibleHeaders} from './headers.js';
+import Headers from './headers.js';
 import Body, {clone, extractContentType, getTotalBytes} from './body.js';
 import {isAbortSignal} from './utils/is.js';
 import {getSearch} from './utils/get-search.js';
@@ -251,7 +251,7 @@ export function getNodeRequestOptions(request) {
 		query: parsedURL.query,
 		href: parsedURL.href,
 		method: request.method,
-		headers: exportNodeCompatibleHeaders(headers),
+		headers: headers[Symbol.for('nodejs.util.inspect.custom')](),
 		agent
 	};
 


Test patch:
diff --git a/test/main.js b/test/main.js
index 750f96a..d28401e 100644
--- a/test/main.js
+++ b/test/main.js
@@ -30,7 +30,7 @@ import fetch, {
 	Response
 } from '../src/index.js';
 import FetchErrorOrig from '../src/errors/fetch-error.js';
-import HeadersOrig, {createHeadersLenient} from '../src/headers.js';
+import HeadersOrig, {fromRawHeaders} from '../src/headers.js';
 import RequestOrig from '../src/request.js';
 import ResponseOrig from '../src/response.js';
 import Body, {getTotalBytes, extractContentType} from '../src/body.js';
@@ -545,15 +545,18 @@ describe('node-fetch', () => {
 	});
 
 	it('should ignore invalid headers', () => {
-		let headers = {
-			'Invalid-Header ': 'abc\r\n',
-			'Invalid-Header-Value': '\u0007k\r\n',
-			'Set-Cookie': ['\u0007k\r\n', '\u0007kk\r\n']
-		};
-		headers = createHeadersLenient(headers);
-		expect(headers).to.not.have.property('Invalid-Header ');
-		expect(headers).to.not.have.property('Invalid-Header-Value');
-		expect(headers).to.not.have.property('Set-Cookie');
+		const headers = fromRawHeaders([
+			'Invalid-Header ',
+			'abc\r\n',
+			'Invalid-Header-Value',
+			'\u0007k\r\n',
+			'Cookie',
+			'\u0007k\r\n',
+			'Cookie',
+			'\u0007kk\r\n'
+		]);
+		expect(headers).to.be.instanceOf(Headers);
+		expect(headers.raw()).to.deep.equal({});
 	});
 
 	it('should handle client-error response', () => {