Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6192c98

Browse filesBrowse files
jasnelljasnell
committed
http: add checkIsHttpToken check for header fields
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
1 parent b50e89e commit 6192c98
Copy full SHA for 6192c98

File tree

Expand file treeCollapse file tree

5 files changed

+85
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+85
-1
lines changed
Open diff view settings
Collapse file

‎doc/api/http.markdown‎

Copy file name to clipboardExpand all lines: doc/api/http.markdown
+5Lines changed: 5 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,9 @@ or
364364

365365
response.setHeader("Set-Cookie", ["type=ninja", "language=javascript"]);
366366

367+
Attempting to set a header field name that contains invalid characters will
368+
result in a `TypeError` being thrown.
369+
367370
### response.headersSent
368371

369372
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.,
439442
response.addTrailers({'Content-MD5': "7895bf4b8828b55ceaf47747b4bca667"});
440443
response.end();
441444

445+
Attempting to set a trailer field name that contains invalid characters will
446+
result in a `TypeError` being thrown.
442447

443448
### response.end([data][, encoding][, callback])
444449

Collapse file

‎lib/_http_client.js‎

Copy file name to clipboardExpand all lines: lib/_http_client.js
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ function ClientRequest(options, cb) {
6767
self.socketPath = options.socketPath;
6868

6969
var method = self.method = (options.method || 'GET').toUpperCase();
70+
if (!common._checkIsHttpToken(method)) {
71+
throw new TypeError('Method must be a valid HTTP token');
72+
}
7073
self.path = options.path || '/';
7174
if (cb) {
7275
self.once('response', cb);
Collapse file

‎lib/_http_common.js‎

Copy file name to clipboardExpand all lines: lib/_http_common.js
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,13 @@ function httpSocketSetup(socket) {
195195
socket.on('drain', ondrain);
196196
}
197197
exports.httpSocketSetup = httpSocketSetup;
198+
199+
/**
200+
* Verifies that the given val is a valid HTTP token
201+
* per the rules defined in RFC 7230
202+
**/
203+
const token = /^[a-zA-Z0-9_!#$%&'*+.^`|~-]+$/;
204+
function checkIsHttpToken(val) {
205+
return typeof val === 'string' && token.test(val);
206+
}
207+
exports._checkIsHttpToken = checkIsHttpToken;
Collapse file

‎lib/_http_outgoing.js‎

Copy file name to clipboardExpand all lines: lib/_http_outgoing.js
+11-1Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,10 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
295295
};
296296

297297
function storeHeader(self, state, field, value) {
298+
if (!common._checkIsHttpToken(field)) {
299+
throw new TypeError(
300+
'Header name must be a valid HTTP Token ["' + field + '"]');
301+
}
298302
value = escapeHeaderValue(value);
299303
state.messageHeader += field + ': ' + value + CRLF;
300304

@@ -323,6 +327,9 @@ function storeHeader(self, state, field, value) {
323327

324328

325329
OutgoingMessage.prototype.setHeader = function(name, value) {
330+
if (!common._checkIsHttpToken(name))
331+
throw new TypeError(
332+
'Header name must be a valid HTTP Token ["' + name + '"]');
326333
if (typeof name !== 'string')
327334
throw new TypeError('`name` should be a string in setHeader(name, value).');
328335
if (value === undefined)
@@ -498,7 +505,10 @@ OutgoingMessage.prototype.addTrailers = function(headers) {
498505
field = key;
499506
value = headers[key];
500507
}
501-
508+
if (!common._checkIsHttpToken(field)) {
509+
throw new TypeError(
510+
'Trailer name must be a valid HTTP Token ["' + field + '"]');
511+
}
502512
this._trailer += field + ': ' + escapeHeaderValue(value) + CRLF;
503513
}
504514
};
Collapse file

‎test/parallel/test-http-invalidheaderfield.js‎

Copy file name to clipboard
+56Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const EventEmitter = require('events');
5+
const http = require('http');
6+
7+
const ee = new EventEmitter();
8+
var count = 3;
9+
10+
const server = http.createServer(function(req, res) {
11+
assert.doesNotThrow(function() {
12+
res.setHeader('testing_123', 123);
13+
});
14+
assert.throws(function() {
15+
res.setHeader('testing 123', 123);
16+
}, TypeError);
17+
res.end('');
18+
});
19+
server.listen(common.PORT, function() {
20+
21+
http.get({port: common.PORT}, function() {
22+
ee.emit('done');
23+
});
24+
25+
assert.throws(
26+
function() {
27+
var options = {
28+
port: common.PORT,
29+
headers: {'testing 123': 123}
30+
};
31+
http.get(options, function() {});
32+
},
33+
function(err) {
34+
ee.emit('done');
35+
if (err instanceof TypeError) return true;
36+
}
37+
);
38+
39+
assert.doesNotThrow(
40+
function() {
41+
var options = {
42+
port: common.PORT,
43+
headers: {'testing_123': 123}
44+
};
45+
http.get(options, function() {
46+
ee.emit('done');
47+
});
48+
}, TypeError
49+
);
50+
});
51+
52+
ee.on('done', function() {
53+
if (--count === 0) {
54+
server.close();
55+
}
56+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.