From 2510b0b12285b96b5d3bbe375c7fd283df4b2a9d Mon Sep 17 00:00:00 2001
From: fengmk2 <fengmk2@gmail.com>
Date: Fri, 15 Mar 2024 14:45:44 +0800
Subject: [PATCH] fix: formatting redirect url on http(s) protocol url (#1803)

avoid security escapes
---
 .github/workflows/node.js.yml  |  2 +-
 __tests__/response/redirect.js | 25 ++++++++++++++++---------
 lib/response.js                |  4 ++++
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml
index c987956c8..175e54929 100644
--- a/.github/workflows/node.js.yml
+++ b/.github/workflows/node.js.yml
@@ -13,7 +13,7 @@ jobs:
 
     strategy:
       matrix:
-        node-version: [12.x, 14.x, 16.x, 18.x]
+        node-version: [12.x, 14.x, 16.x, 18.x, 20.x, 21.x]
 
     steps:
     - uses: actions/checkout@v2
diff --git a/__tests__/response/redirect.js b/__tests__/response/redirect.js
index 362f743e1..db2844460 100644
--- a/__tests__/response/redirect.js
+++ b/__tests__/response/redirect.js
@@ -10,7 +10,14 @@ describe('ctx.redirect(url)', () => {
   it('should redirect to the given url', () => {
     const ctx = context();
     ctx.redirect('http://google.com');
-    assert.strictEqual(ctx.response.header.location, 'http://google.com');
+    assert.strictEqual(ctx.response.header.location, 'http://google.com/');
+    assert.strictEqual(ctx.status, 302);
+  });
+
+  it('should formatting url before redirect', () => {
+    const ctx = context();
+    ctx.redirect('http://google.com\\@apple.com');
+    assert.strictEqual(ctx.response.header.location, 'http://google.com/@apple.com');
     assert.strictEqual(ctx.status, 302);
   });
 
@@ -66,7 +73,7 @@ describe('ctx.redirect(url)', () => {
       ctx.header.accept = 'text/html';
       ctx.redirect(url);
       assert.strictEqual(ctx.response.header['content-type'], 'text/html; charset=utf-8');
-      assert.strictEqual(ctx.body, `Redirecting to <a href="${url}">${url}</a>.`);
+      assert.strictEqual(ctx.body, `Redirecting to <a href="${url}/">${url}/</a>.`);
     });
 
     it('should escape the url', () => {
@@ -86,17 +93,17 @@ describe('ctx.redirect(url)', () => {
       const url = 'http://google.com';
       ctx.header.accept = 'text/plain';
       ctx.redirect(url);
-      assert.strictEqual(ctx.body, `Redirecting to ${url}.`);
+      assert.strictEqual(ctx.body, `Redirecting to ${url}/.`);
     });
   });
 
   describe('when status is 301', () => {
     it('should not change the status code', () => {
       const ctx = context();
-      const url = 'http://google.com';
+      const url = 'http://google.com/';
       ctx.status = 301;
       ctx.header.accept = 'text/plain';
-      ctx.redirect('http://google.com');
+      ctx.redirect(url);
       assert.strictEqual(ctx.status, 301);
       assert.strictEqual(ctx.body, `Redirecting to ${url}.`);
     });
@@ -108,9 +115,9 @@ describe('ctx.redirect(url)', () => {
       const url = 'http://google.com';
       ctx.status = 304;
       ctx.header.accept = 'text/plain';
-      ctx.redirect('http://google.com');
+      ctx.redirect(url);
       assert.strictEqual(ctx.status, 302);
-      assert.strictEqual(ctx.body, `Redirecting to ${url}.`);
+      assert.strictEqual(ctx.body, `Redirecting to ${url}/.`);
     });
   });
 
@@ -118,9 +125,9 @@ describe('ctx.redirect(url)', () => {
     it('should overwrite content-type', () => {
       const ctx = context();
       ctx.body = {};
-      const url = 'http://google.com';
+      const url = 'http://google.com/foo/bar';
       ctx.header.accept = 'text/plain';
-      ctx.redirect('http://google.com');
+      ctx.redirect(url);
       assert.strictEqual(ctx.status, 302);
       assert.strictEqual(ctx.body, `Redirecting to ${url}.`);
       assert.strictEqual(ctx.type, 'text/plain');
diff --git a/lib/response.js b/lib/response.js
index 54f9d49f7..2eec2dc26 100644
--- a/lib/response.js
+++ b/lib/response.js
@@ -261,6 +261,10 @@ module.exports = {
   redirect(url, alt) {
     // location
     if ('back' === url) url = this.ctx.get('Referrer') || alt || '/';
+    if (url.startsWith('https://') || url.startsWith('http://')) {
+      // formatting url again avoid security escapes
+      url = new URL(url).toString();
+    }
     this.set('Location', encodeUrl(url));
 
     // status