Fix temp file handling

IB-8938

Signed-off-by: Raul Metsma <raul@metsma.ee>

Author: metsma

src/Container.cpp

@@ -163,7 +163,6 @@ void digidoc::terminate()
 {
     try {
         Conf::init(nullptr);
-        util::File::deleteTempFiles();
     } catch (...) {
         // Don't throw on terminate
     }

src/DataFile.cpp

@@ -25,6 +25,7 @@
 #include "util/ZipSerialize.h"
 
 #include <array>
+#include <filesystem>
 #include <fstream>
 #include <sstream>
 
@@ -106,25 +107,48 @@ DataFilePrivate::DataFilePrivate(const ZipSerialize &z, string filename, string
     d->size.emplace((unsigned long)r.size);
     if(r.size > MAX_MEM_FILE)
     {
-        auto fs = make_unique<fstream>(util::File::tempFileName(), fstream::in|fstream::out|fstream::binary|fstream::trunc);
-        if(!fs->is_open())
-            THROW("Failed to open destination file");
-        array<char,10240> buf{};
-        for(size_t size = 0, currentStreamSize = 0;
-             (size = r(buf.data(), buf.size())) > 0; currentStreamSize += size)
-        {
-            if(currentStreamSize + size > r.size)
-                THROW("ZIP entry actual size exceeds uncompressed_size %zu", r.size);
-            if(!fs->write(buf.data(), size))
-                THROW("Failed to write '%s' data to stream. Stream size: %d", m_filename.c_str(), currentStreamSize);
+        try {
+            m_tempFile = util::File::tempFileName();
+            auto fs = make_unique<fstream>(m_tempFile, fstream::in|fstream::out|fstream::binary|fstream::trunc);
+            if(!fs->is_open())
+                THROW("Failed to open destination file");
+            array<char,10240> buf{};
+            for(size_t size = 0, currentStreamSize = 0;
+                (size = r(buf.data(), buf.size())) > 0; currentStreamSize += size)
+            {
+                if(currentStreamSize + size > r.size)
+                    THROW("ZIP entry actual size exceeds uncompressed_size %zu", r.size);
+                if(!fs->write(buf.data(), size))
+                    THROW("Failed to write '%s' data to stream. Stream size: %d", m_filename.c_str(), currentStreamSize);
+            }
+            if(!fs->flush())
+                THROW("Failed to flush '%s' data to temporary file.", m_filename.c_str());
+            fs->clear();
+            if(!fs->seekg(0, istream::beg))
+                THROW("Failed to rewind '%s' temporary file.", m_filename.c_str());
+            m_is = std::move(fs);
+        } catch(...) {
+            if(!m_tempFile.empty())
+            {
+                error_code ec;
+                filesystem::remove(m_tempFile, ec);
+            }
+            throw;
         }
-        m_is = std::move(fs);
     }
     else
         m_is = make_unique<stringstream>(r(MAX_MEM_FILE));
 }
 
-DataFilePrivate::~DataFilePrivate() noexcept = default;
+DataFilePrivate::~DataFilePrivate() noexcept
+{
+    m_is.reset();
+    if(!m_tempFile.empty())
+    {
+        error_code ec;
+        filesystem::remove(m_tempFile, ec);
+    }
+}
 
 void DataFilePrivate::digest(const Digest &digest) const
 {

src/DataFile_p.h

@@ -21,6 +21,7 @@
 
 #include "DataFile.h"
 
+#include <filesystem>
 #include <istream>
 #include <memory>
 #include <optional>
@@ -52,6 +53,7 @@ class DataFilePrivate final: public DataFile
 
     struct Private;
     std::unique_ptr<Private> d;
+    std::filesystem::path m_tempFile;
     std::unique_ptr<std::istream> m_is;
     std::string m_id, m_filename, m_mediatype;
 };

src/util/File.cpp

@@ -62,7 +62,6 @@ using f_statbuf = struct stat;
 using f_utimbuf = struct utimbuf;
 #endif
 
-stack<fs::path> File::tempFiles;
 
 static string decodeName(fs::path path)
 {
@@ -224,20 +223,23 @@ string File::path(string dir, string_view relativePath)
  */
 fs::path File::tempFileName()
 {
+    fs::path result;
 #ifdef _WIN32
     // requires TMP environment variable to be set
     wchar_t *fileName = _wtempnam(nullptr, nullptr); // TODO: static buffer, not thread-safe
     if(!fileName)
         THROW("Failed to create a temporary file name.");
-    tempFiles.emplace(fileName);
+    result = fileName;
     free(fileName);
 #else
-    string tmp = "XXXXXX";
-    if(mkstemp(tmp.data()) == -1)
+    auto result_str = (fs::temp_directory_path() / "XXXXXX").string();
+    int fd = mkstemp(result_str.data());
+    if(fd == -1)
         THROW("Failed to create a temporary file name.");
-    tempFiles.push(fs::temp_directory_path() / tmp);
+    ::close(fd);
+    result = result_str;
 #endif
-    return tempFiles.top();
+    return result;
 }
 
 /**
@@ -292,21 +294,6 @@ string File::digidocppPath()
 #endif
 }
 
-/**
- * Tries to delete all temporary files and directories whose names were handled out with tempFileName, tempDirectory and createTempDirectory.
- * The deletion of directories is recursive.
- */
-void File::deleteTempFiles()
-{
-    error_code ec;
-    while(!tempFiles.empty())
-    {
-        if(!fs::remove(tempFiles.top(), ec) || ec)
-            WARN("Tried to remove the temporary file or directory '%s', but failed.", decodeName(tempFiles.top()).c_str());
-        tempFiles.pop();
-    }
-}
-
 /**
  * Helper method for converting strings with non-ascii characters to the URI format (%HH for each non-ascii character).
  *

src/util/File.h

@@ -22,7 +22,6 @@
 #include "../Exception.h"
 
 #include <filesystem>
-#include <stack>
 
 namespace digidoc
 {
@@ -47,7 +46,6 @@ namespace digidoc
               static std::string path(std::string dir, std::string_view relativePath);
               static std::filesystem::path tempFileName();
               static void createDirectory(std::string path);
-              static void deleteTempFiles();
               static std::string toUriPath(const std::string &path);
               static std::string fromUriPath(std::string_view path);
               static std::vector<unsigned char> hexToBin(std::string_view in);
@@ -59,7 +57,6 @@ namespace digidoc
 #ifdef __APPLE__
               static std::string frameworkResourcesPath(std::string_view name);
 #endif
-              static std::stack<std::filesystem::path> tempFiles;
         };
 
     }