780750 - HTTP disk cache never gets used/fixed if wrong permissions on CACHE_MAP file

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Jason Duell]

Attachment: v1

Just found this while reviewing an unrelated patch--no evidence it's happening in the wild.

Steps to repro:   
  1) chmod 000 FIREFOX_PROFILE/Cache/_CACHE_001_ 
  2) run firefox: debugger shows we give up on opening disk cache, but we also don't delete cache.  So the disk cache will never be used again until/unless something else than firefox changes things :(

Culprit: the 'just return error' logic if we see any error code other than NS_ERROR_CORRUPT:

  http://mxr.mozilla.org/mozilla-central/source/netwerk/cache/nsDiskCacheDevice.cpp?rev=4c632ce71344#1007

nsDiskCacheMap::Open does a valiant job of trying to convert errors to CORRUPT when appropriate, but it missed this case at least.   Instead of trying to fix that whole call tree (likely to break when someone changes the code), I think it makes more sense to whitelist the errors.  AFAICT the only error we can hit where we'd NOT want to delete/recreate the cache is NS_ERROR_ALREADY_INITIALIZED (I'm not really clear on whether hitting that is a logical error; for now I'm just returning, same as the code has done previously).

My old comment for the 'else' block was totally off--for new profiles we'll fail the mCacheDirectory->Exists test above, so will never get to that block.

I've verified the fix works for the repro case.

Comment 1

[u408661]

Comment on attachment 649438 [details] [diff] [review]
v1

Review of attachment 649438 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good (and seems like it even fixes a subtle bug in the collection of corrupt details telemetry!) r=me. If you like, I can rebase this on top of my patch for bug 709297 and push them both at the same time (will probably run the combo through try just to make sure there aren't any bad interactions we're missing)

Comment 2

[Jason Duell]

That would be great--thanks.

Comment 3

[u408661]

Try looks good for this + bug 709297 https://tbpl.mozilla.org/?tree=Try&rev=988be862d179

Comment 4

[u408661]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c7231d7d68e8

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c7231d7d68e8