add proxy handler "has" for "http2.Http2ServerResponse.socket" and "http2.Http2ServerRequest.socket" by masx200 · Pull Request #35197 · nodejs/node

masx200 · 2020-09-14T22:43:26Z

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

add proxy handler "has" for "http2.Http2ServerResponse.socket" and "http2.Http2ServerRequest.socket".

Usually check whether the connection is a secure connection using the following judgment method:

const request=new http2.Http2ServerRequest(stream, headers, options, rawHeaders)
//true
assert(true===request.socket.encrypted)

//false
assert("encrypted" in request.socket)

nodejs-github-bot · 2020-09-14T22:43:30Z

Review requested:

@nodejs/http2
@nodejs/net

mcollina

Can you please add a unit test?

masx200 · 2020-09-14T23:00:32Z

Ok, testing is in progress

masx200 · 2020-09-15T00:25:44Z

@codebytere @Trott @schamberg97 @addaleax

Request review

mcollina

lgtm

rickyes · 2020-09-17T14:05:03Z

Can you fix the lint-js errors? it will make the review easier.

Trott · 2020-09-18T16:36:17Z

Can you fix the lint-js errors? it will make the review easier.

I've fixed all the lint errors in a pair of fixup commits. @rickyes PTAL

file was renamed

LGTM but I'm going to defer to others on whether this is expected/idiomatic

Trott

Of course this is expected/idiomatic.

Trott · 2020-09-19T15:18:47Z

lib/internal/http2/compat.js

 const proxySocketHandler = {
+  has(stream, prop) {
+    const ref = stream.session !== undefined ? stream.session[kSocket] : stream;
+    return (prop in stream) || (prop in ref);


Optional suggestion: The test file only covers one of these conditions, right? Does it make sense to add a test case so that both are covered?

nodejs-github-bot · 2020-09-19T15:25:10Z

CI: https://ci.nodejs.org/job/node-test-pull-request/33167/

PR-URL: #35197 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

aduh95 · 2020-11-09T18:04:28Z

Landed in 0d2e198

PR-URL: #35197 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Update compat.js

dd8afa3

masx200 requested review from a team as code owners September 14, 2020 22:43

nodejs-github-bot added the http2 Issues or PRs related to the http2 subsystem. label Sep 14, 2020

masx200 changed the title ~~add proxy handler "has" for http2.Http2ServerResponse.socket~~ add proxy handler "has" for "http2.Http2ServerResponse.socket" and "http2.Http2ServerRequest.socket" Sep 14, 2020

mcollina requested changes Sep 14, 2020

View reviewed changes

masx200 added 5 commits September 15, 2020 07:01

Create test-for-pull-35197.js

fad3ea4

Update compat.js

4ef108c

Update test-for-pull-35197.js

33d14d2

Update test-for-pull-35197.js

f8db9cd

Update test-for-pull-35197.js

40e47a7

masx200 added 5 commits September 15, 2020 09:44

Update compat.js

4def244

Update test-for-pull-35197.js

4f0b74a

Update test-for-pull-35197.js

832d616

Update test-for-pull-35197.js

68268bf

Update compat.js

e634e59

masx200 requested review from mcollina and removed request for a team September 15, 2020 05:32

masx200 added 2 commits September 15, 2020 13:35

Update test-for-pull-35197.js

ccfccf1

Update test-for-pull-35197.js

6a67797

mcollina approved these changes Sep 15, 2020

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 15, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 15, 2020

Trott added 2 commits September 18, 2020 09:30

fixup! Update test-for-pull-35197.js

362a628

fixup! fixup! Update test-for-pull-35197.js

734fa63

masx200 requested a review from Trott September 18, 2020 16:45

Trott previously approved these changes Sep 19, 2020

View reviewed changes

Trott approved these changes Sep 19, 2020

View reviewed changes

Trott reviewed Sep 19, 2020

View reviewed changes

Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 19, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 19, 2020

aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 8, 2020

aduh95 pushed a commit that referenced this pull request Nov 9, 2020

http2: add has method to proxySocketHandler

0d2e198

PR-URL: #35197 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

aduh95 closed this Nov 9, 2020

danielleadams pushed a commit that referenced this pull request Nov 9, 2020

http2: add has method to proxySocketHandler

11f5924

PR-URL: #35197 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

danielleadams mentioned this pull request Nov 9, 2020

v15.2.0 proposal #36055

Merged

BethGriggs pushed a commit that referenced this pull request Dec 9, 2020

http2: add has method to proxySocketHandler

163dc59

PR-URL: #35197 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

BethGriggs pushed a commit that referenced this pull request Dec 10, 2020

http2: add has method to proxySocketHandler

4acb84a

PR-URL: #35197 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

BethGriggs mentioned this pull request Dec 10, 2020

v14.15.2 proposal #36476

Merged

BethGriggs pushed a commit that referenced this pull request Dec 15, 2020

http2: add has method to proxySocketHandler

c4e17cf

PR-URL: #35197 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

wedataintelligence mentioned this pull request Nov 30, 2023

[Snyk] Fix for 1 vulnerabilities wedataintelligence/node#90

Open

aliceUnhinged613 mentioned this pull request Nov 30, 2023

[Snyk] Fix for 1 vulnerabilities aliceUnhinged613/node#108

Open

baby636 mentioned this pull request Dec 1, 2023

[Snyk] Fix for 1 vulnerabilities baby636/node#114

Open

aliceUnhinged613 mentioned this pull request May 13, 2024

[Snyk] Fix for 1 vulnerabilities aliceUnhinged613/node#127

Open

molllyn1 mentioned this pull request May 13, 2024

[Snyk] Fix for 2 vulnerabilities molllyn1/node#111

Open

ron-a20 mentioned this pull request May 13, 2024

[Snyk] Fix for 2 vulnerabilities ron-a20/node#126

Open

137717unity mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities 137717unity/node#128

Open

baby636 mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities baby636/node#149

Open

wedataintelligence mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities wedataintelligence/node#111

Open

aliceUnhinged613 mentioned this pull request May 15, 2024

[Snyk] Fix for 1 vulnerabilities aliceUnhinged613/node#129

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

add proxy handler "has" for "http2.Http2ServerResponse.socket" and "http2.Http2ServerRequest.socket"#35197

add proxy handler "has" for "http2.Http2ServerResponse.socket" and "http2.Http2ServerRequest.socket"#35197
masx200 wants to merge 17 commits intonodejs:masterfrom
masx200:patch-3

masx200 commented Sep 14, 2020 •

edited

Loading

Uh oh!

nodejs-github-bot commented Sep 14, 2020

Uh oh!

mcollina left a comment

Uh oh!

masx200 commented Sep 14, 2020 •

edited

Loading

Uh oh!

masx200 commented Sep 15, 2020

Uh oh!

mcollina left a comment

Uh oh!

rickyes commented Sep 17, 2020

Uh oh!

Trott commented Sep 18, 2020

Uh oh!

Trott left a comment

Uh oh!

Trott Sep 19, 2020

Uh oh!

nodejs-github-bot commented Sep 19, 2020

Uh oh!

aduh95 commented Nov 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

masx200 commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

nodejs-github-bot commented Sep 14, 2020

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

masx200 commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

masx200 commented Sep 15, 2020

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

rickyes commented Sep 17, 2020

Uh oh!

Trott commented Sep 18, 2020

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

Trott Sep 19, 2020

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Sep 19, 2020

Uh oh!

aduh95 commented Nov 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

masx200 commented Sep 14, 2020 •

edited

Loading

masx200 commented Sep 14, 2020 •

edited

Loading