Skip to content

fix: pass full response headers in net module#21552

Merged
nornagon merged 4 commits intomasterfrom
fix-set-cookie
Jan 14, 2020
Merged

fix: pass full response headers in net module#21552
nornagon merged 4 commits intomasterfrom
fix-set-cookie

Conversation

@zcbenz
Copy link
Contributor

@zcbenz zcbenz commented Dec 17, 2019

Description of Change

Close #20631.

Use raw headers instead of filtered headers in net module. This is a regression caused by network service refactoring.

Checklist

Release Notes

Notes: Fix set-cookie header not passed in net module.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Dec 17, 2019
nornagon
nornagon suggested changes Dec 17, 2019
View reviewed changes
Copy link
Contributor

@nornagon nornagon left a comment

Choose a reason for hiding this comment

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

What other kinds of filtering is Chromium doing here? Is it doing any sort of normalization that we might not want to lose? Do we only need the set-cookie headers here?

@zcbenz
Copy link
Contributor Author

zcbenz commented Dec 18, 2019

What other kinds of filtering is Chromium doing here? Is it doing any sort of normalization that we might not want to lose? Do we only need the set-cookie headers here?

The raw headers are populated from net::URLRequest::response_headers(), and it is the same with 6.x, which uses net::URLRequest directly:
https://cs.chromium.org/chromium/src/services/network/loader_util.cc?type=cs&sq=package:chromium&g=0&l=102-120

Currently I'm only aware the set-cookie has been filtered because Chromium blocks third party cookies. I couldn't find a way to work around this without patching Chromium as the requests of net module are sent from browser process, and setting cookie sites does not seem to work.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Dec 18, 2019
@zcbenz
Copy link
Contributor Author

zcbenz commented Jan 13, 2020

@nornagon Can you take another look at this?

nornagon
nornagon reviewed Jan 13, 2020
View reviewed changes
@zcbenz zcbenz requested a review from nornagon January 14, 2020 07:04
@nornagon nornagon merged commit ceacadb into master Jan 14, 2020
@release-clerk
Copy link

release-clerk bot commented Jan 14, 2020

Release Notes Persisted

Fix set-cookie header not passed in net module.

@nornagon nornagon deleted the fix-set-cookie branch January 14, 2020 17:36
@trop trop bot mentioned this pull request Jan 14, 2020
Merged
@trop
Copy link
Contributor

trop bot commented Jan 14, 2020

I have automatically backported this PR to "8-x-y", please check out #21769

@trop trop bot mentioned this pull request Jan 14, 2020
Merged
@trop
Copy link
Contributor

trop bot commented Jan 14, 2020

I have automatically backported this PR to "7-1-x", please check out #21770

@nornagon nornagon mentioned this pull request Mar 11, 2021
Closed
dsanders11 added a commit to dsanders11/electron that referenced this pull request Jun 9, 2021
@dsanders11
chore: revert electron#21552
2bfb5cd
dsanders11 added a commit to dsanders11/electron that referenced this pull request Mar 16, 2022
@dsanders11
chore: partially revert electron#21552
ab5d33a
dsanders11 added a commit to dsanders11/electron that referenced this pull request Mar 16, 2022
@dsanders11
chore: partially revert electron#21552
315f455
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@codebytere codebytere Awaiting requested review from codebytere

1 more reviewer

@nornagon nornagon nornagon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Electron 7, request using net module never contains cookies in response

2 participants

@zcbenz @nornagon