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

Does url match expression in origin with redirect count? takes a URL, not an origin #519

Closed
annevk opened this issue Jun 16, 2023 · 7 comments · Fixed by #523
Closed

Does url match expression in origin with redirect count? takes a URL, not an origin #519

annevk opened this issue Jun 16, 2023 · 7 comments · Fixed by #523
Assignees

Comments

@annevk
Copy link
Member

annevk commented Jun 16, 2023

https://w3c.github.io/webappsec-permissions-policy/#allowlists doesn't appear to invoke it correctly. Origins aren't compatible with URLs.

@clelland
Copy link
Collaborator

This is probably going to have to end up using something like "let |url| be the result of calling the [=url parser=] on the [=serialization of an origin|serialization=] of |origin|" to get an actual URL that can be used.

@annevk
Copy link
Member Author

annevk commented Jun 22, 2023

Yeah, though perhaps we could redo part of CSP whereby the first half of the algorithm or so becomes its own algorithm that we'd pass url's origin and Permissions Policy can invoke directly.

How does it actually work for paths today?

@clelland
Copy link
Collaborator

Do you mean paths in Permissions Policy declarations? I believe that in practice, paths have always been dropped from the string, and only the origin is used. (Chromium parses the URL, and then extracts the origin, throwing away the userinfo/path/query/fragment/whatever-else)

@annevk
Copy link
Member Author

annevk commented Jun 28, 2023

I was thinking more of the expression containing a path.

@clelland
Copy link
Collaborator

I think that's what I meant, too -- in practice, if the expression in the header / attribute contains a path, then we extract the origin, and store that in the allowlist.

Permissions-Policy: fullscreen=https://example.com/index.html

has exactly the same effect (in Chromium's current implementation, at least) as

Permissions-Policy: fullscreen=https://example.com

That said, I think that's not what the spec currently does -- by my reading, the spec currently would parse https://example.com/index.html into a host-source, which includes the path. Then the call to Does url match expression... would try to match a target origin against the whole host-source, and would return false.

If we want to match the existing behaviour, then we'd need to explicitly clear the path-part from the host-source before storing it.

@arichiv, can you double check that that makes sense?

@annevk
Copy link
Member Author

annevk commented Jun 28, 2023

Hmm, it doesn't really make sense to me that you'd extract an origin from an expression. Perhaps you mean an origin-expression from a url-expression or some such? I can see ignoring/dropping the path though in the expression.

@clelland
Copy link
Collaborator

clelland commented Jul 4, 2023

Sure -- before we had wildcard support, we would parse the string as a URL (assuming it wasn't self, src or *), and store only the origin of the parsed URL.

Now, I believe that we invoke the CSP parser on that string instead, which returns a scheme/host/port/path struct, and we clear the path member before storing it in the allowlist.

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 a pull request may close this issue.

3 participants