This is a regression caused by bug 988186. All members of CacheFileMetadataHeader are now stored in network order but we use keySize in CacheFileMetadata::ParseMetadata() without converting it back to host order.

Comment on attachment 8404572 [details] [diff] [review] fix Review of attachment 8404572 [details] [diff] [review]: ----------------------------------------------------------------- locally tested r=honzab sad we don't have a persistence tests to catch this, need to think of some. ::: netwerk/cache2/CacheFileMetadata.cpp @@ +736,5 @@ > + > + if (mMetaHdr.mVersion != kCacheEntryVersion) { > + LOG(("CacheFileMetadata::ParseMetadata() - Not a version we understand to. " > + "[version=0x%x, this=%p]", mMetaHdr.mVersion, this)); > + return NS_ERROR_UNEXPECTED; is UNEXPECTED really the best error here? (no idea for a better one tho)

(In reply to Honza Bambas (:mayhemer) from comment #1) > ::: netwerk/cache2/CacheFileMetadata.cpp > @@ +736,5 @@ > > + > > + if (mMetaHdr.mVersion != kCacheEntryVersion) { > > + LOG(("CacheFileMetadata::ParseMetadata() - Not a version we understand to. " > > + "[version=0x%x, this=%p]", mMetaHdr.mVersion, this)); > > + return NS_ERROR_UNEXPECTED; > > is UNEXPECTED really the best error here? (no idea for a better one tho) I'm curious about your reply because you chose this error in bug 941698 ;) Seriously, I thought about this while doing review of 941698 and I think UNEXPECTED is OK. Error NS_ERROR_FILE_CORRUPTED is returned elsewhere in CacheFileMetadata::ParseMetadata() but it is IMO not appropriate in this case.

(In reply to Michal Novotny (:michal) from comment #2) > I'm curious about your reply because you chose this error in bug 941698 ;) > Seriously, I thought about this while doing review of 941698 and I think > UNEXPECTED is OK. Error NS_ERROR_FILE_CORRUPTED is returned elsewhere in > CacheFileMetadata::ParseMetadata() but it is IMO not appropriate in this > case. You got me :D OK, yeah, I probably was not able to find a better one that time too. And just forgot about that dilemma...

Honza, is this something you may want to write a test for (I would not want you to be sad!) I can also ask Henrik if it's something he can figure out how to cover in the QA testsuite. Thanks!

We have some plans with Michal to add test for our persisting format. It's a bit tricky and will be covered in a different bug.

Honza, when will this happen? I don't think it's worth our time to create a mozmill test if an in-tree test comes up in a bit.

Delegating to Michal.

I'll try to come up with some test for this in bug 994606 in a reasonable time...

Thank you Michal. I will remove in-qa-testsuite to get it out of our todo list.