Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 2bcde83

Browse filesBrowse files
addaleaxBridgeAR
addaleax
authored and
BridgeAR
committed
http2: allow passing FileHandle to respondWithFD
This seems to make sense if we want to promote the use of `fs.promises`, although it’s not strictly necessary. PR-URL: #29876 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent adee998 commit 2bcde83
Copy full SHA for 2bcde83

File tree

Expand file treeCollapse file tree

6 files changed

+91
-32
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+91
-32
lines changed
Open diff view settings
Collapse file

‎doc/api/http2.md‎

Copy file name to clipboardExpand all lines: doc/api/http2.md
+6-3Lines changed: 6 additions & 3 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1439,13 +1439,16 @@ server.on('stream', (stream) => {
14391439
<!-- YAML
14401440
added: v8.4.0
14411441
changes:
1442+
- version: REPLACEME
1443+
pr-url: https://github.com/nodejs/node/pull/29876
1444+
description: The `fd` option may now be a `FileHandle`.
14421445
- version: v10.0.0
14431446
pr-url: https://github.com/nodejs/node/pull/18936
14441447
description: Any readable file descriptor, not necessarily for a
14451448
regular file, is supported now.
14461449
-->
14471450

1448-
* `fd` {number} A readable file descriptor.
1451+
* `fd` {number|FileHandle} A readable file descriptor.
14491452
* `headers` {HTTP/2 Headers Object}
14501453
* `options` {Object}
14511454
* `statCheck` {Function}
@@ -1491,8 +1494,8 @@ The `offset` and `length` options may be used to limit the response to a
14911494
specific range subset. This can be used, for instance, to support HTTP Range
14921495
requests.
14931496

1494-
The file descriptor is not closed when the stream is closed, so it will need
1495-
to be closed manually once it is no longer needed.
1497+
The file descriptor or `FileHandle` is not closed when the stream is closed,
1498+
so it will need to be closed manually once it is no longer needed.
14961499
Using the same file descriptor concurrently for multiple streams
14971500
is not supported and may result in data loss. Re-using a file descriptor
14981501
after a stream has finished is supported.
Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1948,7 +1948,7 @@ Object.defineProperties(fs, {
19481948
enumerable: true,
19491949
get() {
19501950
if (promises === null)
1951-
promises = require('internal/fs/promises');
1951+
promises = require('internal/fs/promises').exports;
19521952
return promises;
19531953
}
19541954
}
Collapse file

‎lib/internal/fs/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js
+30-26Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const {
4646
const pathModule = require('path');
4747
const { promisify } = require('internal/util');
4848

49-
const kHandle = Symbol('handle');
49+
const kHandle = Symbol('kHandle');
5050
const { kUsePromises } = binding;
5151

5252
const getDirectoryEntriesPromise = promisify(getDirents);
@@ -498,29 +498,33 @@ async function readFile(path, options) {
498498
}
499499

500500
module.exports = {
501-
access,
502-
copyFile,
503-
open,
504-
opendir: promisify(opendir),
505-
rename,
506-
truncate,
507-
rmdir,
508-
mkdir,
509-
readdir,
510-
readlink,
511-
symlink,
512-
lstat,
513-
stat,
514-
link,
515-
unlink,
516-
chmod,
517-
lchmod,
518-
lchown,
519-
chown,
520-
utimes,
521-
realpath,
522-
mkdtemp,
523-
writeFile,
524-
appendFile,
525-
readFile
501+
exports: {
502+
access,
503+
copyFile,
504+
open,
505+
opendir: promisify(opendir),
506+
rename,
507+
truncate,
508+
rmdir,
509+
mkdir,
510+
readdir,
511+
readlink,
512+
symlink,
513+
lstat,
514+
stat,
515+
link,
516+
unlink,
517+
chmod,
518+
lchmod,
519+
lchown,
520+
chown,
521+
utimes,
522+
realpath,
523+
mkdtemp,
524+
writeFile,
525+
appendFile,
526+
readFile,
527+
},
528+
529+
FileHandle
526530
};
Collapse file

‎lib/internal/http2/core.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/core.js
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ const {
8282
hideStackFrames
8383
} = require('internal/errors');
8484
const { validateNumber, validateString } = require('internal/validators');
85+
const fsPromisesInternal = require('internal/fs/promises');
8586
const { utcDate } = require('internal/http');
8687
const { onServerStream,
8788
Http2ServerRequest,
@@ -2545,7 +2546,10 @@ class ServerHttp2Stream extends Http2Stream {
25452546
this[kState].flags |= STREAM_FLAGS_HAS_TRAILERS;
25462547
}
25472548

2548-
validateNumber(fd, 'fd');
2549+
if (fd instanceof fsPromisesInternal.FileHandle)
2550+
fd = fd.fd;
2551+
else if (typeof fd !== 'number')
2552+
throw new ERR_INVALID_ARG_TYPE('fd', ['number', 'FileHandle'], fd);
25492553

25502554
debugStreamObj(this, 'initiating response from fd');
25512555
this[kUpdateTimer]();
Collapse file

‎test/parallel/test-http2-respond-file-fd-errors.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http2-respond-file-fd-errors.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ server.on('stream', common.mustCall((stream) => {
4242
{
4343
type: TypeError,
4444
code: 'ERR_INVALID_ARG_TYPE',
45-
message: 'The "fd" argument must be of type number. Received type ' +
45+
message: 'The "fd" argument must be one of type number or FileHandle.' +
46+
' Received type ' +
4647
typeof types[type]
4748
}
4849
);
Collapse file

‎test/parallel/test-http2-respond-file-filehandle.js‎

Copy file name to clipboard
+47Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fixtures = require('../common/fixtures');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
const http2 = require('http2');
8+
const assert = require('assert');
9+
const fs = require('fs');
10+
11+
const {
12+
HTTP2_HEADER_CONTENT_TYPE,
13+
HTTP2_HEADER_CONTENT_LENGTH
14+
} = http2.constants;
15+
16+
const fname = fixtures.path('elipses.txt');
17+
const data = fs.readFileSync(fname);
18+
const stat = fs.statSync(fname);
19+
fs.promises.open(fname, 'r').then(common.mustCall((fileHandle) => {
20+
const server = http2.createServer();
21+
server.on('stream', (stream) => {
22+
stream.respondWithFD(fileHandle, {
23+
[HTTP2_HEADER_CONTENT_TYPE]: 'text/plain',
24+
[HTTP2_HEADER_CONTENT_LENGTH]: stat.size,
25+
});
26+
});
27+
server.on('close', common.mustCall(() => fileHandle.close()));
28+
server.listen(0, common.mustCall(() => {
29+
30+
const client = http2.connect(`http://localhost:${server.address().port}`);
31+
const req = client.request();
32+
33+
req.on('response', common.mustCall((headers) => {
34+
assert.strictEqual(headers[HTTP2_HEADER_CONTENT_TYPE], 'text/plain');
35+
assert.strictEqual(+headers[HTTP2_HEADER_CONTENT_LENGTH], data.length);
36+
}));
37+
req.setEncoding('utf8');
38+
let check = '';
39+
req.on('data', (chunk) => check += chunk);
40+
req.on('end', common.mustCall(() => {
41+
assert.strictEqual(check, data.toString('utf8'));
42+
client.close();
43+
server.close();
44+
}));
45+
req.end();
46+
}));
47+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.