From 6192c9892f3eddf165a4826ba917ea08ea627ea2 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 24 Aug 2015 11:58:48 -0700 Subject: [PATCH] http: add checkIsHttpToken check for header fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ref: https://github.com/nodejs/node-convergence-archive/issues/13 This adds a new check for header and trailer fields names and method names to ensure that they conform to the HTTP token rule. If they do not, a `TypeError` is thrown. Previously this had an additional `strictMode` option that has been removed in favor of making the strict check the default (and only) behavior. Doc and test case are included. On the client-side ```javascript var http = require('http'); var url = require('url'); var p = url.parse('http://localhost:8888'); p.headers = {'testing 123': 123}; http.client(p, function(res) { }); // throws ``` On the server-side ```javascript var http = require('http'); var server = http.createServer(function(req,res) { res.setHeader('testing 123', 123); // throws res.end('...'); }); ``` Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Trevor Norris Reviewed-By: Сковорода Никита Андреевич Reviewed-By: Rod Vagg PR-URL: https://github.com/nodejs/node/pull/2526 --- doc/api/http.markdown | 5 ++ lib/_http_client.js | 3 + lib/_http_common.js | 10 ++++ lib/_http_outgoing.js | 12 +++- test/parallel/test-http-invalidheaderfield.js | 56 +++++++++++++++++++ 5 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http-invalidheaderfield.js diff --git a/doc/api/http.markdown b/doc/api/http.markdown index 8c9e6babb64a04..8ac89b1f388a59 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -364,6 +364,9 @@ or response.setHeader("Set-Cookie", ["type=ninja", "language=javascript"]); +Attempting to set a header field name that contains invalid characters will +result in a `TypeError` being thrown. + ### response.headersSent Boolean (read-only). True if headers were sent, false otherwise. @@ -439,6 +442,8 @@ emit trailers, with a list of the header fields in its value. E.g., response.addTrailers({'Content-MD5': "7895bf4b8828b55ceaf47747b4bca667"}); response.end(); +Attempting to set a trailer field name that contains invalid characters will +result in a `TypeError` being thrown. ### response.end([data][, encoding][, callback]) diff --git a/lib/_http_client.js b/lib/_http_client.js index aef05eb8fa2299..201593e61da5ac 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -67,6 +67,9 @@ function ClientRequest(options, cb) { self.socketPath = options.socketPath; var method = self.method = (options.method || 'GET').toUpperCase(); + if (!common._checkIsHttpToken(method)) { + throw new TypeError('Method must be a valid HTTP token'); + } self.path = options.path || '/'; if (cb) { self.once('response', cb); diff --git a/lib/_http_common.js b/lib/_http_common.js index 757032929444b1..addf9a0fa4f5d0 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -195,3 +195,13 @@ function httpSocketSetup(socket) { socket.on('drain', ondrain); } exports.httpSocketSetup = httpSocketSetup; + +/** + * Verifies that the given val is a valid HTTP token + * per the rules defined in RFC 7230 + **/ +const token = /^[a-zA-Z0-9_!#$%&'*+.^`|~-]+$/; +function checkIsHttpToken(val) { + return typeof val === 'string' && token.test(val); +} +exports._checkIsHttpToken = checkIsHttpToken; diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 0522ecb2a276f2..ddbd6c3828aaa8 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -295,6 +295,10 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) { }; function storeHeader(self, state, field, value) { + if (!common._checkIsHttpToken(field)) { + throw new TypeError( + 'Header name must be a valid HTTP Token ["' + field + '"]'); + } value = escapeHeaderValue(value); state.messageHeader += field + ': ' + value + CRLF; @@ -323,6 +327,9 @@ function storeHeader(self, state, field, value) { OutgoingMessage.prototype.setHeader = function(name, value) { + if (!common._checkIsHttpToken(name)) + throw new TypeError( + 'Header name must be a valid HTTP Token ["' + name + '"]'); if (typeof name !== 'string') throw new TypeError('`name` should be a string in setHeader(name, value).'); if (value === undefined) @@ -498,7 +505,10 @@ OutgoingMessage.prototype.addTrailers = function(headers) { field = key; value = headers[key]; } - + if (!common._checkIsHttpToken(field)) { + throw new TypeError( + 'Trailer name must be a valid HTTP Token ["' + field + '"]'); + } this._trailer += field + ': ' + escapeHeaderValue(value) + CRLF; } }; diff --git a/test/parallel/test-http-invalidheaderfield.js b/test/parallel/test-http-invalidheaderfield.js new file mode 100644 index 00000000000000..7e130e50d86d2d --- /dev/null +++ b/test/parallel/test-http-invalidheaderfield.js @@ -0,0 +1,56 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const EventEmitter = require('events'); +const http = require('http'); + +const ee = new EventEmitter(); +var count = 3; + +const server = http.createServer(function(req, res) { + assert.doesNotThrow(function() { + res.setHeader('testing_123', 123); + }); + assert.throws(function() { + res.setHeader('testing 123', 123); + }, TypeError); + res.end(''); +}); +server.listen(common.PORT, function() { + + http.get({port: common.PORT}, function() { + ee.emit('done'); + }); + + assert.throws( + function() { + var options = { + port: common.PORT, + headers: {'testing 123': 123} + }; + http.get(options, function() {}); + }, + function(err) { + ee.emit('done'); + if (err instanceof TypeError) return true; + } + ); + + assert.doesNotThrow( + function() { + var options = { + port: common.PORT, + headers: {'testing_123': 123} + }; + http.get(options, function() { + ee.emit('done'); + }); + }, TypeError + ); +}); + +ee.on('done', function() { + if (--count === 0) { + server.close(); + } +});