From 84a343728d65d674a7e1aab7e8e5a8b02cc5bf60 Mon Sep 17 00:00:00 2001
From: Van Tigranyan <vtigranyan@adobe.com>
Date: Fri, 27 Oct 2023 12:33:30 +0400
Subject: [PATCH] chore: add negative test case for followRedirect function

---
 .gitignore              |  1 +
 source/core/index.ts    |  2 +-
 source/core/response.ts |  4 +++-
 test/redirects.ts       | 26 +++++++++++++++++++++++++-
 4 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index bd4471e12..5844f7abb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,4 @@ coverage
 .nyc_output
 dist
 *.0x
+.idea
diff --git a/source/core/index.ts b/source/core/index.ts
index a04390ca7..87b9ee377 100644
--- a/source/core/index.ts
+++ b/source/core/index.ts
@@ -336,7 +336,7 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 		void (async () => {
 			// Node.js parser is really weird.
 			// It emits post-request Parse Errors on the same instance as previous request. WTF.
-			// Therefore we need to check if it has been destroyed as well.
+			// Therefore, we need to check if it has been destroyed as well.
 			//
 			// Furthermore, Node.js 16 `response.destroy()` doesn't immediately destroy the socket,
 			// but makes the response unreadable. So we additionally need to check `response.readable`.
diff --git a/source/core/response.ts b/source/core/response.ts
index 1ec3e4845..e883b79e3 100644
--- a/source/core/response.ts
+++ b/source/core/response.ts
@@ -114,7 +114,9 @@ export type Response<T = unknown> = {
 
 export const isResponseOk = (response: PlainResponse): boolean => {
 	const {statusCode} = response;
-	const limitStatusCode = response.request.options.followRedirect ? 299 : 399;
+	const {followRedirect} = response.request.options;
+	const shouldFollow = typeof followRedirect === 'function' ? followRedirect(response) : followRedirect;
+	const limitStatusCode = shouldFollow ? 299 : 399;
 
 	return (statusCode >= 200 && statusCode <= limitStatusCode) || statusCode === 304;
 };
diff --git a/test/redirects.ts b/test/redirects.ts
index 9a9b13051..805182387 100644
--- a/test/redirects.ts
+++ b/test/redirects.ts
@@ -89,7 +89,16 @@ test('follows redirect', withServer, async (t, server, got) => {
 	t.deepEqual(redirectUrls.map(String), [`${server.url}/`]);
 });
 
-test('follows redirect when followRedirect returns true', withServer, async (t, server, got) => {
+test('does not follow redirect when followRedirect is a function and returns false', withServer, async (t, server, got) => {
+	server.get('/', reachedHandler);
+	server.get('/finite', finiteHandler);
+
+	const {body, statusCode} = await got('finite', {followRedirect: () => false});
+	t.not(body, 'reached');
+	t.is(statusCode, 302);
+});
+
+test('follows redirect when followRedirect is a function and returns true', withServer, async (t, server, got) => {
 	server.get('/', reachedHandler);
 	server.get('/finite', finiteHandler);
 
@@ -98,6 +107,21 @@ test('follows redirect when followRedirect returns true', withServer, async (t,
 	t.deepEqual(redirectUrls.map(String), [`${server.url}/`]);
 });
 
+test('followRedirect gets plainResponse and does not follow', withServer, async (t, server, got) => {
+	server.get('/temporary', (_request, response) => {
+		response.writeHead(307, {
+			location: '/redirect',
+		});
+		response.end();
+	});
+
+	const {statusCode} = await got('temporary', {followRedirect(response) {
+		t.is(response.headers.location, '/redirect');
+		return false;
+	}});
+	t.is(statusCode, 307);
+});
+
 test('follows 307, 308 redirect', withServer, async (t, server, got) => {
 	server.get('/', reachedHandler);