Move HTTP response header validation to node

[tomyam1]

I recently got a Header value cannot contain or convert into non-ascii characters: content-disposition error after sending an attachment response for a file named café.pdf.

The error is thrown in here because the ASCII code of é is 233 and only codes smaller than 127 are allowed.

This is not coherent with the content-disposition module which allows up to code 255.

I opened the issue here. If you feel hapi is doing the correct thing, I'll open it with content-disposition.

This check was added in this commit. @jefflembeck since it's your commit, could you please have a look?

[tomyam1]

For reference: since 4.3.0 node's http module checks for invalid headers characters (itroduced in this commit) and it allows characters in the range of: 9, [32-127], [128-255].

[jefflembeck]

This commit was originally introduced to prevent HTTP Response splitting in hapi apps on node 0.12 and 4 before 4.3. node originally did some bit shifting that turned out to cause a problem in this regard. I haven't investigated to see if that particular problem has been fixed, but I can see if this likely can be cleaned up by grabbing the same range that 4.3.0 allows.

[davidkaminsky]

Is there a work-around at all for this? I'm running into it on hapi 13.4.0 and node 6.2.0.

[mtharrison]

@hueniverse would you support changing the header key/value character range to match what node does?

There are 2 issues when our validation differs to what happens in res.setHeader():

1. We disallow valid characters (see original comment)
2. We allow some invalid characters (such as [ and ])

server.route({
    method: 'GET',
    path: '/',
    handler: function (request, reply) {

        return reply().header('[header]', 'value');
    }
});

hapi won't throw on the above, but it will throw once it gets to node:

Debug: internal, implementation, error
    TypeError: Header name must be a valid HTTP Token ["[header]"]
    at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:339:11)
    ...

[kanongil]

If #3178 is merged, this can be handled by simply removing the check from hapi, trusting node to throw on these headers. Given that all supported versions of node already make this check, this should be fine, right? Or do we support all v4.x implementations?

[hueniverse]

I Rather leave this up to node. If the security issue is not longer there, we can remove it from hapi.

[mtharrison]

The strict validation wasn't added to node until 4.3.0. @kanongil not sure about your question regarding which 4.x.x versions we support (I just assumed all).

@jefflembeck are you able to confirm if the underlying security issue that prompted you to add the check has been fixed in node core >= 4.0.0?

[hueniverse]

We only support latest 4.x and 6.x. We can reflect that in the package.json file