diff --git a/doc/api/errors.md b/doc/api/errors.md index 7d6e238f93bdaa..f5190dc52bffa9 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -971,7 +971,14 @@ client. ### ERR_HTTP2_SEND_FILE An attempt was made to use the `Http2Stream.prototype.responseWithFile()` API to -send something other than a regular file. +send a directory. + + +### ERR_HTTP2_SEND_FILE_NOSEEK + +An attempt was made to use the `Http2Stream.prototype.responseWithFile()` API to +send something other than a regular file, but `offset` or `length` options were +provided. ### ERR_HTTP2_SESSION_ERROR diff --git a/doc/api/http2.md b/doc/api/http2.md index 94f25377d0acbf..13aa4d1e2103d1 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1223,6 +1223,11 @@ if the `getTrailers` callback attempts to set such header fields. #### http2stream.respondWithFD(fd[, headers[, options]]) * `fd` {number} A readable file descriptor. @@ -1313,6 +1318,11 @@ if the `getTrailers` callback attempts to set such header fields. #### http2stream.respondWithFile(path[, headers[, options]]) * `path` {string|Buffer|URL} diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 8ec72dd647b1b2..3d0b6cf9b39177 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -714,7 +714,9 @@ E('ERR_HTTP2_PING_LENGTH', 'HTTP2 ping payload must be 8 bytes', RangeError); E('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED', 'Cannot set HTTP/2 pseudo-headers', Error); E('ERR_HTTP2_PUSH_DISABLED', 'HTTP/2 client has disabled push streams', Error); -E('ERR_HTTP2_SEND_FILE', 'Only regular files can be sent', Error); +E('ERR_HTTP2_SEND_FILE', 'Directories cannot be sent', Error); +E('ERR_HTTP2_SEND_FILE_NOSEEK', + 'Offset or length can only be specified for regular files', Error); E('ERR_HTTP2_SESSION_ERROR', 'Session closed with error code %s', Error); E('ERR_HTTP2_SOCKET_BOUND', 'The socket is already bound to an Http2Session', Error); diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 607ae3fd2d297d..7137e527df07aa 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -42,6 +42,7 @@ const { ERR_HTTP2_PING_LENGTH, ERR_HTTP2_PUSH_DISABLED, ERR_HTTP2_SEND_FILE, + ERR_HTTP2_SEND_FILE_NOSEEK, ERR_HTTP2_SESSION_ERROR, ERR_HTTP2_SETTINGS_CANCEL, ERR_HTTP2_SOCKET_BOUND, @@ -2045,12 +2046,21 @@ function doSendFileFD(session, options, fd, headers, streamOptions, err, stat) { } if (!stat.isFile()) { - const err = new ERR_HTTP2_SEND_FILE(); - if (onError) - onError(err); - else - this.destroy(err); - return; + const isDirectory = stat.isDirectory(); + if (options.offset !== undefined || options.offset > 0 || + options.length !== undefined || options.length >= 0 || + isDirectory) { + const err = isDirectory ? + new ERR_HTTP2_SEND_FILE() : new ERR_HTTP2_SEND_FILE_NOSEEK(); + if (onError) + onError(err); + else + this.destroy(err); + return; + } + + options.offset = -1; + options.length = -1; } if (this.destroyed || this.closed) { @@ -2075,12 +2085,14 @@ function doSendFileFD(session, options, fd, headers, streamOptions, err, stat) { return; } - statOptions.length = - statOptions.length < 0 ? stat.size - (+statOptions.offset) : - Math.min(stat.size - (+statOptions.offset), - statOptions.length); + if (stat.isFile()) { + statOptions.length = + statOptions.length < 0 ? stat.size - (+statOptions.offset) : + Math.min(stat.size - (+statOptions.offset), + statOptions.length); - headers[HTTP2_HEADER_CONTENT_LENGTH] = statOptions.length; + headers[HTTP2_HEADER_CONTENT_LENGTH] = statOptions.length; + } processRespondWithFD(this, fd, headers, options.offset | 0, diff --git a/test/parallel/test-http2-respond-file-error-dir.js b/test/parallel/test-http2-respond-file-error-dir.js index 6818616227df89..24a6d2dc96597e 100644 --- a/test/parallel/test-http2-respond-file-error-dir.js +++ b/test/parallel/test-http2-respond-file-error-dir.js @@ -15,7 +15,7 @@ server.on('stream', (stream) => { common.expectsError({ code: 'ERR_HTTP2_SEND_FILE', type: Error, - message: 'Only regular files can be sent' + message: 'Directories cannot be sent' })(err); stream.respond({ ':status': 404 }); diff --git a/test/parallel/test-http2-respond-file-error-pipe-offset.js b/test/parallel/test-http2-respond-file-error-pipe-offset.js new file mode 100644 index 00000000000000..eb782e2dea66c4 --- /dev/null +++ b/test/parallel/test-http2-respond-file-error-pipe-offset.js @@ -0,0 +1,61 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +if (common.isWindows) + common.skip('no mkfifo on Windows'); +const child_process = require('child_process'); +const path = require('path'); +const fs = require('fs'); +const http2 = require('http2'); +const assert = require('assert'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const pipeName = path.join(tmpdir.path, 'pipe'); + +const mkfifo = child_process.spawnSync('mkfifo', [ pipeName ]); +if (mkfifo.error && mkfifo.error.code === 'ENOENT') { + common.skip('missing mkfifo'); +} + +process.on('exit', () => fs.unlinkSync(pipeName)); + +const server = http2.createServer(); +server.on('stream', (stream) => { + stream.respondWithFile(pipeName, { + 'content-type': 'text/plain' + }, { + offset: 10, + onError(err) { + common.expectsError({ + code: 'ERR_HTTP2_SEND_FILE_NOSEEK', + type: Error, + message: 'Offset or length can only be specified for regular files' + })(err); + + stream.respond({ ':status': 404 }); + stream.end(); + }, + statCheck: common.mustNotCall() + }); +}); +server.listen(0, () => { + + const client = http2.connect(`http://localhost:${server.address().port}`); + const req = client.request(); + + req.on('response', common.mustCall((headers) => { + assert.strictEqual(headers[':status'], 404); + })); + req.on('data', common.mustNotCall()); + req.on('end', common.mustCall(() => { + client.close(); + server.close(); + })); + req.end(); +}); + +fs.writeFile(pipeName, 'Hello, world!\n', common.mustCall()); diff --git a/test/parallel/test-http2-respond-file-with-pipe.js b/test/parallel/test-http2-respond-file-with-pipe.js new file mode 100644 index 00000000000000..2b7ca3fc9a51ee --- /dev/null +++ b/test/parallel/test-http2-respond-file-with-pipe.js @@ -0,0 +1,58 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +if (common.isWindows) + common.skip('no mkfifo on Windows'); +const child_process = require('child_process'); +const path = require('path'); +const fs = require('fs'); +const http2 = require('http2'); +const assert = require('assert'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +const pipeName = path.join(tmpdir.path, 'pipe'); + +const mkfifo = child_process.spawnSync('mkfifo', [ pipeName ]); +if (mkfifo.error && mkfifo.error.code === 'ENOENT') { + common.skip('missing mkfifo'); +} + +process.on('exit', () => fs.unlinkSync(pipeName)); + +const server = http2.createServer(); +server.on('stream', (stream) => { + stream.respondWithFile(pipeName, { + 'content-type': 'text/plain' + }, { + onError: common.mustNotCall(), + statCheck: common.mustCall() + }); +}); + +server.listen(0, () => { + const client = http2.connect(`http://localhost:${server.address().port}`); + const req = client.request(); + + req.on('response', common.mustCall((headers) => { + assert.strictEqual(headers[':status'], 200); + })); + let body = ''; + req.setEncoding('utf8'); + req.on('data', (chunk) => body += chunk); + req.on('end', common.mustCall(() => { + assert.strictEqual(body, 'Hello, world!\n'); + client.close(); + server.close(); + })); + req.end(); +}); + +fs.open(pipeName, 'w', common.mustCall((err, fd) => { + assert.ifError(err); + fs.writeSync(fd, 'Hello, world!\n'); + fs.closeSync(fd); +}));