This repository was archived by the owner on Jul 6, 2018. It is now read-only.
Repository navigation
This repository was archived by the owner on Jul 6, 2018. It is now read-only.
http2: status code is string instead of number #118
Description
The status code is a string now, not of type number any more.
Apart from the compat change, from a user perspective its a bit unhandy to get back the statuscode as a string, as you have to cast it before you make something like:
if (statusCode >= 200) { // todo ... }Example for failing, ported http1 test:
'use strict';
const common = require('../common');
const assert = require('assert');
const http2 = require('http2');
const server = http2.createServer(function(request, response) {
console.log('responding to ' + request.url);
response.writeHead(200, {'Content-Type': 'text/plain'});
response.write('1\n');
response.write('');
response.write('2\n');
response.write('');
response.end('3\n');
this.close();
});
server.listen(0, common.mustCall(function() {
const client = http2.connect(`http://localhost:${this.address().port}`);
const headers = { ':path': '/' };
const req = client.request(headers).setEncoding('ascii');
let response = '';
req.on('response', (headers) => {
assert.strictEqual(200, headers[':status']);
});
req.on('data', function(chunk) {
response += chunk;
});
req.on('end', common.mustCall(function(a,b,c) {
assert.strictEqual('1\n2\n3\n', response);
}));
req.end();
}));fails with:
assert.js:86
throw new assert.AssertionError({
^
AssertionError: 200 === '200'
at ClientHttp2Stream.req.on (/Users/robert/projects/http2/test/parallel/test-http2-write-empty-string.js:30:12)
at emitTwo (events.js:125:13)
at ClientHttp2Stream.emit (events.js:213:7)
at Http2Session.onSessionHeaders [as onheaders] (internal/http2/core.js:194:12)
Activity
jasnell commentedon May 15, 2017
Yep... this would be a fairly easy fix if you'd like to give it a shot. There are three ways to approach it:
In the native later, if the header field name is
:key, coerce the value to a number. This likely would be the much more difficult way.In core.js, in the
onSessionHeadersfunction, check for the existence of the:statusheader and coerce it's value to a number (e.g.headers[HTTP2_HEADER_STATUS] |= 0)Coerce the value to a number in compat.js
Option 2 is likely the best choice.
robertkowalski commentedon May 15, 2017
Cool, I went for 2) and opened #120 -- I ran into an issue with closing the connection and its getting late here, maybe a bug?
sebdeckers commentedon May 21, 2017
Can be closed