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

Assert same-origin for registration matching #1138

Merged
merged 2 commits into from
May 12, 2017
Merged

Conversation

jungkees
Copy link
Collaborator

This patch adds an assertion in the Match Service Worker Registration
that the given URL and the matched URL are the same origin. This also
adds a note that the URL serializer serializes the URLs with a trailing
slash at the end of the origin part of the URLs.

Fixes #1118.

This patch adds an assertion in the Match Service Worker Registration
that the given URL and the matched URL are the same origin. This also
adds a note that the URL serializer serializes the URLs with a trailing
slash at the end of the origin part of the URLs.

Fixes #1118.
@jungkees jungkees requested a review from annevk May 12, 2017 10:35
@jungkees
Copy link
Collaborator Author

@annevk, could you review?

/cc @estark37 @noncombatant

docs/index.bs Outdated
@@ -3345,10 +3345,12 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
1. Let |scopeStringSet| be the result of [=map/get the keys|getting the keys=] from <a>scope to registration map</a>.
1. Set |matchingScopeString| to the longest value in |scopeStringSet| which the value of |clientURLString| starts with, if it exists.

Note: The URL string matching in this step is prefix-based rather than path-structural (e.g. a client URL string with "/prefix-of/resource.html" will match a registration for a scope with "/prefix").
Note: The URL string matching in this step is prefix-based rather than path-structural. E.g. a client URL string with "https://example.com/prefix-of/resource.html" will match a registration for a scope with "https://example.com/prefix". The URL string comparison is safe for the same-origin security as the URLs are serialized with a trailing slash at the end of the origin part of the URLs.
Copy link
Member

Choose a reason for hiding this comment

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

as HTTP(S) URLs are always serialized*

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

docs/index.bs Outdated
1. If |matchingScopeString| is not the empty string, set |matchingScope| to the result of <a lt="URL parser">parsing</a> |matchingScopeString|.
1. If |matchingScopeString| is not the empty string, then:
1. Set |matchingScope| to the result of <a lt="URL parser">parsing</a> |matchingScopeString|.
1. Assert: |matchingScope|'s [=url/origin=] and |clientURL|'s [=url/origin=] are the [=same origin=].
Copy link
Member

Choose a reason for hiding this comment

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

s/the//

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@jungkees jungkees merged commit ddc7c64 into master May 12, 2017
@jungkees jungkees deleted the assert-same-origin branch May 12, 2017 12:49
@jungkees
Copy link
Collaborator Author

@annevk, thanks for reviewing!

@annevk
Copy link
Member

annevk commented May 12, 2017

So I didn't mean that you should remove the end of the sentence... The bit about slashes and origins was still useful.

@annevk
Copy link
Member

annevk commented May 12, 2017

I don't think the sentence makes much sense as-is.

@jungkees
Copy link
Collaborator Author

Oh.. so you meant something like the following?

The URL string comparison is safe for the same-origin security as HTTP(S) URLs are always serialized with a trailing slash at the end of the origin part of the URLs.

@annevk
Copy link
Member

annevk commented May 12, 2017

Yeah, that looks better.

@annevk
Copy link
Member

annevk commented May 12, 2017

(For next time, ask for review again. Happy to do it.)

@jungkees
Copy link
Collaborator Author

Okay. Will do. Thanks!

@jungkees
Copy link
Collaborator Author

Addressed: 46b5d67.

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.

None yet

2 participants