fix(fetch): align Request with spec (#85)

* fix: align with spec for Request method normalization * chore: fix changeset --------- Co-authored-by: Matt Brophy <[email protected]>
web-std · Aug 28, 2023 · ace4223 · ace4223
1 parent c1339b9
commit ace4223
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 11 deletions.
diff --git a/.changeset/align-method-to-spec.md b/.changeset/align-method-to-spec.md
@@ -0,0 +1,9 @@
+---
+"@web-std/fetch": patch
+---
+
+Align with [spec](https://fetch.spec.whatwg.org/#methods) for `new Request()` `method` normalization
+
+- Only `DELETE`, `GET`, `HEAD`, `OPTIONS`, `POST`, `PUT` get automatically uppercased
+- Note that `method: "patch"` will no longer be automatically uppercased
+- Throw a `TypeError` for `CONNECT`, `TRACE`, and `TRACK`
diff --git a/packages/fetch/src/request.js b/packages/fetch/src/request.js
@@ -15,6 +15,9 @@ import {getSearch} from './utils/get-search.js';
 
 const INTERNALS = Symbol('Request internals');
 
+const forbiddenMethods = new Set(["CONNECT", "TRACE", "TRACK"]);
+const normalizedMethods = new Set(["DELETE", "GET", "HEAD", "OPTIONS", "POST", "PUT"]);
+
 /**
  * Check if `obj` is an instance of Request.
  *
@@ -32,15 +35,15 @@ const isRequest = object => {
 /**
  * Request class
  * @implements {globalThis.Request}
- * 
+ *
  * @typedef {Object} RequestState
  * @property {string} method
  * @property {RequestRedirect} redirect
  * @property {globalThis.Headers} headers
  * @property {RequestCredentials} credentials
  * @property {URL} parsedURL
  * @property {AbortSignal|null} signal
- * 
+ *
  * @typedef {Object} RequestExtraOptions
  * @property {number} [follow]
  * @property {boolean} [compress]
@@ -49,15 +52,15 @@ const isRequest = object => {
  * @property {Agent} [agent]
  * @property {number} [highWaterMark]
  * @property {boolean} [insecureHTTPParser]
- * 
+ *
  * @typedef {((url:URL) => import('http').Agent) | import('http').Agent} Agent
- * 
+ *
  * @typedef {Object} RequestOptions
  * @property {string} [method]
  * @property {ReadableStream<Uint8Array>|null} [body]
  * @property {globalThis.Headers} [headers]
  * @property {RequestRedirect} [redirect]
- * 
+ *
  */
 export default class Request extends Body {
 	/**
@@ -80,8 +83,13 @@ export default class Request extends Body {
 
 
 
+		// Normalize method: https://fetch.spec.whatwg.org/#methods
 		let method = init.method || settings.method || 'GET';
-		method = method.toUpperCase();
+		if (forbiddenMethods.has(method.toUpperCase())) {
+			throw new TypeError(`Failed to construct 'Request': '${method}' HTTP method is unsupported.`)
+		} else if (normalizedMethods.has(method.toUpperCase())) {
+			method = method.toUpperCase();
+		}
 
 		const inputBody = init.body != null
 			? init.body
@@ -99,7 +107,7 @@ export default class Request extends Body {
 		});
 		const input = settings
 
-		
+
 		const headers = /** @type {globalThis.Headers} */
 			(new Headers(init.headers || input.headers || {}));
 
@@ -170,11 +178,11 @@ export default class Request extends Body {
 	get destination() {
 		return ""
 	}
-	
+
 	get integrity() {
 		return ""
 	}
-	
+
 	/** @type {RequestMode} */
 	get mode() {
 		return "cors"
@@ -184,7 +192,7 @@ export default class Request extends Body {
 	get referrer() {
 		return  ""
 	}
-	
+
 	/** @type {ReferrerPolicy} */
 	get referrerPolicy() {
 		return ""
@@ -308,7 +316,7 @@ export const getNodeRequestOptions = request => {
 		port: parsedURL.port,
 		hash: parsedURL.hash,
 		search: parsedURL.search,
-		// @ts-ignore - it does not has a query 
+		// @ts-ignore - it does not has a query
 		query: parsedURL.query,
 		href: parsedURL.href,
 		method: request.method,

diff --git a/packages/fetch/test/request.js b/packages/fetch/test/request.js
@@ -86,6 +86,59 @@ describe('Request', () => {
 		expect(r2.counter).to.equal(0);
 	});
 
+	it('should throw a TypeError for forbidden methods', () => {
+		// https://fetch.spec.whatwg.org/#methods
+		const forbiddenMethods = [
+			"CONNECT",
+			"TRACE",
+			"TRACK",
+		];
+
+		forbiddenMethods.forEach(method => {
+			try {
+				new Request(base, { method: method.toLowerCase() });
+				expect(true).to.equal(false);
+			} catch (e) {
+				expect(e instanceof TypeError).to.equal(true);
+				expect(e.message).to.equal(`Failed to construct 'Request': '${method.toLowerCase()}' HTTP method is unsupported.`)
+			}
+			try {
+				new Request(base, { method: method.toUpperCase() });
+				expect(true).to.equal(false);
+			} catch (e) {
+				expect(e instanceof TypeError).to.equal(true);
+				expect(e.message).to.equal(`Failed to construct 'Request': '${method.toUpperCase()}' HTTP method is unsupported.`)
+			}
+		});
+	});
+
+	it('should normalize method', () => {
+		// https://fetch.spec.whatwg.org/#methods
+		const shouldUpperCaseMethods = [
+			"DELETE",
+			"GET",
+			"HEAD",
+			"OPTIONS",
+			"POST",
+			"PUT",
+		];
+		const otherMethods = ["PATCH", "CHICKEN"];
+
+		shouldUpperCaseMethods.forEach(method => {
+			const r1 = new Request(base, { method: method.toLowerCase() });
+			expect(r1.method).to.equal(method.toUpperCase());
+			const r2 = new Request(base, { method: method.toUpperCase() });
+			expect(r2.method).to.equal(method.toUpperCase());
+		});
+
+		otherMethods.forEach(method => {
+			const r1 = new Request(base, { method: method.toLowerCase() });
+			expect(r1.method).to.equal(method.toLowerCase());
+			const r2 = new Request(base, { method: method.toUpperCase() });
+			expect(r2.method).to.equal(method.toUpperCase());
+		});
+	});
+
 	it('should override signal on derived Request instances', () => {
 		const parentAbortController = new AbortController();
 		const derivedAbortController = new AbortController();