Don't return error response when clientError is a pipelined HPE_INVALID_METHOD

lib/core.js

@@ -557,6 +557,25 @@ exports = module.exports = internals.Core = class {
             if (socket.readable) {
                 const request = this.settings.operations.cleanStop && this.actives.get(socket);
                 if (request) {
+
+                    // If a request is available, it means that the connection and parsing has progressed far enough to have created the request.
+
+                    if (err.code === 'HPE_INVALID_METHOD') {
+
+                        // This parser error is for a pipelined request. Schedule destroy once current request is done.
+
+                        request.raw.res.once('close', () => {
+
+                            if (socket.readable) {
+                                socket.end(internals.badRequestResponse);
+                            }
+                            else {
+                                socket.destroy(err);
+                            }
+                        });
+                        return;
+                    }
+
                     const error = Boom.badRequest();
                     error.output.headers = { connection: 'close' };
                     request._reply(error);

lib/request.js

@@ -493,17 +493,15 @@ exports = module.exports = internals.Request = class {
 
         this._eventContext.request = null;              // Disable req events
 
-        if (!Boom.isBoom(this.response)) {
+        if (this.response._close) {
             if (this.response.statusCode === 500 &&
                 this.response._error) {
 
                 const tags = this.response._error.isDeveloperError ? ['internal', 'implementation', 'error'] : ['internal', 'error'];
                 this._log(tags, this.response._error, 'error');
             }
 
-            if (this.response._close) {
-                this.response._close();
-            }
+            this.response._close();
         }
 
         this.info.completed = Date.now();
@@ -522,13 +520,11 @@ exports = module.exports = internals.Request = class {
             this.response !== response &&
             this.response.source !== response.source) {
 
-            this.response._close();
+            this.response._close?.();
         }
 
         if (this.info.completed) {
-            if (response._close) {
-                response._close();
-            }
+            response._close?.();
 
             return;
         }

test/request.js

@@ -2276,38 +2276,58 @@ describe('Request', () => {
             await server.stop({ timeout: 1 });
         });
 
-        it('returns a logged bad request error when parser fails after request is setup', async () => {
+        it('returns normal response when parser fails with bad method after request is setup', async () => {
 
             const server = Hapi.server({ routes: { timeout: { server: false } } });
-            server.route({ path: '/', method: 'GET', handler: Hoek.block });
+            server.route({ path: '/', method: 'GET', handler: () => 'PAYLOAD' });
             await server.start();
 
             const log = server.events.once('response');
             const client = Net.connect(server.info.port);
-            const clientEnded = new Promise((resolve, reject) => {
+            const clientEnded = Wreck.read(client);
 
-                let response = '';
-                client.on('data', (chunk) => {
+            await new Promise((resolve) => client.on('connect', resolve));
+            client.write('GET / HTTP/1.1\r\nHost: test\r\nContent-Length: 0\r\n\r\ninvalid data');
 
-                    response = response + chunk.toString();
-                });
+            const [request] = await log;
+            expect(request.response.statusCode).to.equal(200);
+            expect(request.response.source).to.equal('PAYLOAD');
+            const clientResponse = (await clientEnded).toString();
+            expect(clientResponse).to.contain('HTTP/1.1 200 OK');
 
-                client.on('end', () => resolve(response));
-                client.on('error', reject);
-            });
+            const nextResponse = clientResponse.slice(clientResponse.indexOf('PAYLOAD') + 7);
+            expect(nextResponse).to.startWith('HTTP/1.1 400 Bad Request');
+
+            await server.stop({ timeout: 1 });
+        });
+
+        it('returns nothing when parser fails with bad method after request is setup and the connection is closed', async () => {
+
+            const server = Hapi.server({ routes: { timeout: { server: false } } });
+            server.route({ path: '/', method: 'GET', handler: (request, h) => {
+
+                request.raw.res.destroy();
+                return h.abandon;
+            } });
+
+            await server.start();
+
+            const log = server.events.once('response');
+            const client = Net.connect(server.info.port);
+            const clientEnded = Wreck.read(client);
 
             await new Promise((resolve) => client.on('connect', resolve));
             client.write('GET / HTTP/1.1\r\nHost: test\r\nContent-Length: 0\r\n\r\n\r\ninvalid data');
 
             const [request] = await log;
-            expect(request.response.statusCode).to.equal(400);
-            const clientResponse = await clientEnded;
-            expect(clientResponse).to.contain('400 Bad Request');
+            expect(request.response.statusCode).to.be.undefined();
+            const clientResponse = (await clientEnded).toString();
+            expect(clientResponse).to.equal('');
 
             await server.stop({ timeout: 1 });
         });
 
-        it('returns a logged bad request error when parser fails after request is setup (cleanStop false)', async () => {
+        it('returns a bad request when parser fails after request is setup (cleanStop false)', async () => {
 
             const server = Hapi.server({ routes: { timeout: { server: false } }, operations: { cleanStop: false } });
             server.route({ path: '/', method: 'GET', handler: Hoek.block });
@@ -2327,14 +2347,84 @@ describe('Request', () => {
             });
 
             await new Promise((resolve) => client.on('connect', resolve));
-            client.write('GET / HTTP/1.1\r\nHost: test\nContent-Length: 0\r\n\r\n\r\ninvalid data');
+            client.write('GET / HTTP/1.1\r\nHost: test\nContent-Length: 0\r\n\r\ninvalid data');
 
             const clientResponse = await clientEnded;
             expect(clientResponse).to.contain('400 Bad Request');
 
             await server.stop({ timeout: 1 });
         });
 
+        it('returns a bad request for POST request when chunked parsing fails', async () => {
+
+            const server = Hapi.server({ routes: { timeout: { server: false } } });
+            server.route({ path: '/', method: 'POST', handler: () => 'ok', options: { payload: { parse: true } } });
+            await server.start();
+
+            const log = server.events.once('response');
+            const client = Net.connect(server.info.port);
+            const clientEnded = Wreck.read(client);
+
+            await new Promise((resolve) => client.on('connect', resolve));
+            client.write('POST / HTTP/1.1\r\nHost: test\r\nTransfer-Encoding: chunked\r\n\r\n');
+            await Hoek.wait(10);
+            client.write('not chunked\r\n');
+
+            const [request] = await log;
+            expect(request.response.statusCode).to.equal(400);
+            expect(request.response.source).to.contain({ error: 'Bad Request' });
+            const clientResponse = (await clientEnded).toString();
+            expect(clientResponse).to.contain('400 Bad Request');
+
+            await server.stop({ timeout: 1 });
+        });
+
+        it('returns a bad request for POST request when chunked parsing fails (cleanStop false)', async () => {
+
+            const server = Hapi.server({ routes: { timeout: { server: false } }, operations: { cleanStop: false } });
+            server.route({ path: '/', method: 'POST', handler: () => 'ok', options: { payload: { parse: true } } });
+            await server.start();
+
+            const client = Net.connect(server.info.port);
+            const clientEnded = Wreck.read(client);
+
+            await new Promise((resolve) => client.on('connect', resolve));
+            client.write('POST / HTTP/1.1\r\nHost: test\r\nTransfer-Encoding: chunked\r\n\r\n');
+            await Hoek.wait(10);
+            client.write('not chunked\r\n');
+
+            const clientResponse = (await clientEnded).toString();
+            expect(clientResponse).to.contain('400 Bad Request');
+
+            await server.stop({ timeout: 1 });
+        });
+
+        it('returns a bad request for POST request when chunked parsing fails', async () => {
+
+            const server = Hapi.server({ routes: { timeout: { server: false } } });
+            server.route({ path: '/', method: 'POST', handler: () => 'ok', options: { payload: { parse: true } } });
+            await server.start();
+
+            const log = server.events.once('response');
+            const client = Net.connect(server.info.port);
+            const clientEnded = Wreck.read(client);
+
+            await new Promise((resolve) => client.on('connect', resolve));
+            client.write('POST / HTTP/1.1\r\nHost: test\r\nContent-Length: 5\r\n\r\n');
+            await Hoek.wait(10);
+            client.write('111A1');        // Doesn't work if 'A' is replaced with '1' !?!
+            client.write('\Q\r\n');       // Extra bytes considered to be start of next request
+            client.end();
+
+            const [request] = await log;
+            expect(request.response.statusCode).to.equal(400);
+            expect(request.response.source).to.contain({ error: 'Bad Request' });
+            const clientResponse = (await clientEnded).toString();
+            expect(clientResponse).to.contain('400 Bad Request');
+
+            await server.stop({ timeout: 1 });
+        });
+
         it('does not return an error when server is responding when the timeout occurs', async () => {
 
             let ended = false;