Skip to content
This repository was archived by the owner on Jul 6, 2018. It is now read-only.

http2: request.socket|connection #130

Closed
wants to merge 4 commits into from
Closed

http2: request.socket|connection #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
674d237
http2: request.socket|connection
sebdeckers May 17, 2017
360c27a
http2: Validate the returned value type (Squash)
sebdeckers May 17, 2017
9f3e375
http2: Docs warning against direct socket access (Squash)
sebdeckers May 17, 2017
dc1f350
http2: Prefer the language in #132 (Squash)
sebdeckers May 18, 2017
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: 8 additions & 0 deletions lib/internal/http2/compat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,14 @@ class Http2ServerRequest extends Readable {
return '2.0';
}

get socket() {
return this.stream.session.socket;
}

get connection() {
return this.socket;
Copy link
Member

@jasnell jasnell May 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One point on this: we really need to discourage direct access to the socket. With an http/2 connection, bad things can happen if the user does not know what they are doing. The underlying native Http2Session class takes complete ownership of reading and writing to the socket and any direct interactions with the socket by user code could throw the entire connection out of sync. I'd prefer not to expose these at all but if we need to, let's add a strong warning in the docs.

Copy link
Contributor Author

@sebdeckers sebdeckers May 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, will rebase and update the docs that were just landed.

}

_read(nread) {
var stream = this[kStream];
if (stream) {
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-http2-compat-serverrequest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const common = require('../common');
const assert = require('assert');
const h2 = require('http2');
const net = require('net');

// Http2ServerRequest should expose convenience properties

Expand All @@ -22,6 +23,10 @@ server.listen(0, common.mustCall(function() {
assert.strictEqual(request.httpVersionMajor, expected.httpVersionMajor);
assert.strictEqual(request.httpVersionMinor, expected.httpVersionMinor);

assert.ok(request.socket instanceof net.Socket);
assert.ok(request.connection instanceof net.Socket);
assert.strictEqual(request.socket, request.connection);

response.stream.on('finish', common.mustCall(function() {
server.close();
}));
Expand Down