feat: avoid circular redirects

app/middleware/notfound.js

@@ -18,11 +18,19 @@ module.exports = options => {
       return;
     }
 
+    const notFoundHtml = '<h1>404 Not Found</h1>';
+
+    // notfound handler is unimplemented
+    if (options.pageUrl && this.path === options.pageUrl) {
+      this.body = `${notFoundHtml}config.notfound.pageUrl(${options.pageUrl}) is unimplemented`;
+      return;
+    }
+
     if (options.pageUrl) {
       this.realStatus = 404;
       this.redirect(options.pageUrl);
       return;
     }
-    this.body = '<h1>404 Not Found</h1>';
+    this.body = notFoundHtml;
   };
 };

test/app/middleware/notfound.test.js

@@ -1,6 +1,5 @@
 'use strict';
 
-const pedding = require('pedding');
 const request = require('supertest');
 const mm = require('egg-mock');
 const utils = require('../../utils');
@@ -51,7 +50,7 @@ describe('test/app/middleware/notfound.test.js', () => {
       .expect(404);
   });
 
-  describe('app.404.url=/404', () => {
+  describe('config.notfound.pageUrl = "/404"', () => {
     let app;
     before(() => {
       app = utils.app('apps/notfound-custom-404');
@@ -61,19 +60,32 @@ describe('test/app/middleware/notfound.test.js', () => {
 
     afterEach(mm.restore);
 
-    it('should 302 redirect to custom /404 on production env', done => {
-      done = pedding(2, done);
-
-      request(app.callback())
+    it('should 302 redirect to custom /404 when required html', function* () {
+      yield request(app.callback())
         .get('/test/404')
         .set('Accept', 'test/html')
         .expect('Location', '/404')
-        .expect(302, done);
+        .expect(302);
 
-      request(app.callback())
+      yield request(app.callback())
         .get('/404')
         .expect('Hi, this is 404')
-        .expect(200, done);
+        .expect(200);
+    });
+
+    it('should not avoid circular redirects', function* () {
+      mm(app.config.notfound, 'pageUrl', '/notfound');
+
+      yield request(app.callback())
+        .get('/test/404')
+        .set('Accept', 'test/html')
+        .expect('Location', '/notfound')
+        .expect(302);
+
+      yield request(app.callback())
+        .get('/notfound')
+        .expect('<h1>404 Not Found</h1>config.notfound.pageUrl(/notfound) is unimplemented')
+        .expect(404);
     });
   });
 });