ServiceWorker: * is only wildcard at end of scope URLs

ServiceWorker: * is only wildcard at end of scope URLs

BUG=372829
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272621

[jsbell, 2014-05-14 11:59:03]

Anyone?

The spec says:

A path expression consists of a relative url which may, optionally, terminate
with the character "*". Ending a path expression with "*" enables longest-prefix
wildcard matching.

The algorithm just says 'glob-matches URL' which is insufficient - we don't want
? to have special meanings. I'll file a spec bug.

[jsbell, 2014-05-14 12:14:49]

https://github.com/slightlyoff/ServiceWorker/issues/276

[falken, 2014-05-14 14:51:30]

lgtm

https://codereview.chromium.org/288693002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_utils.cc (right):

https://codereview.chromium.org/288693002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_utils.cc:21: const std::string
scope_spec(scope.spec());
can it just be const std::string& scope_spec = scope.spec()?

[jsbell, 2014-05-14 20:31:37]

I still need an OWNER to approve. Anyone else?

https://codereview.chromium.org/288693002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_utils.cc (right):

https://codereview.chromium.org/288693002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_utils.cc:21: const std::string
scope_spec(scope.spec());
On 2014/05/14 14:51:30, falken wrote:
> can it just be const std::string& scope_spec = scope.spec()?

Cool, yes it can! Done.

[kinuko, 2014-05-14 23:44:23]

lgtm

https://codereview.chromium.org/288693002/diff/20001/content/browser/service_...
File content/browser/service_worker/service_worker_utils.cc (right):

https://codereview.chromium.org/288693002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_utils.cc:23: if (len > 0 &&
scope_spec[len - 1] == '*') {
Does this code assume scope is relative or * is in its path part? We should be
calling this only after origin chec's done so I assume it's ok...

[jsbell, 2014-05-16 07:05:45]

https://codereview.chromium.org/288693002/diff/20001/content/browser/service_...
File content/browser/service_worker/service_worker_utils.cc (right):

https://codereview.chromium.org/288693002/diff/20001/content/browser/service_...
content/browser/service_worker/service_worker_utils.cc:23: if (len > 0 &&
scope_spec[len - 1] == '*') {
On 2014/05/14 23:44:23, kinuko wrote:
> Does this code assume scope is relative or * is in its path part? We should be
> calling this only after origin chec's done so I assume it's ok...

It assumes scope GURL is absolute (it's made so in Blink), but I'm not sure
where we're checking to see that scope doesn't have a query or fragment. Since
that can early-exit I'd assume it's on the Blink side, but we can DCHECK here at
least - I'll add that, and add Blink-side tests/fixes if necessary.

[jsbell, 2014-05-19 22:18:19]

FYI: filed https://github.com/slightlyoff/ServiceWorker/issues/284 to track
rejecting the register/unregister request if there's a query or fragment
identifier.

[jsbell, 2014-05-19 22:49:47]

I've changed this so that in the URL-to-test the query & ref are ignored. (In
theory the ref should never make it to the back end anyway.)

[jsbell, 2014-05-20 20:19:30]

https://codereview.chromium.org/291263002/ should land first, which will ensure
that scope doesn't have a fragment.

[jsbell, 2014-05-23 19:33:27]

The CQ bit was checked by jsbell@chromium.org

[commit-bot: I haz the power, 2014-05-23 19:34:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jsbell@chromium.org/288693002/80001

[commit-bot: I haz the power, 2014-05-23 22:46:14]

Change committed as 272621