Having trouble catching an error where payload triggers a parse error

[trippo80]

Hi,

I've just started developing with nodejs and hapijs and found a way to make my app respond in a weird way - almost like it doesn't really end the response properly.

Node version: 7.1.0
HapiJS version: 16.1.0

So, I was able to reproduce this easily with a super simple example.

'use strict';

const Hapi = require('hapi');

const server = new Hapi.Server();
server.connection({
    host: 'localhost',
    port: 3031
});

server.route({
    method: 'POST',
    path: '/hello',
    handler: (req, reply) => {
        return reply({ status: 'ok' });
    }
});

server.start((err) => {
    if (err) {
        throw err;
    }
    console.log('Server running at: ', server.info.uri);
});

Now, to trigger the problem you need a client that supports keep-alive (because we want to do two request on the same connection) - for example Postman.

Include the header "Content-Length: 0" and some payload - for example username and password - and do a POST request to http://localhost:3031/hello.

This will make node/hapi have issues parsing the payload and it makes the app behave unexpected. Basically - the first request will show the response (in this case { status: 'ok' }, but if you redo the request (while keep-alive) it will just sit there waiting.

The problem for me isn't that it triggers a parse error (because we're basically telling the api that the content length is 0 but still including a payload) - but I can't find a way to catch that issue and end the request properly (so that the next request will work exactly like the first one).

Will be happy to include any extra information needed.

[kanongil]

You want to support clients in an indeterminate state? The only thing Hapi could be expected to do is close the connection on such parse errors, preventing any further keep-alive requests.

If it doesn't already do this, a PR to fix this might be interesting, though I doubt it will be a simple task. You need to only close the reading side so that the response for the first request can be submitted successfully and, for good measure, communicate a connection: close header.

[trippo80]

@kanongil That's exactly what I want to do. Close the connection properly - and not keep it open which prevents the next request (from that client) to work properly. However - I don't know how to achieve that.

[trippo80]

Update - I actually managed to catch the error and close the socket with the following code:

server.connections[0].server.listener.on('clientError', (err, socket) => {
    socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
});

I just need to fix the message so it's a proper json message.

[AdriVanHoudt]

Does this mean that hapi does not clean up sockets with clientErrors correctly?

edit: does not seem so https://github.com/hapijs/hapi/blob/386bb944ef6881b09684b1d08bc05a5890569d7a/lib/connection.js#L95 😱

[nlf]

we're doing a little research here, but i can say that this appears to be a problem in node core

[DamodarSojka]

Is there any update on that? (hapi 16.6.2)

[kanongil]

This was fixed a long time ago in v17 with hapijs/hapi#3624. It was not deemed critical enough to be backported to v16. I suggest you update to the latest version.

[DamodarSojka]

Ok, it will take a while to migrate to 17 but it's indeed not that much of an issue. I guess it can be closed then.