Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http: add strictMode option and checkIsHttpToken check #2526

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
});