feat: formatting redirect url on http(s) protocol url (#4)

avoid security escapes

closes eggjs/egg-security#91

.github/workflows/node.js.yml

@@ -13,4 +13,4 @@ jobs:
     uses: node-modules/github-actions/.github/workflows/node-test.yml@master
     with:
       os: 'ubuntu-latest, macos-latest, windows-latest'
-      version: '16.13.0, 16, 18, 20'
+      version: '16.13.0, 16, 18, 20, 21'

.gitignore

@@ -7,3 +7,4 @@ npm-debug.log
 dist
 lib
 !lib/application.test-d.ts
+package-lock.json

src/response.ts

@@ -206,11 +206,15 @@ export default class Response {
    *    this.redirect('back');
    *    this.redirect('back', '/index.html');
    *    this.redirect('/login');
-   *    this.redirect('http://google.com');
+   *    this.redirect('http://google.com'); // will format to 'http://google.com/'
    */
   redirect(url: string, alt?: string) {
     // location
     if (url === 'back') url = this.ctx.get<string>('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

test/response/redirect.test.ts

@@ -7,23 +7,30 @@ 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 url formatting is required before redirect', () => {
+    const ctx = context();
+    ctx.redirect('http://google.com\\@baidu.com');
+    assert.strictEqual(ctx.response.header.location, 'http://google.com/@baidu.com');
     assert.strictEqual(ctx.status, 302);
   });
 
   it('should auto fix not encode url', done => {
     const app = new Koa();
 
     app.use(ctx => {
-      ctx.redirect('http://google.com/😓');
+      ctx.redirect('https://google.com/😓?hello=你好(*´▽｀)ノノ&p=123&q=%F0%9F%98%93%3Fhello%3D%E4%BD%A0%E5%A5%BD%28');
     });
 
     request(app.callback())
       .get('/')
       .end((err, res) => {
         if (err) return done(err);
         assert.strictEqual(res.status, 302);
-        assert.strictEqual(res.headers.location, 'http://google.com/%F0%9F%98%93');
+        assert.strictEqual(res.headers.location, 'https://google.com/%F0%9F%98%93?hello=%E4%BD%A0%E5%A5%BD(*%C2%B4%E2%96%BD%EF%BD%80)%E3%83%8E%E3%83%8E&p=123&q=%F0%9F%98%93%3Fhello%3D%E4%BD%A0%E5%A5%BD%28');
         done();
       });
   });
@@ -63,7 +70,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', () => {
@@ -83,7 +90,7 @@ 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}/.`);
     });
   });
 
@@ -95,7 +102,7 @@ describe('ctx.redirect(url)', () => {
       ctx.header.accept = 'text/plain';
       ctx.redirect('http://google.com');
       assert.strictEqual(ctx.status, 301);
-      assert.strictEqual(ctx.body, `Redirecting to ${url}.`);
+      assert.strictEqual(ctx.body, `Redirecting to ${url}/.`);
     });
   });
 
@@ -107,17 +114,17 @@ describe('ctx.redirect(url)', () => {
       ctx.header.accept = 'text/plain';
       ctx.redirect('http://google.com');
       assert.strictEqual(ctx.status, 302);
-      assert.strictEqual(ctx.body, `Redirecting to ${url}.`);
+      assert.strictEqual(ctx.body, `Redirecting to ${url}/.`);
     });
   });
 
   describe('when content-type was present', () => {
     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');