Skip to content

net: Allow * in Access-Control-Allow-Headers to cover authorization - #47600

Merged
yezhizhen merged 2 commits into
servo:mainfrom
yezhizhen:net
Aug 30, 2026
Merged

yezhizhen merged 2 commits into
servo:mainfrom
yezhizhen:net

Conversation

@yezhizhen

@yezhizhen yezhizhen commented Aug 28, 2026 •

Copy link
Copy Markdown
Member

Zulip chat.

According to MDN/Spec/WPT tests, this should not be allowed.
However, all browsers allow it, and fail these WPT tests.
This is discovered when visiting some websites but failed with "CORS Authorization check".

Testing: Updated WPT. All other browsers fail these specific tests.
Test1
Test2
Test3


Spec issue: whatwg/fetch#1919

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Aug 28, 2026
@yezhizhen yezhizhen changed the title net: Allow * in Access-Control-Allow-Headers to cover authorization net: Allow * in Access-Control-Allow-Headers to cover Authorization Aug 28, 2026
@yezhizhen yezhizhen changed the title net: Allow * in Access-Control-Allow-Headers to cover Authorization net: Allow * in Access-Control-Allow-Headers to cover authorization Aug 28, 2026
@Narfinger

Copy link
Copy Markdown
Contributor

Should you file something with the spec?

@yezhizhen

yezhizhen commented Aug 28, 2026 •

Copy link
Copy Markdown
Member Author

Should you file something with the spec?

Thanks! Found it: whatwg/fetch#1919

@Narfinger Narfinger 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.

I think we should allow this but I kind of don't want to merge this on my own.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Aug 28, 2026

@TimvdLippe TimvdLippe 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.

Can you send a PR to the spec with an updated text and then use that text here? Additionally, let's update the relevant WPT tests that started failing to instead expect the behavior of browsers. Then, let's ask for consensus on the issue you linked that the updated spec text is accepted and the tests are asserting the correct behavior. Once that's settled, I would be comfortable merging this PR.

@servo-highfive servo-highfive added the S-needs-code-changes Changes have not yet been made that were requested by a reviewer. label Aug 28, 2026
@yezhizhen

yezhizhen commented Aug 28, 2026 •

Copy link
Copy Markdown
Member Author

Can you send a PR to the spec with an updated text and then use that text here? Additionally, let's update the relevant WPT tests that started failing to instead expect the behavior of browsers. Then, let's ask for consensus on the issue you linked that the updated spec text is accepted and the tests are asserting the correct behavior. Once that's settled, I would be comfortable merging this PR.

  1. That's a lot to ask from me. All I want is trying to visit some sites all other browsers can do. I think this is a very general issue across many sites.
  2. I still get several pending WHATWG spec PRs such as Editorial: Remove redundant assertion in "ReadableStream pipe through" and "ReadableStream pipe to" whatwg/streams#1375 because I cannot do IPR. So opening PR to the spec is not possible for me.
  3. For updating wpt tests, people are aware of the issue, but still added new ones recently to reflect current spec situation: Disallowing Authorization in CORS-preflight wildcards is currently not web compatible whatwg/fetch#1919 (comment)
  4. LibWeb: Allow CORS wildcards to cover Authorization LadybirdBrowser/ladybird#11181 Ladybird also recently adapted to this behaviour.

@yezhizhen

yezhizhen commented Aug 28, 2026 •

Copy link
Copy Markdown
Member Author

And 5. The very original issue whatwg/fetch#1278 is open for 5 years. If it's not settled yet, it is difficult for me alone to reach the conclusive spec text fix, assuming I can resolve IPR issue someday.

@TimvdLippe

Copy link
Copy Markdown
Contributor

Seems like Anne pinged Chrome folks in WhatWG chat: https://matrix.to/#/!AGetWbsMpFPdSgUrbs:matrix.org/$f3_ETYVhzovvCkMW0mgYFctXNX_rimXqamHyoXWTQro?via=matrix.org&via=mozilla.org&via=igalia.com Let's wait on their resolution and then decide how to proceed forwards.

@yezhizhen
yezhizhen requested a review from jdm August 29, 2026 00:30
@yezhizhen

yezhizhen commented Aug 29, 2026 •

Copy link
Copy Markdown
Member Author

Let's wait on their resolution

These can take months/years assuming it finally happens. This blocks many sites such as https://bsky.app/. Maybe you are not happy with wpt passing rate, but at the end of the day, it is actual web compatibility that matters. And some people do use Servo to access websites.

I think we should land this like other browsers. If there is real progress on the spec, I'm happy to adapt then.

@TimvdLippe

Copy link
Copy Markdown
Contributor

I am not proposing we wait months/years, that indeed is unreasonable. My proposal is as follows:

  1. Gather a list of sites that you know are affected and post on the spec issue describing the problem with web compatibility. The fact that BlueSky is affected is a big sign imo
  2. We wait for the response from Chromium folks on Monday
  3. If there is no resolution by Friday, we merge this PR
  4. If there is a resolution, we follow that, helping out with updating WPT and spec updates if that's what is required

Does that sound reasonable to you?

@yezhizhen

yezhizhen commented Aug 30, 2026 •

Copy link
Copy Markdown
Member Author

I find this conversation uncomfortable, if this is called conversation at all. I'm only receiving imperatives from very beginning and specific timelines now.
Maybe I can do these and I wanted to in the beginning, but only on my own will. But still, they are independent of this one.

@yezhizhen
yezhizhen added this pull request to the merge queue Aug 30, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Aug 30, 2026
Merged via the queue into servo:main with commit 3ed5883 Aug 30, 2026
42 checks passed
@yezhizhen
yezhizhen deleted the net branch August 30, 2026 03:22
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants