FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: fix prefixed headers order by ofirbarak · Pull Request #41735 · nodejs/node · GitHub

/ node Public

http2: fix prefixed headers order - #41735

Closed
ofirbarak wants to merge 4 commits into
nodejs:masterfrom
ofirbarak:http-prfixed-headers-order
ConversationCommits (4)ChecksFiles changed
Closed

http2: fix prefixed headers order#41735
ofirbarak wants to merge 4 commits into
nodejs:masterfrom
ofirbarak:http-prfixed-headers-order

Conversation

ofirbarak commented Jan 28, 2022

Copy link
Copy Markdown
Contributor

Keep prefixed headers order same as sent order

Fixes: #38797

nodejs-github-bot commented Jan 28, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

nodejs-github-bot added http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Jan 28, 2022
ofirbarak force-pushed the http-prfixed-headers-order branch from cce4dce to 32f9aaf Compare January 28, 2022 21:36
mcollina reviewed Jan 28, 2022
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you please add a unit test?

Trott previously requested changes Jan 29, 2022
View reviewed changes

Trott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Welcome @ofirbarak and thanks for the pull request. Are you able to rebase to edit the first commit message so that it starts with http2: rather than lib:, and then force push to your branch?

If this change doesn't happen and some collaborator wants to land this anyway, that's fine, clear this objection, but please make sure to land manually so you can edit the commit message yourself. Thanks!

ofirbarak force-pushed the http-prfixed-headers-order branch from 378aa75 to cc03955 Compare January 30, 2022 13:10
ofirbarak changed the title lib: fix prefixed headers order http2: fix prefixed headers order Jan 30, 2022
mcollina approved these changes Jan 30, 2022
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

Trott dismissed their stale review January 30, 2022 15:43

commit message updated

nodejs-github-bot commented Feb 1, 2022

Copy link
Copy Markdown
Collaborator

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

This was referenced Feb 2, 2022
Open
Open
Open
Open

nodejs-github-bot commented Feb 6, 2022 •
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

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

jasnell pushed a commit that referenced this pull request Feb 6, 2022
http2: fix pseudo-headers order
ee4bd95 
Keep pseudo-headers order same as sent order

Fixes: #38797

PR-URL: #41735
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell commented Feb 6, 2022

Copy link
Copy Markdown
Member

Landed in ee4bd95

jasnell closed this Feb 6, 2022
ruyadorno pushed a commit that referenced this pull request Feb 8, 2022
http2: fix pseudo-headers order
bfddcb1 
Keep pseudo-headers order same as sent order

Fixes: #38797

PR-URL: #41735
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
ruyadorno mentioned this pull request Feb 8, 2022
Merged
danielleadams pushed a commit that referenced this pull request Mar 2, 2022
http2: fix pseudo-headers order
e4cb290 
Keep pseudo-headers order same as sent order

Fixes: #38797

PR-URL: #41735
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
danielleadams mentioned this pull request Mar 3, 2022
Merged
danielleadams pushed a commit that referenced this pull request Mar 3, 2022
http2: fix pseudo-headers order
511102b 
Keep pseudo-headers order same as sent order

Fixes: #38797

PR-URL: #41735
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
danielleadams pushed a commit to danielleadams/node that referenced this pull request Mar 4, 2022
http2: fix pseudo-headers order
fbb96a8 
Keep pseudo-headers order same as sent order

Fixes: nodejs#38797

PR-URL: nodejs#41735
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 8, 2022
http2: fix pseudo-headers order
ce8e212 
Keep pseudo-headers order same as sent order

Fixes: #38797

PR-URL: #41735
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
danielleadams pushed a commit that referenced this pull request Mar 14, 2022
http2: fix pseudo-headers order
e747ef5 
Keep pseudo-headers order same as sent order

Fixes: #38797

PR-URL: #41735
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
jasnell jasnell approved these changes
Trott Trott left review comments

Assignees

No one assigned

Labels

http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Http2 reverses order of headers starting with ":" prefix

5 participants

Back | FazBrowse Home | New Git URL