Bug 1141555 - HTTP cache v2 generates corrupted entries, r=jduell
authorMichal Novotny <michal.novotny@gmail.com>
Thu, 26 Mar 2015 23:05:34 +0100
changeset 235918 1b374964865c550409812bc72d3aa3804ce599ca
parent 235917 b617b76326f7b7000703e8625d49543662846ca9
child 235919 28dca8aee5844b99d18954af858fdbdeb6e2bfc3
push id57549
push usermnovotny@mozilla.com
push dateThu, 26 Mar 2015 22:05:46 +0000
treeherdermozilla-inbound@1b374964865c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjduell
bugs1141555
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1141555 - HTTP cache v2 generates corrupted entries, r=jduell
netwerk/cache2/CacheFileChunk.cpp
netwerk/cache2/CacheFileIOManager.cpp
netwerk/cache2/CacheFileIOManager.h
netwerk/cache2/CacheFileMetadata.cpp
netwerk/cache2/CacheIndex.cpp
--- a/netwerk/cache2/CacheFileChunk.cpp
+++ b/netwerk/cache2/CacheFileChunk.cpp
@@ -233,17 +233,17 @@ CacheFileChunk::Write(CacheFileHandle *a
 
   mState = WRITING;
   mRWBuf = mBuf;
   mRWBufSize = mBufSize;
   mBuf = nullptr;
   mBufSize = 0;
 
   rv = CacheFileIOManager::Write(aHandle, mIndex * kChunkSize, mRWBuf,
-                                 mDataSize, false, this);
+                                 mDataSize, false, false, this);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     SetError(rv);
   } else {
     mListener = aCallback;
     mIsDirty = false;
   }
 
   return rv;
--- a/netwerk/cache2/CacheFileIOManager.cpp
+++ b/netwerk/cache2/CacheFileIOManager.cpp
@@ -661,22 +661,24 @@ protected:
   char                         *mBuf;
   int32_t                       mCount;
   nsCOMPtr<CacheFileIOListener> mCallback;
 };
 
 class WriteEvent : public nsRunnable {
 public:
   WriteEvent(CacheFileHandle *aHandle, int64_t aOffset, const char *aBuf,
-             int32_t aCount, bool aValidate, CacheFileIOListener *aCallback)
+             int32_t aCount, bool aValidate, bool aTruncate,
+             CacheFileIOListener *aCallback)
     : mHandle(aHandle)
     , mOffset(aOffset)
     , mBuf(aBuf)
     , mCount(aCount)
     , mValidate(aValidate)
+    , mTruncate(aTruncate)
     , mCallback(aCallback)
   {
     MOZ_COUNT_CTOR(WriteEvent);
 
     MOZ_EVENT_TRACER_NAME_OBJECT(static_cast<nsIRunnable*>(this), aHandle->Key().get());
     MOZ_EVENT_TRACER_WAIT(static_cast<nsIRunnable*>(this), "net::cache::write-background");
   }
 
@@ -695,17 +697,17 @@ public:
   {
     nsresult rv;
 
     MOZ_EVENT_TRACER_EXEC(static_cast<nsIRunnable*>(this), "net::cache::write-background");
     if (mHandle->IsClosed()) {
       rv = NS_ERROR_NOT_INITIALIZED;
     } else {
       rv = CacheFileIOManager::gInstance->WriteInternal(
-          mHandle, mOffset, mBuf, mCount, mValidate);
+          mHandle, mOffset, mBuf, mCount, mValidate, mTruncate);
       if (NS_FAILED(rv) && !mCallback) {
         // No listener is going to handle the error, doom the file
         CacheFileIOManager::gInstance->DoomFileInternal(mHandle);
       }
     }
     MOZ_EVENT_TRACER_DONE(static_cast<nsIRunnable*>(this), "net::cache::write-background");
 
     MOZ_EVENT_TRACER_EXEC(static_cast<nsIRunnable*>(this), "net::cache::write-result");
