Actually respond with code 400 to pipelined request

hapijs · Nov 7, 2023 · c2d3018 · c2d3018
1 parent 8acc195
commit c2d3018
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 5 deletions.
diff --git a/lib/core.js b/lib/core.js
@@ -558,7 +558,14 @@ exports = module.exports = internals.Core = class {
 
                         // This parser error is for a pipelined request. Schedule destroy once current request is done.
 
-                        request.raw.res.once('close', () => socket.destroy(err));
+                        request.raw.res.once('close', () => {
+
+                            if (socket.readable) {
+                                socket.end(internals.badRequestResponse);
+                            }
+
+                            socket.destroy(err);
+                        });
                         return;
                     }
 

diff --git a/test/request.js b/test/request.js
@@ -2279,22 +2279,51 @@ describe('Request', () => {
         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: () => 'ok' });
+            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 = 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');
+            client.write('GET / HTTP/1.1\r\nHost: test\r\nContent-Length: 0\r\n\r\ninvalid data');
 
             const [request] = await log;
             expect(request.response.statusCode).to.equal(200);
-            expect(request.response.source).to.equal('ok');
+            expect(request.response.source).to.equal('PAYLOAD');
             const clientResponse = (await clientEnded).toString();
             expect(clientResponse).to.contain('HTTP/1.1 200 OK');
 
+            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.be.undefined();
+            const clientResponse = (await clientEnded).toString();
+            expect(clientResponse).to.equal('');
+
             await server.stop({ timeout: 1 });
         });
 
@@ -2318,7 +2347,7 @@ 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');
@@ -2370,6 +2399,32 @@ describe('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;