Harden logic

Author: sindresorhus

source/core/Ky.ts

@@ -37,6 +37,7 @@ import {
 
 const maxErrorResponseBodySize = 10 * 1024 * 1024;
 const prefixUrlRenamedErrorMessage = 'The `prefixUrl` option has been renamed `prefix` in v2 and enhanced to allow slashes in input. See also the new `baseUrl` option for improved flexibility with standard URL resolution: https://github.com/sindresorhus/ky#baseurl';
+const timedOutResponseData = Symbol('timedOutResponseData');
 
 const createTextDecoder = (contentType: string): TextDecoder => {
 	const match = /;\s*charset\s*=\s*(?:"([^"]+)"|([^;,\s]+))/i.exec(contentType);
@@ -505,8 +506,15 @@ export class Ky {
 	async #getResponseData(response: Response): Promise<unknown> {
 		// Even with request timeouts disabled, bound error-body reads so retries and error propagation
 		// cannot be stalled indefinitely by never-ending response streams.
-		const errorDataTimeout = this.#options.timeout === false ? 10_000 : this.#options.timeout;
-		const text = await this.#readResponseText(response, errorDataTimeout);
+		const readTimeout = this.#getErrorDataTimeout();
+		const text = await this.#readResponseText(response, readTimeout.timeout);
+		if (text === timedOutResponseData) {
+			if (readTimeout.totalTimeoutReachedOnTimeout) {
+				throw new TimeoutError(this.request);
+			}
+
+			return undefined;
+		}
 
 		if (!text) {
 			return undefined;
@@ -516,7 +524,38 @@ export class Ky {
 			return text;
 		}
 
-		return this.#parseJson(text, response, errorDataTimeout);
+		const parseTimeout = this.#getErrorDataTimeout();
+		const data = await this.#parseJson(text, response, parseTimeout.timeout);
+		if (data === timedOutResponseData) {
+			if (parseTimeout.totalTimeoutReachedOnTimeout) {
+				throw new TimeoutError(this.request);
+			}
+
+			return undefined;
+		}
+
+		return data;
+	}
+
+	#getErrorDataTimeout(): {timeout: number; totalTimeoutReachedOnTimeout: boolean} {
+		const errorDataTimeout = this.#options.timeout === false ? 10_000 : this.#options.timeout;
+		const remainingTotal = this.#getRemainingTotalTimeout();
+
+		if (remainingTotal === undefined) {
+			return {
+				timeout: errorDataTimeout,
+				totalTimeoutReachedOnTimeout: false,
+			};
+		}
+
+		if (remainingTotal <= 0) {
+			throw new TimeoutError(this.request);
+		}
+
+		return {
+			timeout: Math.min(errorDataTimeout, remainingTotal),
+			totalTimeoutReachedOnTimeout: remainingTotal <= errorDataTimeout,
+		};
 	}
 
 	#isJsonContentType(contentType: string): boolean {
@@ -525,7 +564,7 @@ export class Ky {
 		return /\/(?:.*[.+-])?json$/.test(mimeType);
 	}
 
-	async #readResponseText(response: Response, timeoutMs: number): Promise<string | undefined> {
+	async #readResponseText(response: Response, timeoutMs: number): Promise<string | typeof timedOutResponseData | undefined> {
 		const {body} = response;
 		if (!body) {
 			try {
@@ -572,17 +611,17 @@ export class Ky {
 			return chunks.join('');
 		})();
 
-		const timeoutPromise = new Promise<undefined>(resolve => {
+		const timeoutPromise = new Promise<typeof timedOutResponseData>(resolve => {
 			const timeoutId = setTimeout(() => {
-				resolve(undefined);
+				resolve(timedOutResponseData);
 			}, timeoutMs);
 			void readAll.finally(() => {
 				clearTimeout(timeoutId);
 			});
 		});
 
 		const result = await Promise.race([readAll, timeoutPromise]);
-		if (result === undefined) {
+		if (result === timedOutResponseData) {
 			void reader.cancel().catch(() => undefined);
 		}
 
@@ -597,9 +636,9 @@ export class Ky {
 					? this.#options.parseJson(text, {request: this.request, response})
 					: JSON.parse(text),
 				),
-				new Promise<undefined>(resolve => {
+				new Promise<typeof timedOutResponseData>(resolve => {
 					timeoutId = setTimeout(() => {
-						resolve(undefined);
+						resolve(timedOutResponseData);
 					}, timeoutMs);
 				}),
 			]);

test/retry.ts

@@ -1678,6 +1678,113 @@ test('totalTimeout with timeout: false - exceeds total budget', async t => {
 	t.is(requestCount, 1);
 });
 
+test('totalTimeout bounds hanging HTTPError body reads when timeout is disabled', async t => {
+	t.timeout(2000);
+	let requestCount = 0;
+
+	const customFetch: typeof fetch = async () => {
+		requestCount++;
+
+		const body = new ReadableStream<Uint8Array>({
+			start(controller) {
+				controller.enqueue(new TextEncoder().encode('{"error":"partial"'));
+			},
+		});
+
+		return new Response(body, {
+			status: 500,
+			headers: {'content-type': 'application/json'},
+		});
+	};
+
+	await t.throwsAsync(
+		ky('https://example.com', {
+			fetch: customFetch,
+			timeout: false,
+			totalTimeout: 50,
+			retry: {
+				limit: 5,
+				delay: () => 0,
+			},
+		}).text(),
+		{
+			name: 'TimeoutError',
+		},
+	);
+
+	t.is(requestCount, 1);
+});
+
+test('totalTimeout bounds hanging HTTPError body reads when timeout is larger', async t => {
+	t.timeout(2000);
+	let requestCount = 0;
+
+	const customFetch: typeof fetch = async () => {
+		requestCount++;
+
+		const body = new ReadableStream<Uint8Array>({
+			start(controller) {
+				controller.enqueue(new TextEncoder().encode('{"error":"partial"'));
+			},
+		});
+
+		return new Response(body, {
+			status: 500,
+			headers: {'content-type': 'application/json'},
+		});
+	};
+
+	await t.throwsAsync(
+		ky('https://example.com', {
+			fetch: customFetch,
+			timeout: 1000,
+			totalTimeout: 50,
+			retry: {
+				limit: 5,
+				delay: () => 0,
+			},
+		}).text(),
+		{
+			name: 'TimeoutError',
+		},
+	);
+
+	t.is(requestCount, 1);
+});
+
+test('totalTimeout bounds hanging HTTPError parseJson when timeout is disabled', async t => {
+	t.timeout(2000);
+	let requestCount = 0;
+
+	const customFetch: typeof fetch = async () => {
+		requestCount++;
+		return new Response('{"error":"parse-timeout"}', {
+			status: 500,
+			headers: {'content-type': 'application/json'},
+		});
+	};
+
+	await t.throwsAsync(
+		ky('https://example.com', {
+			fetch: customFetch,
+			timeout: false,
+			totalTimeout: 50,
+			parseJson: async () => new Promise<never>(() => {
+				// Intentionally never settles
+			}),
+			retry: {
+				limit: 5,
+				delay: () => 0,
+			},
+		}).text(),
+		{
+			name: 'TimeoutError',
+		},
+	);
+
+	t.is(requestCount, 1);
+});
+
 test('totalTimeout smaller than timeout - effective timeout is capped', async t => {
 	let requestCount = 0;