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.

Sign up for GitHub

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

Jump to bottom

http2: fix refs to status 205, add tests #15153

Closed
wants to merge 1 commit into from
Closed

http2: fix refs to status 205, add tests #15153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions lib/internal/http2/core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ const {
HTTP2_METHOD_CONNECT,

HTTP_STATUS_CONTINUE,
HTTP_STATUS_CONTENT_RESET,
HTTP_STATUS_RESET_CONTENT,
HTTP_STATUS_OK,
HTTP_STATUS_NO_CONTENT,
HTTP_STATUS_NOT_MODIFIED,
Expand Down Expand Up @@ -1871,7 +1871,7 @@ class ServerHttp2Stream extends Http2Stream {
// the options.endStream option to true so that the underlying
// bits do not attempt to send any.
if (statusCode === HTTP_STATUS_NO_CONTENT ||
statusCode === HTTP_STATUS_CONTENT_RESET ||
statusCode === HTTP_STATUS_RESET_CONTENT ||
statusCode === HTTP_STATUS_NOT_MODIFIED ||
state.headRequest === true) {
options.endStream = true;
Expand Down Expand Up @@ -1965,7 +1965,7 @@ class ServerHttp2Stream extends Http2Stream {
const statusCode = headers[HTTP2_HEADER_STATUS] |= 0;
// Payload/DATA frames are not permitted in these cases
if (statusCode === HTTP_STATUS_NO_CONTENT ||
statusCode === HTTP_STATUS_CONTENT_RESET ||
statusCode === HTTP_STATUS_RESET_CONTENT ||
statusCode === HTTP_STATUS_NOT_MODIFIED) {
throw new errors.Error('ERR_HTTP2_PAYLOAD_FORBIDDEN', statusCode);
}
Expand Down Expand Up @@ -2042,7 +2042,7 @@ class ServerHttp2Stream extends Http2Stream {
const statusCode = headers[HTTP2_HEADER_STATUS] |= 0;
// Payload/DATA frames are not permitted in these cases
if (statusCode === HTTP_STATUS_NO_CONTENT ||
statusCode === HTTP_STATUS_CONTENT_RESET ||
statusCode === HTTP_STATUS_RESET_CONTENT ||
statusCode === HTTP_STATUS_NOT_MODIFIED) {
throw new errors.Error('ERR_HTTP2_PAYLOAD_FORBIDDEN', statusCode);
}
Expand Down
103 changes: 103 additions & 0 deletions test/parallel/test-http2-respond-file-errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const http2 = require('http2');
const path = require('path');

const optionsWithTypeError = {
offset: 'number',
length: 'number',
statCheck: 'function',
getTrailers: 'function'
};

const types = {
boolean: true,
function: () => {},
number: 1,
object: {},
array: [],
null: null,
symbol: Symbol('test')
};

const fname = path.resolve(common.fixturesDir, 'elipses.txt');

const server = http2.createServer();

server.on('stream', common.mustCall((stream) => {
// Check for all possible TypeError triggers on options
Object.keys(optionsWithTypeError).forEach((option) => {
Object.keys(types).forEach((type) => {
if (type === optionsWithTypeError[option]) {
return;
}

common.expectsError(
() => stream.respondWithFile(fname, {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}, {
[option]: types[type]
}),
{
type: TypeError,
code: 'ERR_INVALID_OPT_VALUE',
message: `The value "${String(types[type])}" is invalid ` +
`for option "${option}"`
}
);
});
});

// Should throw if :status 204, 205 or 304
[204, 205, 304].forEach((status) => common.expectsError(
() => stream.respondWithFile(fname, {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain',
':status': status,
}),
{
code: 'ERR_HTTP2_PAYLOAD_FORBIDDEN',
message: `Responses with ${status} status must not have a payload`
}
));

// Should throw if headers already sent
stream.respond({
':status': 200,
});
common.expectsError(
() => stream.respondWithFile(fname, {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}),
{
code: 'ERR_HTTP2_HEADERS_SENT',
message: 'Response has already been initiated.'
}
);

// Should throw if stream already destroyed
stream.destroy();
common.expectsError(
() => stream.respondWithFile(fname, {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}),
{
code: 'ERR_HTTP2_INVALID_STREAM',
message: 'The stream has been destroyed'
}
);
}));

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();

req.on('streamClosed', common.mustCall(() => {
client.destroy();
server.close();
}));
req.end();
}));
123 changes: 123 additions & 0 deletions test/parallel/test-http2-respond-file-fd-errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const http2 = require('http2');
const path = require('path');
const fs = require('fs');

const optionsWithTypeError = {
offset: 'number',
length: 'number',
statCheck: 'function',
getTrailers: 'function'
};

const types = {
boolean: true,
function: () => {},
number: 1,
object: {},
array: [],
null: null,
symbol: Symbol('test')
};

const fname = path.resolve(common.fixturesDir, 'elipses.txt');
const fd = fs.openSync(fname, 'r');

const server = http2.createServer();

server.on('stream', common.mustCall((stream) => {
// should throw if fd isn't a number
Object.keys(types).forEach((type) => {
if (type === 'number') {
return;
}

common.expectsError(
() => stream.respondWithFD(types[type], {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}),
{
type: TypeError,
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "fd" argument must be of type number'
}
);
});

// Check for all possible TypeError triggers on options
Object.keys(optionsWithTypeError).forEach((option) => {
Object.keys(types).forEach((type) => {
if (type === optionsWithTypeError[option]) {
return;
}

common.expectsError(
() => stream.respondWithFD(fd, {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}, {
[option]: types[type]
}),
{
type: TypeError,
code: 'ERR_INVALID_OPT_VALUE',
message: `The value "${String(types[type])}" is invalid ` +
`for option "${option}"`
}
);
});
});

// Should throw if :status 204, 205 or 304
[204, 205, 304].forEach((status) => common.expectsError(
() => stream.respondWithFD(fd, {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain',
':status': status,
}),
{
code: 'ERR_HTTP2_PAYLOAD_FORBIDDEN',
message: `Responses with ${status} status must not have a payload`
}
));

// Should throw if headers already sent
stream.respond({
':status': 200,
});
common.expectsError(
() => stream.respondWithFD(fd, {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}),
{
code: 'ERR_HTTP2_HEADERS_SENT',
message: 'Response has already been initiated.'
}
);

// Should throw if stream already destroyed
stream.destroy();
common.expectsError(
() => stream.respondWithFD(fd, {
[http2.constants.HTTP2_HEADER_CONTENT_TYPE]: 'text/plain'
}),
{
code: 'ERR_HTTP2_INVALID_STREAM',
message: 'The stream has been destroyed'
}
);
}));

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();

req.on('streamClosed', common.mustCall(() => {
client.destroy();
server.close();
}));
req.end();
}));
40 changes: 40 additions & 0 deletions test/parallel/test-http2-respond-no-data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Flags: --expose-http2
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const http2 = require('http2');
const assert = require('assert');

const server = http2.createServer();

// Check that stream ends immediately after respond on :status 204, 205 & 304

const status = [204, 205, 304];

server.on('stream', common.mustCall((stream) => {
stream.on('streamClosed', common.mustCall(() => {
assert.strictEqual(stream.destroyed, true);
}));
stream.respond({ ':status': status.shift() });
}, 3));

server.listen(0, common.mustCall(makeRequest));

function makeRequest() {
const client = http2.connect(`http://localhost:${server.address().port}`);
const req = client.request();
req.resume();

req.on('end', common.mustCall(() => {
client.destroy();

if (!status.length) {
server.close();
} else {
makeRequest();
}
}));
req.end();
}