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

http: split set-cookie when using setHeaders by marco-ippolito · Pull Request #51649 · nodejs/node · GitHub

/ node Public

http: split set-cookie when using setHeaders - #51649

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
marco-ippolito:feat/set-headers-cookies
Feb 4, 2024
Merged

http: split set-cookie when using setHeaders#51649
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
marco-ippolito:feat/set-headers-cookies

Conversation

marco-ippolito commented Feb 2, 2024
edited
Loading

Copy link
Copy Markdown
Member

Fixes: #51599
This allows set-cookie header to work correctly with setHeaders

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Feb 2, 2024
marco-ippolito force-pushed the feat/set-headers-cookies branch from a6ad1c6 to ab75703 Compare February 2, 2024 11:41

ShogunPanda left a comment

Copy link
Copy Markdown
Contributor

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!

Comment thread lib/_http_outgoing.js Outdated
marco-ippolito force-pushed the feat/set-headers-cookies branch from ab75703 to ff72f2f Compare February 2, 2024 14:35
Comment thread lib/_http_outgoing.js Outdated
Comment thread lib/_http_outgoing.js Outdated
Comment thread lib/_http_outgoing.js Outdated
marco-ippolito force-pushed the feat/set-headers-cookies branch from ff72f2f to 330e8bb Compare February 2, 2024 17:00
marco-ippolito force-pushed the feat/set-headers-cookies branch from 330e8bb to 094349f Compare February 3, 2024 08:14
marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2024

Copy link
Copy Markdown
Collaborator

Comment thread lib/_http_outgoing.js Outdated
Co-authored-by: akhil marsonya <16393876+marsonya@users.noreply.github.com>
marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2024

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

marco-ippolito added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed needs-ci PRs that need a full CI run. labels Feb 4, 2024
marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 4, 2024
nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 4, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/51649
✔  Done loading data for nodejs/node/pull/51649
----------------------------------- PR info ------------------------------------
Title      http: split set-cookie when using setHeaders (#51649)
Author     Marco Ippolito  (@marco-ippolito)
Branch     marco-ippolito:feat/set-headers-cookies -> nodejs:main
Labels     http, author ready
Commits    2
 - http: split set-cookie when using setHeaders
 - Update lib/_http_outgoing.js
Committers 2
 - marco-ippolito 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/51649
Fixes: https://github.com/nodejs/node/issues/51599
Reviewed-By: Paolo Insogna 
Reviewed-By: Luigi Pinca 
Reviewed-By: Akhil Marsonya 
Reviewed-By: Minwoo Jung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51649
Fixes: https://github.com/nodejs/node/issues/51599
Reviewed-By: Paolo Insogna 
Reviewed-By: Luigi Pinca 
Reviewed-By: Akhil Marsonya 
Reviewed-By: Minwoo Jung 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 02 Feb 2024 11:18:39 GMT
   ✔  Approvals: 4
   ✔  - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/51649#pullrequestreview-1860949554
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/51649#pullrequestreview-1860948676
   ✔  - Akhil Marsonya (@marsonya): https://github.com/nodejs/node/pull/51649#pullrequestreview-1860999209
   ✔  - Minwoo Jung (@JungMinu): https://github.com/nodejs/node/pull/51649#pullrequestreview-1861448465
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-02-04T09:38:30Z: https://ci.nodejs.org/job/node-test-pull-request/57047/
- Querying data for job/node-test-pull-request/57047/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 51649
From https://github.com/nodejs/node
 * branch                  refs/pull/51649/merge -> FETCH_HEAD
✔  Fetched commits as c975384264dc..d546c502a56e
--------------------------------------------------------------------------------
[main 4c1cc83b8f] http: split set-cookie when using setHeaders
 Author: marco-ippolito 
 Date: Fri Feb 2 11:55:37 2024 +0100
 2 files changed, 64 insertions(+), 2 deletions(-)
[main 918a9ad1d1] Update lib/_http_outgoing.js
 Author: Marco Ippolito 
 Date: Sat Feb 3 12:23:09 2024 +0100
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
http: split set-cookie when using setHeaders

PR-URL: #51649
Fixes: #51599
Reviewed-By: Paolo Insogna paolo@cowtech.it
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Akhil Marsonya akhil.marsonya27@gmail.com
Reviewed-By: Minwoo Jung nodecorelab@gmail.com

[detached HEAD 23373b6075] http: split set-cookie when using setHeaders
Author: marco-ippolito marcoippolito54@gmail.com
Date: Fri Feb 2 11:55:37 2024 +0100
2 files changed, 64 insertions(+), 2 deletions(-)
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Update lib/_http_outgoing.js

Co-authored-by: akhil marsonya 16393876+marsonya@users.noreply.github.com
PR-URL: #51649
Fixes: #51599
Reviewed-By: Paolo Insogna paolo@cowtech.it
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Akhil Marsonya akhil.marsonya27@gmail.com
Reviewed-By: Minwoo Jung nodecorelab@gmail.com

[detached HEAD 33837b68ff] Update lib/_http_outgoing.js
Author: Marco Ippolito marcoippolito54@gmail.com
Date: Sat Feb 3 12:23:09 2024 +0100
1 file changed, 1 insertion(+), 1 deletion(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/7775666607

marco-ippolito added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Feb 4, 2024
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 4, 2024
nodejs-github-bot merged commit 9448c61 into nodejs:main Feb 4, 2024

Copy link
Copy Markdown
Collaborator

Landed in 9448c61

rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Feb 9, 2024
PR-URL: nodejs#51649
Fixes: nodejs#51599
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
targos pushed a commit that referenced this pull request Feb 15, 2024
PR-URL: #51649
Fixes: #51599
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
marco-ippolito mentioned this pull request Mar 1, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
PR-URL: #51649
Fixes: #51599
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
richardlau mentioned this pull request Mar 25, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. http Issues or PRs related to the http subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OutgoingMessage.setHeaders(Headers) doesn't handle Set-Cookie headers properly.

10 participants


Back | FazBrowse Home | New Git URL