Fix Data URI handling and drop all URL analysis RegExps (#853)

* add breaking test
* don't use RegExp for URLs

package.json

@@ -67,7 +67,7 @@
 		"xo": "^0.30.0"
 	},
 	"dependencies": {
-		"data-uri-to-buffer": "^3.0.0",
+		"data-uri-to-buffer": "^3.0.1",
 		"fetch-blob": "^1.0.6"
 	},
 	"tsd": {

src/index.js

@@ -10,7 +10,7 @@ import http from 'http';
 import https from 'https';
 import zlib from 'zlib';
 import Stream, {PassThrough, pipeline as pump} from 'stream';
-import dataURIToBuffer from 'data-uri-to-buffer';
+import dataUriToBuffer from 'data-uri-to-buffer';
 
 import {writeToStream, getTotalBytes} from './body.js';
 import Response from './response.js';
@@ -22,36 +22,32 @@ import {isRedirect} from './utils/is-redirect.js';
 
 export {Headers, Request, Response, FetchError, AbortError, isRedirect};
 
+const supportedSchemas = new Set(['data:', 'http:', 'https:']);
+
 /**
  * Fetch function
  *
- * @param   Mixed    url   Absolute url or Request instance
- * @param   Object   opts  Fetch options
- * @return  Promise
+ * @param   {string | URL | import('./request').default} url - Absolute url or Request instance
+ * @param   {*} [options_] - Fetch options
+ * @return  {Promise<import('./response').default>}
  */
 export default async function fetch(url, options_) {
-	// Regex for data uri
-	const dataUriRegex = /^\s*data:([a-z]+\/[a-z]+(;[a-z-]+=[a-z-]+)?)?(;base64)?,[\w!$&',()*+;=\-.~:@/?%\s]*\s*$/i;
-
-	// If valid data uri
-	if (dataUriRegex.test(url)) {
-		const data = dataURIToBuffer(url);
-		const response = new Response(data, {headers: {'Content-Type': data.type}});
-		return response;
-	}
-
-	// If invalid data uri
-	if (url.toString().startsWith('data:')) {
-		const request = new Request(url, options_);
-		throw new FetchError(`[${request.method}] ${request.url} invalid URL`, 'system');
-	}
-
-	// Wrap http.request into fetch
 	return new Promise((resolve, reject) => {
 		// Build request object
 		const request = new Request(url, options_);
 		const options = getNodeRequestOptions(request);
+		if (!supportedSchemas.has(options.protocol)) {
+			throw new TypeError(`node-fetch cannot load ${url}. URL scheme "${options.protocol.replace(/:$/, '')}" is not supported.`);
+		}
 
+		if (options.protocol === 'data:') {
+			const data = dataUriToBuffer(request.url);
+			const response = new Response(data, {headers: {'Content-Type': data.typeFull}});
+			resolve(response);
+			return;
+		}
+
+		// Wrap http.request into fetch
 		const send = (options.protocol === 'https:' ? https : http).request;
 		const {signal} = request;
 		let response = null;
@@ -282,4 +278,3 @@ export default async function fetch(url, options_) {
 		writeToStream(request_, request);
 	});
 }
-

src/request.js

@@ -28,26 +28,6 @@ const isRequest = object => {
 	);
 };
 
-/**
- * Wrapper around `new URL` to handle relative URLs (https://github.com/nodejs/node/issues/12682)
- *
- * @param  {string} urlStr
- * @return {void}
- */
-const parseURL = urlString => {
-	/*
-		Check whether the URL is absolute or not
-
-		Scheme: https://tools.ietf.org/html/rfc3986#section-3.1
-		Absolute URL: https://tools.ietf.org/html/rfc3986#section-4.3
-	*/
-	if (/^[a-zA-Z][a-zA-Z\d+\-.]*:/.exec(urlString)) {
-		return new URL(urlString);
-	}
-
-	throw new TypeError('Only absolute URLs are supported');
-};
-
 /**
  * Request class
  *
@@ -61,18 +41,9 @@ export default class Request extends Body {
 
 		// Normalize input and force URL to be encoded as UTF-8 (https://github.com/bitinn/node-fetch/issues/245)
 		if (isRequest(input)) {
-			parsedURL = parseURL(input.url);
+			parsedURL = new URL(input.url);
 		} else {
-			if (input && input.href) {
-				// In order to support Node.js' Url objects; though WHATWG's URL objects
-				// will fall into this branch also (since their `toString()` will return
-				// `href` property anyway)
-				parsedURL = parseURL(input.href);
-			} else {
-				// Coerce input to a string before attempting to parse
-				parsedURL = parseURL(`${input}`);
-			}
-
+			parsedURL = new URL(input);
 			input = {};
 		}
 
@@ -189,10 +160,6 @@ export const getNodeRequestOptions = request => {
 		headers.set('Accept', '*/*');
 	}
 
-	if (!/^https?:$/.test(parsedURL.protocol)) {
-		throw new TypeError('Only HTTP(S) protocols are supported');
-	}
-
 	// HTTP-network-or-cache fetch steps 2.4-2.7
 	let contentLengthValue = null;
 	if (request.body === null && /^(post|put)$/i.test(request.method)) {

test/external-encoding.js

@@ -19,7 +19,7 @@ describe('external encoding', () => {
 		it('should accept data uri of plain text', () => {
 			return fetch('data:,Hello%20World!').then(r => {
 				expect(r.status).to.equal(200);
-				expect(r.headers.get('Content-Type')).to.equal('text/plain');
+				expect(r.headers.get('Content-Type')).to.equal('text/plain;charset=US-ASCII');
 				return r.text().then(t => expect(t).to.equal('Hello World!'));
 			});
 		});
@@ -27,7 +27,7 @@ describe('external encoding', () => {
 		it('should reject invalid data uri', () => {
 			return fetch('data:@@@@').catch(error => {
 				expect(error).to.exist;
-				expect(error.message).to.include('invalid URL');
+				expect(error.message).to.include('malformed data: URI');
 			});
 		});
 	});

test/main.js

@@ -95,17 +95,17 @@ describe('node-fetch', () => {
 
 	it('should reject with error if url is protocol relative', () => {
 		const url = '//example.com/';
-		return expect(fetch(url)).to.eventually.be.rejectedWith(TypeError, 'Only absolute URLs are supported');
+		return expect(fetch(url)).to.eventually.be.rejectedWith(TypeError, /Invalid URL/);
 	});
 
 	it('should reject with error if url is relative path', () => {
 		const url = '/some/path';
-		return expect(fetch(url)).to.eventually.be.rejectedWith(TypeError, 'Only absolute URLs are supported');
+		return expect(fetch(url)).to.eventually.be.rejectedWith(TypeError, /Invalid URL/);
 	});
 
 	it('should reject with error if protocol is unsupported', () => {
 		const url = 'ftp://example.com/';
-		return expect(fetch(url)).to.eventually.be.rejectedWith(TypeError, 'Only HTTP(S) protocols are supported');
+		return expect(fetch(url)).to.eventually.be.rejectedWith(TypeError, /URL scheme "ftp" is not supported/);
 	});
 
 	itIf(process.platform !== 'win32')('should reject with error on network failure', () => {
@@ -2132,6 +2132,15 @@ describe('node-fetch', () => {
 			});
 		});
 
+		it('should accept data uri 2', async () => {
+			const r = await fetch('data:text/plain;charset=UTF-8;page=21,the%20data:1234,5678');
+			expect(r.status).to.equal(200);
+			expect(r.headers.get('Content-Type')).to.equal('text/plain;charset=UTF-8;page=21');
+
+			const b = await r.text();
+			expect(b).to.equal('the data:1234,5678');
+		});
+
 		it('should reject invalid data uri', () => {
 			return fetch(invalidDataUrl).catch(error => {
 				console.assert(error);