feat: add 400 response for broken client request to instead of empty …

…response (#1829)
eggjs · Dec 15, 2017 · 40df153 · 40df153
1 parent d0ee9f2
commit 40df153
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 1 deletion.
diff --git a/.gitignore b/.gitignore
@@ -24,3 +24,4 @@ docs/plugins.png
 package-lock.json
 yarn.lock
 !test/fixtures/apps/loader-plugin/node_modules
+.editorconfig
diff --git a/lib/application.js b/lib/application.js
@@ -19,6 +19,20 @@ const EGG_LOADER = Symbol.for('egg#loader');
 const EGG_PATH = Symbol.for('egg#eggPath');
 const CLUSTER_CLIENTS = Symbol.for('egg#clusterClients');
 
+// client error => 400 Bad Request
+// Refs: https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_event_clienterror
+const DEFAULT_BAD_REQUEST_HTML = `<html>
+  <head><title>400 Bad Request</title></head>
+  <body bgcolor="white">
+  <center><h1>400 Bad Request</h1></center>
+  <hr><center>❤</center>
+  </body>
+  </html>`;
+const DEFAULT_BAD_REQUEST_HTML_LENGTH = Buffer.byteLength(DEFAULT_BAD_REQUEST_HTML);
+const DEFAULT_BAD_REQUEST_RESPONSE =
+  `HTTP/1.1 400 Bad Request\r\nContent-Length: ${DEFAULT_BAD_REQUEST_HTML_LENGTH}` +
+  `\r\n\r\n${DEFAULT_BAD_REQUEST_HTML}`;
+
 /**
  * Singleton instance in App Worker, extend {@link EggApplication}
  * @extends EggApplication
@@ -55,6 +69,18 @@ class Application extends EggApplication {
     return path.join(__dirname, '..');
   }
 
+  onClientError(err, socket) {
+    this.logger.error('A client (%s:%d) error [%s] occurred: %s',
+      socket.remoteAddress,
+      socket.remotePort,
+      err.code,
+      err.message);
+
+    // because it's a raw socket object, we should return the raw HTTP response
+    // packet.
+    socket.end(DEFAULT_BAD_REQUEST_RESPONSE);
+  }
+
   onServer(server) {
     /* istanbul ignore next */
     graceful({
@@ -66,6 +92,8 @@ class Application extends EggApplication {
         this.coreLogger.error(err);
       },
     });
+
+    server.on('clientError', (err, socket) => this.onClientError(err, socket));
   }
 
   /**
@@ -247,7 +275,7 @@ class Application extends EggApplication {
       ctx.coreLogger.error(err);
     });
     // expose server to support websocket
-    this.on('server', server => this.onServer(server));
+    this.once('server', server => this.onServer(server));
   }
 
   /**

diff --git a/test/lib/cluster/app_worker.test.js b/test/lib/cluster/app_worker.test.js
@@ -19,6 +19,49 @@ describe('test/lib/cluster/app_worker.test.js', () => {
       .expect('true');
   });
 
+  it('should response 400 bad request when HTTP request packet broken', async () => {
+    const test1 = app.httpRequest()
+      // Node.js (http-parser) will occur an error while the raw URI in HTTP
+      // request packet containing space.
+      //
+      // Refs: https://zhuanlan.zhihu.com/p/31966196
+      .get('/foo bar');
+    const test2 = app.httpRequest().get('/foo baz');
+
+    // app.httpRequest().expect() will encode the uri so that we cannot
+    // request the server with raw `/foo bar` to emit 400 status code.
+    //
+    // So we generate `test.req` via `test.request()` first and override the
+    // encoded uri.
+    //
+    // `test.req` will only generated once:
+    //
+    //   ```
+    //   function Request::request() {
+    //     if (this.req) return this.req;
+    //
+    //     // code to generate this.req
+    //
+    //     return this.req;
+    //   }
+    //   ```
+    test1.request().path = '/foo bar';
+    test2.request().path = '/foo baz';
+
+    const html = `<html>
+  <head><title>400 Bad Request</title></head>
+  <body bgcolor="white">
+  <center><h1>400 Bad Request</h1></center>
+  <hr><center>❤</center>
+  </body>
+  </html>`;
+
+    await Promise.all([
+      test1.expect(html).expect(400),
+      test2.expect(html).expect(400),
+    ]);
+  });
+
   describe('listen hostname', () => {
     let app;
     before(() => {