Add hash check on document read/write (#890)

* Add sha256sum function
* Add hash check to document class
* Make sha256sum test more thorough
* Handle exceptions in the sha256sum function directly
* Update test coverage
* Clarify title on dialog box
* Don't write blank lines in makeAlert

novelwriter/common.py

@@ -24,6 +24,7 @@ along with this program. If not, see <https://www.gnu.org/licenses/>.
 """
 
 import json
+import hashlib
 import logging
 
 from datetime import datetime
@@ -411,7 +412,7 @@ def jsonEncode(data, n=0, nmax=0):
 
 
 # =============================================================================================== #
-#  Other Functions
+#  File and File System Functions
 # =============================================================================================== #
 
 def makeFileNameSafe(theText):
@@ -424,6 +425,29 @@ def makeFileNameSafe(theText):
     return cleanName
 
 
+def sha256sum(filePath):
+    """Make a shasum of a file using a buffer.
+    Based on: https://stackoverflow.com/a/44873382/5825851
+    """
+    hDigest = hashlib.sha256()
+    bData = bytearray(65536)
+    mData = memoryview(bData)
+    try:
+        with open(filePath, mode="rb", buffering=0) as inFile:
+            for n in iter(lambda: inFile.readinto(mData), 0):
+                hDigest.update(mData[:n])
+    except Exception:
+        logger.error("Could not read sha256sum of: %s", filePath)
+        logException()
+        return None
+
+    return hDigest.hexdigest()
+
+
+# =============================================================================================== #
+#  Other Functions
+# =============================================================================================== #
+
 def getGuiItem(theName):
     """Returns a QtWidget based on its objectName.
     """

novelwriter/core/document.py

@@ -27,7 +27,7 @@ import os
 import logging
 
 from novelwriter.enum import nwItemLayout, nwItemClass
-from novelwriter.common import isHandle
+from novelwriter.common import isHandle, sha256sum
 
 logger = logging.getLogger(__name__)
 
@@ -44,6 +44,8 @@ class NWDoc():
         self._fileLoc   = None  # The file location of the currently open item
         self._docMeta   = {}    # The meta data of the currently open item
         self._docError  = ""    # The latest encountered IO error
+        self._prevHash  = None  # Previous sha256sum of the document file
+        self._currHash  = None  # Latest sha256sum of the document file
 
         if isHandle(theHandle):
             self._docHandle = theHandle
@@ -80,6 +82,8 @@ class NWDoc():
 
         theText = ""
         self._docMeta = {}
+        self._prevHash = sha256sum(docPath)
+
         if os.path.isfile(docPath):
             try:
                 with open(docPath, mode="r", encoding="utf-8") as inFile:
@@ -108,7 +112,7 @@ class NWDoc():
 
         return theText
 
-    def writeDocument(self, docText):
+    def writeDocument(self, docText, forceWrite=False):
         """Write the document. The file is saved via a temp file in case
         of save failure. Returns True if successful, False if not.
         """
@@ -125,7 +129,13 @@ class NWDoc():
         docPath = os.path.join(self.theProject.projContent, docFile)
         docTemp = os.path.join(self.theProject.projContent, docFile+"~")
 
-        # DocMeta line
+        if self._prevHash is not None and not forceWrite:
+            self._currHash = sha256sum(docPath)
+            if self._currHash is not None and self._currHash != self._prevHash:
+                logger.error("File has been altered on disk since opened")
+                return False
+
+        # DocMeta Line
         if self._theItem is None:
             docMeta = ""
         else:
@@ -149,6 +159,9 @@ class NWDoc():
             os.unlink(docPath)
         os.rename(docTemp, docPath)
 
+        self._prevHash = sha256sum(docPath)
+        self._currHash = self._prevHash
+
         return True
 
     def deleteDocument(self):

novelwriter/gui/doceditor.py

@@ -453,9 +453,23 @@ class GuiDocEditor(QTextEdit):
 
         self.saveCursorPosition()
         if not self._nwDocument.writeDocument(docText):
-            self.theParent.makeAlert([
+            saveOk = False
-                self.tr("Could not save document."), self._nwDocument.getError()
+            if self._nwDocument._currHash != self._nwDocument._prevHash:
-            ], nwAlert.ERROR)
+                msgYes = self.theParent.askQuestion(
+                    self.tr("File Changed on Disk"),
+                    self.tr(
+                        "This document has been changed outside of novelWriter "
+                        "while it was open. Overvrite the file on disk?"
+                    )
+                )
+                if msgYes:
+                    saveOk = self._nwDocument.writeDocument(docText, forceWrite=True)
+
+            if not saveOk:
+                self.theParent.makeAlert([
+                    self.tr("Could not save document."), self._nwDocument.getError()
+                ], nwAlert.ERROR)
+
             return False
 
         self.setDocumentChanged(False)

novelwriter/guimain.py

@@ -1121,6 +1121,7 @@ class GuiMain(QMainWindow):
         can be either a string or an array of strings.
         """
         if isinstance(theMessage, list):
+            theMessage = list(filter(None, theMessage))
             popMsg = "<br>".join(theMessage)
             logMsg = theMessage
         else:

tests/test_base/test_base_common.py

@@ -19,12 +19,14 @@ You should have received a copy of the GNU General Public License
 along with this program. If not, see <https://www.gnu.org/licenses/>.
 """
 
+import hashlib
 import os
 import time
 import pytest
 
 from datetime import datetime
 
+from mock import causeOSError
 from tools import writeFile
 
 from novelwriter.common import (
@@ -32,7 +34,7 @@ from novelwriter.common import (
     isItemClass, isItemType, isItemLayout, hexToInt, formatInt,
     formatTimeStamp, formatTime, parseTimeStamp, splitVersionNumber,
     transferCase, fuzzyTime, numberToRoman, jsonEncode, makeFileNameSafe,
-    NWConfigParser
+    sha256sum, NWConfigParser
 )
 
 
@@ -342,18 +344,6 @@ def testBaseCommon_FuzzyTime():
 # END Test testBaseCommon_FuzzyTime
 
 
-@pytest.mark.base
-def testBaseCommon_MakeFileNameSafe():
-    """Test the fuzzyTime function.
-    """
-    assert makeFileNameSafe(" aaaa ") == "aaaa"
-    assert makeFileNameSafe("aaaa,bbbb") == "aaaabbbb"
-    assert makeFileNameSafe("aaaa\tbbbb") == "aaaabbbb"
-    assert makeFileNameSafe("aaaa bbbb") == "aaaa bbbb"
-
-# END Test testBaseCommon_MakeFileNameSafe
-
-
 @pytest.mark.core
 def testBaseCommon_RomanNumbers():
     """Test conversion of integers to Roman numbers.
@@ -466,6 +456,58 @@ def testBaseCommon_JsonEncode():
 # END Test testBaseCommon_JsonEncode
 
 
+@pytest.mark.base
+def testBaseCommon_MakeFileNameSafe():
+    """Test the makeFileNameSafe function.
+    """
+    assert makeFileNameSafe(" aaaa ") == "aaaa"
+    assert makeFileNameSafe("aaaa,bbbb") == "aaaabbbb"
+    assert makeFileNameSafe("aaaa\tbbbb") == "aaaabbbb"
+    assert makeFileNameSafe("aaaa bbbb") == "aaaa bbbb"
+
+# END Test testBaseCommon_MakeFileNameSafe
+
+
+@pytest.mark.base
+def testBaseCommon_Sha256Sum(monkeypatch, fncDir, ipsumText):
+    """Test the sha256sum function.
+    """
+    longText = 50*(" ".join(ipsumText) + " ")
+    shortText = "This is a short file"
+    noneText = ""
+
+    assert len(longText) == 175650
+
+    longFile = os.path.join(fncDir, "long_file.txt")
+    shortFile = os.path.join(fncDir, "short_file.txt")
+    noneFile = os.path.join(fncDir, "none_file.txt")
+
+    writeFile(longFile, longText)
+    writeFile(shortFile, shortText)
+    writeFile(noneFile, noneText)
+
+    # Taken with sha256sum command on command line
+    longHash = "9b22aee35660da4fae204acbe96aec7f563022746ca2b7a3831f5e44544765eb"
+    shortHash = "6d7c9b2722364c471b8a8666bcb35d18500272d05b23b3427288e2e34c6618f0"
+    noneHash = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+
+    assert sha256sum(longFile) == longHash
+    assert sha256sum(shortFile) == shortHash
+    assert sha256sum(noneFile) == noneHash
+
+    assert hashlib.sha256(longText.encode("utf-8")).hexdigest() == longHash
+    assert hashlib.sha256(shortText.encode("utf-8")).hexdigest() == shortHash
+    assert hashlib.sha256(noneText.encode("utf-8")).hexdigest() == noneHash
+
+    with monkeypatch.context() as mp:
+        mp.setattr("builtins.open", causeOSError)
+        assert sha256sum(longFile) is None
+        assert sha256sum(shortFile) is None
+        assert sha256sum(noneFile) is None
+
+# END Test testBaseCommon_Sha256Sum
+
+
 @pytest.mark.base
 def testBaseCommon_NWConfigParser(fncDir):
     """Test the NWConfigParser subclass.

tests/test_core/test_core_document.py

@@ -23,7 +23,7 @@ import os
 import pytest
 
 from mock import causeOSError
-from tools import readFile
+from tools import readFile, writeFile
 
 from novelwriter.core import NWProject, NWDoc
 from novelwriter.enum import nwItemClass, nwItemLayout
@@ -34,11 +34,14 @@ def testCoreDocument_LoadSave(monkeypatch, mockGUI, nwMinimal):
     """Test loading and saving a document with the NWDoc class.
     """
     theProject = NWProject(mockGUI)
-    assert theProject.openProject(nwMinimal)
+    assert theProject.openProject(nwMinimal) is True
     assert theProject.projPath == nwMinimal
 
     sHandle = "8c659a11cd429"
 
+    # Read Document
+    # =============
+
     # Not a valid handle
     theDoc = NWDoc(theProject, "stuff")
     assert theDoc.readDocument() is None
@@ -46,6 +49,7 @@ def testCoreDocument_LoadSave(monkeypatch, mockGUI, nwMinimal):
     # Non-existent handle
     theDoc = NWDoc(theProject, "0000000000000")
     assert theDoc.readDocument() is None
+    assert theDoc._currHash is None
 
     # Cause open() to fail while loading
     with monkeypatch.context() as mp:
@@ -65,11 +69,14 @@ def testCoreDocument_LoadSave(monkeypatch, mockGUI, nwMinimal):
     theDoc = NWDoc(theProject, xHandle)
     assert theDoc.readDocument() == ""
 
+    # Write Document
+    # ==============
+
     # Set handle and save again
     theText = "### Test File\n\nText ...\n\n"
     theDoc = NWDoc(theProject, xHandle)
     assert theDoc.readDocument(xHandle) == ""
-    assert theDoc.writeDocument(theText)
+    assert theDoc.writeDocument(theText) is True
 
     # Save again to ensure temp file and previous file is handled
     assert theDoc.writeDocument(theText)
@@ -84,36 +91,46 @@ def testCoreDocument_LoadSave(monkeypatch, mockGUI, nwMinimal):
         "Text ...\n\n"
     )
 
+    # Alter the document on disk and save again
+    writeFile(docPath, "blablabla")
+    assert theDoc.writeDocument(theText) is False
+
+    # Force the overwrite
+    assert theDoc.writeDocument(theText, forceWrite=True) is True
+
     # Force no meta data
     theDoc._theItem = None
-    assert theDoc.writeDocument(theText)
+    assert theDoc.writeDocument(theText) is True
     assert readFile(docPath) == theText
 
     # Cause open() to fail while saving
     with monkeypatch.context() as mp:
         mp.setattr("builtins.open", causeOSError)
-        assert not theDoc.writeDocument(theText)
+        assert theDoc.writeDocument(theText) is False
         assert theDoc.getError() == "OSError"
 
     # Saving with no handle
     theDoc._docHandle = None
-    assert not theDoc.writeDocument(theText)
+    assert theDoc.writeDocument(theText) is False
+
+    # Delete Document
+    # ===============
 
     # Delete the last document
     theDoc = NWDoc(theProject, "stuff")
-    assert not theDoc.deleteDocument()
+    assert theDoc.deleteDocument() is False
     assert os.path.isfile(docPath)
 
     # Cause the delete to fail
     with monkeypatch.context() as mp:
         mp.setattr("os.unlink", causeOSError)
         theDoc = NWDoc(theProject, xHandle)
-        assert not theDoc.deleteDocument()
+        assert theDoc.deleteDocument() is False
         assert theDoc.getError() == "OSError"
 
     # Make the delete pass
     theDoc = NWDoc(theProject, xHandle)
-    assert theDoc.deleteDocument()
+    assert theDoc.deleteDocument() is True
     assert not os.path.isfile(docPath)
 
 # END Test testCoreDocument_Load