Repository navigation
http: http_parser_llhttp regression #28137
Description
Based on test-http-response-status-message.js.
This will succeed with --http-parser legacy but fail otherwise.
Related #27984 (comment), #28120
'use strict';
const common = require('../common');
const http = require('http');
const net = require('net');
const server = net.createServer(function(con) {
con.on('data', function(data) {
con.write('HTTP/1.1 200\r\n\r\n');
con.end();
});
});
server.listen(0, function() {
http.get({
port: server.address().port
}, function(res) {
res.on('aborted', common.mustNotCall());
res.on('end', common.mustCall());
res.resume();
});
});
Activity
mscdex commentedon Jun 8, 2019
I believe the newer parser is more correct. From RFC 7230:
So you can have an empty
reason-phrase, but you still need at least the space before it.ronag commentedon Jun 8, 2019
@mscdex so I guess the tests need to be updated?
ronag commentedon Jun 8, 2019
con.write('HTTP/1.1 200 OK\r\n\r\n');will also failmscdex commentedon Jun 8, 2019
An issue should be posted to https://github.com/nodejs/llhttp/issues then.
indutny commentedon Jun 9, 2019
This should fix it: https://github.com/nodejs/llhttp/pull/26/files . Sorry if it caused a disruption!
indutny commentedon Jun 10, 2019
The fix has been landed and released. Here is a PR to update llhttp: #28154