avoid cache.add() and cache.addAll() for remote resources

[wanderview]

FYI, there is a footgun in cache.addAll(). If the remote server response with a 501 it will still go into the cache.

I recommend replacing these calls with explicit fetch(), response.ok() check, and cache.put().

Note, if you are dealing with any opaque responses, though, there is no way to tell if you actually got an ok response or not. Because security.

[marcoscaceres]

On 2 Dec 2015, at 5:08 AM, Ben Kelly notifications@github.com wrote:

FYI, there is a footgun in cache.addAll(). If the remote server response with a 501 it will still go into the cache.

Yeah, I had also realized that. It basically subverts my whole cache tasks thing I made because there is no cache control checks:( will fix as you suggest below.

I recommend replacing these calls with explicit fetch(), response.ok() check, and cache.put().

Agree.

Note, if you are dealing with any opaque responses, though, there is no way to tell if you actually got an ok response or not. Because security.

I think we should be mostly ok... Though this might affect the Ads Provider because those image responses are coming from a CDN.

— Reply to this email directly or view it on GitHub.

[wanderview]

For no-cors cross-origin stuff you may want to do network-first and always update cache with network result to mitigate any bad values that get stored for opaque responses.

[rlr]

As @wanderview suggested, I kept cache.addAll() but changed every fetch to go to network before cache. Closing.