Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a structured header for preflights #45

Open
letitz opened this issue Apr 13, 2021 · 10 comments
Open

Use a structured header for preflights #45

letitz opened this issue Apr 13, 2021 · 10 comments

Comments

@letitz
Copy link
Collaborator

letitz commented Apr 13, 2021

@martinthomson makes a point in w3ctag/design-reviews#572 (comment):

It seems like this could use a structured field, but I realize that the original version of this predates that work by a lot.

It's true that we could use a structured field for the new headers. The only downside I could see is inconsistency with other CORS headers. @mikewest what do you think?

@annevk
Copy link

annevk commented Apr 13, 2021

I suppose the main inconsistency would be with Access-Control-Allow-Credentials if we changed Access-Control-Allow-Private-Network to be a boolean. If it were a token it could remain consistent (and we could upgrade Access-Control-Allow-Credentials to that), but it would perhaps be a little weird.

@letitz
Copy link
Collaborator Author

letitz commented Apr 13, 2021

I see. I think it would be best to maintain consistency between both. We can indeed use a token "true" for this, but it feels weird when boolean structured fields are right there. I guess the question becomes: what are the benefits to doing that?

@annevk
Copy link

annevk commented Apr 13, 2021

There might be a benefit in the future if/when structured fields get their own on-the-wire representation.

We should probably migrate these legacy fields to the extent possible as it's a minor win of sorts, but as there's no on-the-wire representation that's different from legacy fields it hasn't been a priority (or observable).

@letitz
Copy link
Collaborator Author

letitz commented Apr 14, 2021

Filed whatwg/fetch#1216 to use structured fields "upstream". That issue blocks this one for now.

@letitz letitz added the blocked Blocked on some other issue. label Apr 14, 2021
@annevk
Copy link

annevk commented Dec 2, 2021

I think this might be a rare instance where structured headers are not quite the right fit, largely due to it extending an existing design. I do think specifying it as a structured header could work, but it should be with a token value, not a boolean, as only that's fully compatible.

@letitz
Copy link
Collaborator Author

letitz commented Dec 2, 2021

It could be specified as both, to keep the door open to structuring Access-Control-Allow-Credentials in the future? It seems to me that such a migration would not have great benefit though, so I agree with you.

@annevk
Copy link

annevk commented Dec 2, 2021

Both token and boolean seems more trouble than it's worth.

@letitz
Copy link
Collaborator Author

letitz commented Dec 2, 2021

Oh, would a "true" token be incompatible? I was under the impression that it could be a spec-only change.

@annevk
Copy link

annevk commented Dec 2, 2021

I meant having both. :-) I think a `true` token works.

@letitz
Copy link
Collaborator Author

letitz commented Dec 2, 2021

Oh, ha! Got it.

@letitz letitz removed the blocked Blocked on some other issue. label Dec 6, 2021
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

No branches or pull requests

2 participants