[cpp-xmltooling] branch master updated: CPPXT-134 - Reloadable configuration deleting backing file on a 304

[Scott Cantor]

This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch master
in repository cpp-xmltooling.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-xmltooling.git;a=commit;h=bc105b8ad5f1ab1bf26dda5a904dcc85ca32e097

The following commit(s) were added to refs/heads/master by this push:
       new  bc105b8   CPPXT-134 - Reloadable configuration deleting backing file on a 304
bc105b8 is described below

commit bc105b8ad5f1ab1bf26dda5a904dcc85ca32e097
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Jul 17 21:35:55 2018 -0400

    CPPXT-134 - Reloadable configuration deleting backing file on a 304
    
    https://issues.shibboleth.net/jira/browse/CPPXT-134
---
 xmltooling/util/ReloadableXMLFile.cpp | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/xmltooling/util/ReloadableXMLFile.cpp b/xmltooling/util/ReloadableXMLFile.cpp
index 679319c..6e496f4 100644
--- a/xmltooling/util/ReloadableXMLFile.cpp
+++ b/xmltooling/util/ReloadableXMLFile.cpp
@@ -52,6 +52,8 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#include <boost/lexical_cast.hpp>
+
 #include <xercesc/framework/LocalFileInputSource.hpp>
 #include <xercesc/framework/Wrapper4InputSource.hpp>
 #include <xercesc/util/XMLUniDefs.hpp>
@@ -506,19 +508,33 @@ pair<bool,DOMElement*> ReloadableXMLFile::load()
 {
     // If this method is used, we're responsible for managing failover to a
     // backup of a remote resource (if available), and for backing up remote
-    // resources.
+    // resources by "committing" the temporary copy to the backup location.
+    // The changes to the backup process mean that the parsing step itself
+    // actually creates the backup copy.
+
+    // Note that this is still not robust. We don't know whether the XML
+    // is fit for purpose because we don't process it until after this step
+    // finishes, which means any well-formed and/or valid XML can overwrite the
+    // backup and only afterward be flagged as unusable.
+
+    // Generate a simple random suffix for the temp file, anything will do.
+    string backupKey = m_backing;
+    if (!backupKey.empty())
+        backupKey += '.' + boost::lexical_cast<string>(rand());
+
     try {
-        pair<bool,DOMElement*> ret = load(false, m_backing);
-        if (!m_backing.empty()) {
-            m_log.debug("backing up remote resource to (%s)", m_backing.c_str());
+        pair<bool,DOMElement*> ret = load(false, backupKey);
+        if (!backupKey.empty()) {
+            m_log.debug("committing backup file to permanent location (%s)", m_backing.c_str());
             try {
                 Locker locker(getBackupLock());
-                ofstream backer(m_backing.c_str());
-                backer << *(ret.second->getOwnerDocument());
+                remove(m_backing.c_str());
+                if (rename(backupKey.c_str(), m_backing.c_str()) != 0)
+                    m_log.crit("unable to rename backup file");
                 preserveCacheTag();
             }
-            catch (const exception& ex) {
-                m_log.crit("exception while backing up resource: %s", ex.what());
+            catch (const std::exception& ex) {
+                m_log.crit("exception while committing backup file: %s", ex.what());
             }
         }
         return ret;

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.