Update algorithm and redirects

[annevk]

I don't understand why this section checks "redirect count". That is a bookkeeping detail of the Fetch specification and is not for other specifications.

If somebody could explain the intent I can maybe say what we should do instead.

[annevk]

This issue is blocking Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1131271

[jungkees]

I'm also looking up the history. If a redirection should be considered as a security error, it should not be able to be succeeded at all, either.

/cc @slightlyoff @jakearchibald

[annevk]

Yeah, but if you want to block redirects you should not be checking "redirect count". Instead you should set the manual redirect flag and check the status code of the response (likely require it to be in range 200 to 299).

[jungkees]

Alright. Depending on the decision, I'll update it as commented. A question is, shouldn't the response code be one of 301/302/303/307/308 when the manual redirect flag is set?

[annevk]

I don't understand the question.

[jungkees]

I mean, "Shouldn't the response code be one of 301/302/303/307/308 in order to consider it a security (redirection) error"? With "likely require it to be in range 200 to 299" in your comment above, did you mean responses with those status code are NOT security error?

[annevk]

I mean we should only respect those response code if we are not going to handle redirects.

[jungkees]

Discussed with @slightlyoff and concluded preventing redirection is the safe thing to do here. Updated the text as such: 4f3a555. @annevk please review.

[annevk]

Could you please share the rationale? Why do you reject with a "NetworkError" while fetch() rejects with a TypeError?

[jungkees]

I thought it was a right DOMException as fetch pertains to network operation. Happy to align it to fetch. Is there any general guideline when to use simple exception (TypeError) and when to use DOMException?

[jungkees]

If you asked about the rationale why we'd want to prevent redirection, we thought it'd be part of not allowing x-origin SWs.

[annevk]

You can prevent that by setting the fetch mode to same-origin, no?

[annevk]

(I don't think we have guidelines on exceptions.)

[jakearchibald]

If we want to allow redirects for the SW script, there's a few questions we need to answer:

1. If the registration's script url is /foo.js, and during an update it hits a redirect to '/bar.js', which url will be used for the next update check? Do redirects update the script url on the registration object?
2. If the scope is /foo/ and the script url is /foo/bar.js, but redirects to /foo/scripts/bar.js, should that work?
3. If the scope is /foo/ and the script url is /foo/scripts/bar.js, but redirects to /foo/bar.js, should that work?

I think 'no' for all three.

[annevk]

I would expect yes for 1 and the others explain why introducing scopes just for service workers is a hack. :-(

[jakearchibald]

The reason I say no for 1 is because you're otherwise likely to introduce thrashing.

1. registration's script url is /foo.js
2. update happens, redirect hit, registration's script url is /bar.js
3. page refreshes, encounters .register('/foo.js'), registration's script url is back to /foo.js, this triggers a fetch of /foo.js because the registration has changed as a result of the register call

2 & 3 aren't solved by dropping scopes, the problem is caused by catering to sites that thinking giving someone webspace in a directory sandboxes them to that director.

[annevk]

If /foo.js is fetched it will redirect as well so I'm not sure why the script URL would be /foo.js.

[jungkees]

As the script URL passed to .register was /foo.js again while registration's workers' script URL had been set to /bar.js in the previous register, this new register request would not resolve with the existing registration but rather trigger new update every time.

Why do you reject with a "NetworkError" while fetch() rejects with a TypeError?

e9fabe3

[annevk]

So this is all done now, correct? That is, we gave up on redirects due to the design of the register() API.

[annevk]

Where is the issue about setting request's client to null?

[jungkees]

So this is all done now, correct? That is, we gave up on redirects due to the design of the register() API.

Yes.

Where is the issue about setting request's client to null?

This is the one: #615.

[jungkees]

So let me close this issue. The issue about setting request's client to null is tracked in #615, and pointed by from Soft Update algorithm in the spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#soft-update-algorithm.