Skip to content

Commit

Permalink
http: add checkIsHttpToken check for header fields
Browse files Browse the repository at this point in the history
Ref: nodejs/node-convergence-archive#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 <thechargingvolcano@gmail.com>
Reviewed-By: Trevor Norris <trevnorris@nodejs.org>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: #2526
  • Loading branch information
jasnell committed Sep 25, 2015
1 parent b50e89e commit 6192c98
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 1 deletion.
5 changes: 5 additions & 0 deletions doc/api/http.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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])

Expand Down
3 changes: 3 additions & 0 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
12 changes: 11 additions & 1 deletion lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
};
Expand Down
56 changes: 56 additions & 0 deletions test/parallel/test-http-invalidheaderfield.js
Original file line number Diff line number Diff line change
@@ -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();
}
});

0 comments on commit 6192c98

Please sign in to comment.