From c852a5bda36ab5190db3fc6abf00bdf0d6f52385 Mon Sep 17 00:00:00 2001
From: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com>
Date: Sun, 19 Sep 2021 13:55:54 +0200
Subject: [PATCH] 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 | 26 +++++++++-
novelwriter/core/document.py | 19 ++++++--
novelwriter/gui/doceditor.py | 20 ++++++--
novelwriter/guimain.py | 1 +
tests/test_base/test_base_common.py | 68 ++++++++++++++++++++++-----
tests/test_core/test_core_document.py | 35 ++++++++++----
6 files changed, 140 insertions(+), 29 deletions(-)
diff --git a/novelwriter/common.py b/novelwriter/common.py
index f3b91c32..d4eb5a83 100644
--- a/novelwriter/common.py
+++ b/novelwriter/common.py
@@ -24,6 +24,7 @@ along with this program. If not, see .
"""
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.
"""
diff --git a/novelwriter/core/document.py b/novelwriter/core/document.py
index 4726a8fb..0dee4ecf 100644
--- a/novelwriter/core/document.py
+++ b/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):
diff --git a/novelwriter/gui/doceditor.py b/novelwriter/gui/doceditor.py
index c156f5bc..39605f0a 100644
--- a/novelwriter/gui/doceditor.py
+++ b/novelwriter/gui/doceditor.py
@@ -453,9 +453,23 @@ class GuiDocEditor(QTextEdit):
self.saveCursorPosition()
if not self._nwDocument.writeDocument(docText):
- self.theParent.makeAlert([
- self.tr("Could not save document."), self._nwDocument.getError()
- ], nwAlert.ERROR)
+ saveOk = False
+ if self._nwDocument._currHash != self._nwDocument._prevHash:
+ 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)
diff --git a/novelwriter/guimain.py b/novelwriter/guimain.py
index 2dd45d80..a2f30297 100644
--- a/novelwriter/guimain.py
+++ b/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 = "
".join(theMessage)
logMsg = theMessage
else:
diff --git a/tests/test_base/test_base_common.py b/tests/test_base/test_base_common.py
index c8a64ba2..087cd5c1 100644
--- a/tests/test_base/test_base_common.py
+++ b/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 .
"""
+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.
diff --git a/tests/test_core/test_core_document.py b/tests/test_core/test_core_document.py
index 448c13e7..dd72b654 100644
--- a/tests/test_core/test_core_document.py
+++ b/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