@@ -720,17 +722,18 @@ public:
     return NS_OK;
   }
 
 protected:
   nsRefPtr<CacheFileHandle>     mHandle;
   int64_t                       mOffset;
   const char                   *mBuf;
   int32_t                       mCount;
-  bool                          mValidate;
+  bool                          mValidate : 1;
+  bool                          mTruncate : 1;
   nsCOMPtr<CacheFileIOListener> mCallback;
 };
 
 class DoomFileEvent : public nsRunnable {
 public:
   DoomFileEvent(CacheFileHandle *aHandle,
                 CacheFileIOListener *aCallback)
     : mCallback(aCallback)
@@ -1851,49 +1854,75 @@ CacheFileIOManager::ReadInternal(CacheFi
 
   return NS_OK;
 }
 
 // static
 nsresult
 CacheFileIOManager::Write(CacheFileHandle *aHandle, int64_t aOffset,
                           const char *aBuf, int32_t aCount, bool aValidate,
-                          CacheFileIOListener *aCallback)
+                          bool aTruncate, CacheFileIOListener *aCallback)
 {
   LOG(("CacheFileIOManager::Write() [handle=%p, offset=%lld, count=%d, "
-       "validate=%d, listener=%p]", aHandle, aOffset, aCount, aValidate,
-       aCallback));
+       "validate=%d, truncate=%d, listener=%p]", aHandle, aOffset, aCount,
+       aValidate, aTruncate, aCallback));
 
   nsresult rv;
   nsRefPtr<CacheFileIOManager> ioMan = gInstance;
 
   if (aHandle->IsClosed() || !ioMan) {
     if (!aCallback) {
       // When no callback is provided, CacheFileIOManager is responsible for
       // releasing the buffer. We must release it even in case of failure.
       free(const_cast<char *>(aBuf));
     }
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   nsRefPtr<WriteEvent> ev = new WriteEvent(aHandle, aOffset, aBuf, aCount,
-                                           aValidate, aCallback);
+                                           aValidate, aTruncate, aCallback);
   rv = ioMan->mIOThread->Dispatch(ev, CacheIOThread::WRITE);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
