From dd45e6db683f50b9d8b1b18c2cfe50a4f9a38feb Mon Sep 17 00:00:00 2001
From: David Kalnischkies <david@kalnischkies.de>
Date: Wed, 29 Jun 2016 14:46:34 +0200
Subject: don't do atomic overrides with failed files
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We deploy atomic renames for some files, but these renames also happen
if something about the file failed which isn't really the point of the
exercise…

Closes: 828908
(cherry picked from commit fc5db01bb7d1546944200d197866b0b5c378f100)
---
 apt-pkg/contrib/fileutl.cc  |  2 +-
 test/libapt/fileutl_test.cc | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index 68298d785..09dce4123 100644
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -2619,7 +2619,7 @@ bool FileFd::Close()
    }
 
    if ((Flags & Replace) == Replace) {
-      if (rename(TemporaryFileName.c_str(), FileName.c_str()) != 0)
+      if (Failed() == false && rename(TemporaryFileName.c_str(), FileName.c_str()) != 0)
 	 Res &= _error->Errno("rename",_("Problem renaming the file %s to %s"), TemporaryFileName.c_str(), FileName.c_str());
 
       FileName = TemporaryFileName; // for the unlink() below.
diff --git a/test/libapt/fileutl_test.cc b/test/libapt/fileutl_test.cc
index 607c4a195..ff13e9e92 100644
--- a/test/libapt/fileutl_test.cc
+++ b/test/libapt/fileutl_test.cc
@@ -317,6 +317,7 @@ TEST(FileUtlTest, flAbsPath)
 
 static void TestDevNullFileFd(unsigned int const filemode)
 {
+   SCOPED_TRACE(filemode);
    FileFd f("/dev/null", filemode);
    EXPECT_FALSE(f.Failed());
    EXPECT_TRUE(f.IsOpen());
@@ -344,3 +345,37 @@ TEST(FileUtlTest, WorkingWithDevNull)
    TestDevNullFileFd(FileFd::WriteTemp);
    TestDevNullFileFd(FileFd::WriteAtomic);
 }
+constexpr char const * const TESTSTRING = "This is a test";
+static void TestFailingAtomicKeepsFile(char const * const label, std::string const &filename)
+{
+   SCOPED_TRACE(label);
+   EXPECT_TRUE(FileExists(filename));
+   FileFd fd;
+   EXPECT_TRUE(fd.Open(filename, FileFd::ReadOnly));
+   char buffer[50];
+   EXPECT_NE(nullptr, fd.ReadLine(buffer, sizeof(buffer)));
+   EXPECT_STREQ(TESTSTRING, buffer);
+}
+TEST(FileUtlTest, FailingAtomic)
+{
+   FileFd fd;
+   std::string filename;
+   createTemporaryFile("failingatomic", fd, &filename, TESTSTRING);
+   TestFailingAtomicKeepsFile("init", filename);
+
+   FileFd f;
+   EXPECT_TRUE(f.Open(filename, FileFd::ReadWrite | FileFd::Atomic));
+   f.EraseOnFailure();
+   EXPECT_FALSE(f.Failed());
+   EXPECT_TRUE(f.IsOpen());
+   TestFailingAtomicKeepsFile("before-fail", filename);
+   EXPECT_TRUE(f.Write("Bad file write", 10));
+   f.OpFail();
+   EXPECT_TRUE(f.Failed());
+   TestFailingAtomicKeepsFile("after-fail", filename);
+   EXPECT_TRUE(f.Close());
+   TestFailingAtomicKeepsFile("closed", filename);
+
+   if (filename.empty() == false)
+      unlink(filename.c_str());
+}
-- 
cgit v1.2.3