diff --git a/benchmark/http/incoming_headers.js b/benchmark/http/incoming_headers.js index f10178c1891805..810c92687bd981 100644 --- a/benchmark/http/incoming_headers.js +++ b/benchmark/http/incoming_headers.js @@ -3,12 +3,12 @@ const common = require('../common.js'); const http = require('http'); const bench = common.createBenchmark(main, { - // Unicode confuses ab on os x. - c: [50, 500], - n: [0, 5, 20] + c: [50], // Concurrent connections + n: [20], // Number of header lines to append after the common headers + w: [0, 6], // Amount of trailing whitespace }); -function main({ c, n }) { +function main({ c, n, w }) { const server = http.createServer((req, res) => { res.end(); }); @@ -22,7 +22,12 @@ function main({ c, n }) { 'Cache-Control': 'no-cache' }; for (let i = 0; i < n; i++) { - headers[`foo${i}`] = `some header value ${i}`; + // Note: + // - autocannon does not send header values with OWS + // - wrk can only send trailing OWS. This is a side-effect of wrk + // processing requests with http-parser before sending them, causing + // leading OWS to be stripped. + headers[`foo${i}`] = `some header value ${i}${' \t'.repeat(w / 2)}`; } bench.http({ path: '/', diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 5e1da912e0c31e..a8c48999c57901 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -77,6 +77,10 @@ const uint32_t kOnExecute = 4; // Any more fields than this will be flushed into JS const size_t kMaxHeaderFieldsCount = 32; +inline bool IsOWS(char c) { + return c == ' ' || c == '\t'; +} + // helper class for the Parser struct StringPtr { StringPtr() { @@ -136,13 +140,22 @@ struct StringPtr { Local ToString(Environment* env) const { - if (str_) + if (size_ != 0) return OneByteString(env->isolate(), str_, size_); else return String::Empty(env->isolate()); } + // Strip trailing OWS (SPC or HTAB) from string. + Local ToTrimmedString(Environment* env) { + while (size_ > 0 && IsOWS(str_[size_ - 1])) { + size_--; + } + return ToString(env); + } + + const char* str_; bool on_heap_; size_t size_; @@ -730,7 +743,7 @@ class Parser : public AsyncWrap, public StreamListener { for (size_t i = 0; i < num_values_; ++i) { headers_v[i * 2] = fields_[i].ToString(env()); - headers_v[i * 2 + 1] = values_[i].ToString(env()); + headers_v[i * 2 + 1] = values_[i].ToTrimmedString(env()); } return Array::New(env()->isolate(), headers_v, num_values_ * 2); diff --git a/test/parallel/test-http-header-owstext.js b/test/parallel/test-http-header-owstext.js new file mode 100644 index 00000000000000..bc094137a29e5e --- /dev/null +++ b/test/parallel/test-http-header-owstext.js @@ -0,0 +1,49 @@ +'use strict'; +const common = require('../common'); + +// This test ensures that the http-parser strips leading and trailing OWS from +// header values. It sends the header values in chunks to force the parser to +// build the string up through multiple calls to on_header_value(). + +const assert = require('assert'); +const http = require('http'); +const net = require('net'); + +function check(hdr, snd, rcv) { + const server = http.createServer(common.mustCall((req, res) => { + assert.strictEqual(req.headers[hdr], rcv); + req.pipe(res); + })); + + server.listen(0, common.mustCall(function() { + const client = net.connect(this.address().port, start); + function start() { + client.write('GET / HTTP/1.1\r\n' + hdr + ':', drain); + } + + function drain() { + if (snd.length === 0) { + return client.write('\r\nConnection: close\r\n\r\n'); + } + client.write(snd.shift(), drain); + } + + const bufs = []; + client.on('data', function(chunk) { + bufs.push(chunk); + }); + client.on('end', common.mustCall(function() { + const head = Buffer.concat(bufs) + .toString('latin1') + .split('\r\n')[0]; + assert.strictEqual(head, 'HTTP/1.1 200 OK'); + server.close(); + })); + })); +} + +check('host', [' \t foo.com\t'], 'foo.com'); +check('host', [' \t foo\tcom\t'], 'foo\tcom'); +check('host', [' \t', ' ', ' foo.com\t', '\t '], 'foo.com'); +check('host', [' \t', ' \t'.repeat(100), '\t '], ''); +check('host', [' \t', ' - - - - ', '\t '], '- - - -');