+static nsresult
+TruncFile(PRFileDesc *aFD, int64_t aEOF)
+{
+#if defined(XP_UNIX)
+  if (ftruncate(PR_FileDesc2NativeHandle(aFD), aEOF) != 0) {
+    NS_ERROR("ftruncate failed");
+    return NS_ERROR_FAILURE;
+  }
+#elif defined(XP_WIN)
+  int64_t cnt = PR_Seek64(aFD, aEOF, PR_SEEK_SET);
+  if (cnt == -1) {
+    return NS_ERROR_FAILURE;
+  }
+  if (!SetEndOfFile((HANDLE) PR_FileDesc2NativeHandle(aFD))) {
+    NS_ERROR("SetEndOfFile failed");
+    return NS_ERROR_FAILURE;
+  }
+#else
+  MOZ_ASSERT(false, "Not implemented!");
+  return NS_ERROR_NOT_IMPLEMENTED;
+#endif
+
+  return NS_OK;
+}
+
 nsresult
 CacheFileIOManager::WriteInternal(CacheFileHandle *aHandle, int64_t aOffset,
                                   const char *aBuf, int32_t aCount,
-                                  bool aValidate)
+                                  bool aValidate, bool aTruncate)
 {
   LOG(("CacheFileIOManager::WriteInternal() [handle=%p, offset=%lld, count=%d, "
-       "validate=%d]", aHandle, aOffset, aCount, aValidate));
+       "validate=%d, truncate=%d]", aHandle, aOffset, aCount, aValidate,
+       aTruncate));
 
   nsresult rv;
 
   if (!aHandle->mFileExists) {
     rv = CreateFile(aHandle);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -1931,23 +1960,40 @@ CacheFileIOManager::WriteInternal(CacheF
 
   int64_t offset = PR_Seek64(aHandle->mFD, aOffset, PR_SEEK_SET);
   if (offset == -1) {
     return NS_ERROR_FAILURE;
   }
 
   int32_t bytesWritten = PR_Write(aHandle->mFD, aBuf, aCount);
 
-  if (bytesWritten != -1 && aHandle->mFileSize < aOffset+bytesWritten) {
-    aHandle->mFileSize = aOffset+bytesWritten;
-
-    if (!aHandle->IsDoomed() && !aHandle->IsSpecialFile()) {
-      uint32_t size = aHandle->FileSizeInK();
-      CacheIndex::UpdateEntry(aHandle->Hash(), nullptr, nullptr, &size);
-      EvictIfOverLimitInternal();
+  if (bytesWritten != -1) {
+    uint32_t oldSizeInK = aHandle->FileSizeInK();
+    int64_t writeEnd = aOffset + bytesWritten;
+
+    if (aTruncate) {
+      rv = TruncFile(aHandle->mFD, writeEnd);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      aHandle->mFileSize = writeEnd;
+    } else {
+      if (aHandle->mFileSize < writeEnd) {
+        aHandle->mFileSize = writeEnd;
+      }
+    }
+
+    uint32_t newSizeInK = aHandle->FileSizeInK();
+
+    if (oldSizeInK != newSizeInK && !aHandle->IsDoomed() &&
+        !aHandle->IsSpecialFile()) {
+      CacheIndex::UpdateEntry(aHandle->Hash(), nullptr, nullptr, &newSizeInK);
+
+      if (oldSizeInK < newSizeInK) {
+        EvictIfOverLimitInternal();
+      }
     }
   }
 
   if (bytesWritten != aCount) {
     return NS_ERROR_FAILURE;
   }
 
   // Write was successful and this write validates the entry (i.e. metadata)
@@ -2297,40 +2343,16 @@ CacheFileIOManager::GetEntryInfo(const S
 
   // Call directly on the callback.
   aCallback->OnEntryInfo(uriSpec, enhanceId, dataSize, fetchCount,
                          lastModified, expirationTime);
 
   return NS_OK;
 }
 
-static nsresult
-TruncFile(PRFileDesc *aFD, uint32_t aEOF)
-{
-#if defined(XP_UNIX)
-  if (ftruncate(PR_FileDesc2NativeHandle(aFD), aEOF) != 0) {
-    NS_ERROR("ftruncate failed");
-    return NS_ERROR_FAILURE;
-  }
-#elif defined(XP_WIN)
-  int32_t cnt = PR_Seek(aFD, aEOF, PR_SEEK_SET);
-  if (cnt == -1) {
-    return NS_ERROR_FAILURE;
-  }
-  if (!SetEndOfFile((HANDLE) PR_FileDesc2NativeHandle(aFD))) {
-    NS_ERROR("SetEndOfFile failed");
-    return NS_ERROR_FAILURE;
-  }
-#else
-  MOZ_ASSERT(false, "Not implemented!");
-#endif
-
-  return NS_OK;
-}
-
 nsresult
 CacheFileIOManager::TruncateSeekSetEOFInternal(CacheFileHandle *aHandle,
                                                int64_t aTruncatePos,
                                                int64_t aEOFPos)
 {
   LOG(("CacheFileIOManager::TruncateSeekSetEOFInternal() [handle=%p, "
        "truncatePos=%lld, EOFPos=%lld]", aHandle, aTruncatePos, aEOFPos));
 
@@ -2348,24 +2370,57 @@ CacheFileIOManager::TruncateSeekSetEOFIn
     NSPRHandleUsed(aHandle);
   }
 
   // Check again, OpenNSPRHandle could figure out the file was gone.
   if (!aHandle->mFileExists) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
+  // Check whether this operation would cause critical low disk space.
+  if (aHandle->mFileSize < aEOFPos) {
+    int64_t freeSpace = -1;
+    rv = mCacheDirectory->GetDiskSpaceAvailable(&freeSpace);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      LOG(("CacheFileIOManager::TruncateSeekSetEOFInternal() - "
+           "GetDiskSpaceAvailable() failed! [rv=0x%08x]", rv));
+    } else {
+      uint32_t limit = CacheObserver::DiskFreeSpaceHardLimit();
+      if (freeSpace - aEOFPos + aHandle->mFileSize < limit) {
+        LOG(("CacheFileIOManager::TruncateSeekSetEOFInternal() - Low free space"
+             ", refusing to write! [freeSpace=%lld, limit=%u]", freeSpace,
+             limit));
+        return NS_ERROR_FILE_DISK_FULL;
+      }
+    }
+  }
+
   // This operation always invalidates the entry
   aHandle->mInvalid = true;
 
-  rv = TruncFile(aHandle->mFD, static_cast<uint32_t>(aTruncatePos));
+  rv = TruncFile(aHandle->mFD, aTruncatePos);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = TruncFile(aHandle->mFD, static_cast<uint32_t>(aEOFPos));
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (aTruncatePos != aEOFPos) {
+    rv = TruncFile(aHandle->mFD, aEOFPos);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  uint32_t oldSizeInK = aHandle->FileSizeInK();
+  aHandle->mFileSize = aEOFPos;
+  uint32_t newSizeInK = aHandle->FileSizeInK();
+
+  if (oldSizeInK != newSizeInK && !aHandle->IsDoomed() &&
+      !aHandle->IsSpecialFile()) {
+    CacheIndex::UpdateEntry(aHandle->Hash(), nullptr, nullptr, &newSizeInK);
+
+    if (oldSizeInK < newSizeInK) {
+      EvictIfOverLimitInternal();
+    }
+  }
 
   return NS_OK;
 }
 
 // static
 nsresult
 CacheFileIOManager::RenameFile(CacheFileHandle *aHandle,
                                const nsACString &aNewName,
--- a/netwerk/cache2/CacheFileIOManager.h
+++ b/netwerk/cache2/CacheFileIOManager.h
@@ -242,17 +242,17 @@ public:
 
   static nsresult OpenFile(const nsACString &aKey,
                            uint32_t aFlags, CacheFileIOListener *aCallback);
   static nsresult Read(CacheFileHandle *aHandle, int64_t aOffset,
                        char *aBuf, int32_t aCount,
                        CacheFileIOListener *aCallback);
   static nsresult Write(CacheFileHandle *aHandle, int64_t aOffset,
                         const char *aBuf, int32_t aCount, bool aValidate,
-                        CacheFileIOListener *aCallback);
+                        bool aTruncate, CacheFileIOListener *aCallback);
   static nsresult DoomFile(CacheFileHandle *aHandle,
                            CacheFileIOListener *aCallback);
   static nsresult DoomFileByKey(const nsACString &aKey,
                                 CacheFileIOListener *aCallback);
   static nsresult ReleaseNSPRHandle(CacheFileHandle *aHandle);
   static nsresult TruncateSeekSetEOF(CacheFileHandle *aHandle,
                                      int64_t aTruncatePos, int64_t aEOFPos,
                                      CacheFileIOListener *aCallback);
@@ -323,17 +323,18 @@ private:
                             CacheFileHandle **_retval);
   nsresult OpenSpecialFileInternal(const nsACString &aKey,
                                    uint32_t aFlags,
                                    CacheFileHandle **_retval);
   nsresult CloseHandleInternal(CacheFileHandle *aHandle);
   nsresult ReadInternal(CacheFileHandle *aHandle, int64_t aOffset,
                         char *aBuf, int32_t aCount);
   nsresult WriteInternal(CacheFileHandle *aHandle, int64_t aOffset,
-                         const char *aBuf, int32_t aCount, bool aValidate);
+                         const char *aBuf, int32_t aCount, bool aValidate,
+                         bool aTruncate);
   nsresult DoomFileInternal(CacheFileHandle *aHandle);
   nsresult DoomFileByKeyInternal(const SHA1Sum::Hash *aHash);
   nsresult ReleaseNSPRHandleInternal(CacheFileHandle *aHandle);
   nsresult TruncateSeekSetEOFInternal(CacheFileHandle *aHandle,
                                       int64_t aTruncatePos, int64_t aEOFPos);
   nsresult RenameFileInternal(CacheFileHandle *aHandle,
                               const nsACString &aNewName);
   nsresult EvictIfOverLimitInternal();
--- a/netwerk/cache2/CacheFileMetadata.cpp
+++ b/netwerk/cache2/CacheFileMetadata.cpp
@@ -280,17 +280,17 @@ CacheFileMetadata::WriteMetadata(uint32_
     mListener = aListener;
   } else {
     // We are not going to pass |this| as a callback so the buffer will be
     // released by CacheFileIOManager. Just null out mWriteBuf here.
     mWriteBuf = nullptr;
   }
 
   rv = CacheFileIOManager::Write(mHandle, aOffset, writeBuffer, p - writeBuffer,
-                                 true, aListener ? this : nullptr);
+                                 true, true, aListener ? this : nullptr);
   if (NS_FAILED(rv)) {
     LOG(("CacheFileMetadata::WriteMetadata() - CacheFileIOManager::Write() "
          "failed synchronously. [this=%p, rv=0x%08x]", this, rv));
 
     mListener = nullptr;
     if (mWriteBuf) {
       free(mWriteBuf);
       mWriteBuf = nullptr;
@@ -768,16 +768,21 @@ CacheFileMetadata::InitEmptyMetadata()
   }
   mOffset = 0;
   mMetaHdr.mVersion = kCacheEntryVersion;
   mMetaHdr.mFetchCount = 0;
   mMetaHdr.mExpirationTime = nsICacheEntry::NO_EXPIRATION_TIME;
   mMetaHdr.mKeySize = mKey.Length();
 
   DoMemoryReport(MemoryUsage());
+
+  // We're creating a new entry. If there is any old data truncate it.
+  if (mHandle && mHandle->FileExists() && mHandle->FileSize()) {
+    CacheFileIOManager::TruncateSeekSetEOF(mHandle, 0, 0, nullptr);
+  }
 }
 
 nsresult
 CacheFileMetadata::ParseMetadata(uint32_t aMetaOffset, uint32_t aBufOffset,
                                  bool aHaveKey)
 {
   LOG(("CacheFileMetadata::ParseMetadata() [this=%p, metaOffset=%d, "
        "bufOffset=%d, haveKey=%u]", this, aMetaOffset, aBufOffset, aHaveKey));
--- a/netwerk/cache2/CacheIndex.cpp
+++ b/netwerk/cache2/CacheIndex.cpp
@@ -1665,17 +1665,17 @@ CacheIndex::WriteRecords()
 
     NetworkEndian::writeUint32(mRWBuf + mRWBufPos, mRWHash->GetHash());
     mRWBufPos += sizeof(CacheHash::Hash32_t);
   } else {
     MOZ_ASSERT(data.mHasMore);
   }
 
   rv = CacheFileIOManager::Write(mIndexHandle, fileOffset, mRWBuf, mRWBufPos,
-                                 mSkipEntries == mProcessEntries, this);
+                                 mSkipEntries == mProcessEntries, false, this);
   if (NS_FAILED(rv)) {
     LOG(("CacheIndex::WriteRecords() - CacheFileIOManager::Write() failed "
          "synchronously [rv=0x%08x]", rv));
     FinishWrite(false);
   }
 
   mRWBufPos = 0;
 }
@@ -2126,17 +2126,18 @@ CacheIndex::ParseRecords()
     } else {
       NetworkEndian::writeUint32(&hdr->mIsDirty, 1);
 
       // Mark index dirty. The buffer is freed by CacheFileIOManager when
       // nullptr is passed as the listener and the call doesn't fail
       // synchronously.
       rv = CacheFileIOManager::Write(mIndexHandle, 0,
                                      reinterpret_cast<char *>(hdr),
-                                     sizeof(CacheIndexHeader), true, nullptr);
+                                     sizeof(CacheIndexHeader), true, false,
+                                     nullptr);
       if (NS_FAILED(rv)) {
         // This is not fatal, just free the memory
         free(hdr);
       }
     }
 
     pos += sizeof(CacheIndexHeader);
   }