Bug 436647 - Overwriting a gpg-encrypted file results in a zero byte file
Summary: Overwriting a gpg-encrypted file results in a zero byte file
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: file (show other bugs)
Version: 5.1.1
Platform: Microsoft Windows Microsoft Windows
: NOR major
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL:
Keywords:
Depends on:
Blocks: 426400
  Show dependency treegraph
 
Reported: 2021-05-05 17:27 UTC by Ralf Habacker
Modified: 2021-05-12 06:54 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 5.1.2
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Habacker 2021-05-05 17:27:44 UTC
STEPS TO REPRODUCE
1. make sure that there is a matching gpg key for the KMyMoney file you are using
2. download portable kmymoney package from https://kmymoney.org/snapshots.php
3. unpack portable archive, start KMyMoney and load encrypted KMyMoney file
4. overwrite the used file with File->"save as" (make a backup copy before)

OBSERVED RESULT
The created file has a length of 0 bytes and is not loadable anymore.

EXPECTED RESULT
The created file should be created completely

SOFTWARE/OS VERSIONS
Windows: 10
KDE Frameworks Version: 5.65.0
Qt Version: 5.11

ADDITIONAL INFORMATION
In explorer a file with a corresponding file name and an additional extension .1~ is displayed, which corresponds to the saved state. This suggests that the final renaming/copying failed.
Comment 1 Ralf Habacker 2021-05-09 14:41:25 UTC
An investigation shows that there is a problem when QTemporaryFile and QSaveFile are used together on Windows.

For the case of creating a local gpg-encrypted file, only one KSaveFile instance inside KMyMoneyView::saveToLocalFile() https://github.com/KDE/kmymoney/blob/07d368edf5098cb06774a1cecc41551ce3ad43c5/kmymoney/views/kmymoneyview.cpp#L1151 was used in the 4.8 branch, which also works as expected for the case that the file to be created already existed before.

In the 5.1 branch the procedure was changed so that in the mentioned function first a QTemporaryFile instance is created and if the resulting file already exists, the file name created in this way is passed to the KGPGFile instance, which internally uses a QSaveFile instance, which in turn creates a temporary file.

Something goes wrong with this multiple renaming or copying. If you omit the superfluous use of the QTemporaryFile instance in the case mentioned, it works as expected.
Comment 2 Thomas Baumgart 2021-05-12 06:33:45 UTC
Git commit 39785a3d80c56dc6e53f301d76c41bcc8ca048c6 by Thomas Baumgart.
Committed on 12/05/2021 at 06:33.
Pushed by tbaumgart into branch '5.1'.

Solve problems with usage of QTemporaryFile on MS-Windows

A call to QTemporaryFile::close() may not close the file but only rewind
the file pointer to the beginning allwing faster re-open. This causes
problems on MS-Windows filesystems, as one cannot e.g. rename the
temporary file in this case because it is still kept open from the
perspective of the filesystem. Only destroying the object really closes
the file on the filesystem.

This change makes sure, that the QTemporaryFile object only lives as
long as it is needed and closed immediately after by destroying the
object.
FIXED-IN: 5.1.2

M  +18   -10   kmymoney/plugins/xml/xmlstorage.cpp

https://invent.kde.org/office/kmymoney/commit/39785a3d80c56dc6e53f301d76c41bcc8ca048c6
Comment 3 Thomas Baumgart 2021-05-12 06:54:29 UTC
Git commit 646c4cb810a960699b1b659d72bef248992f4e40 by Thomas Baumgart.
Committed on 12/05/2021 at 06:54.
Pushed by tbaumgart into branch 'master'.

Solve problems with usage of QTemporaryFile on MS-Windows

A call to QTemporaryFile::close() may not close the file but only rewind
the file pointer to the beginning allwing faster re-open. This causes
problems on MS-Windows filesystems, as one cannot e.g. rename the
temporary file in this case because it is still kept open from the
perspective of the filesystem. Only destroying the object really closes
the file on the filesystem.

This change makes sure, that the QTemporaryFile object only lives as
long as it is needed and closed immediately after by destroying the
object.
FIXED-IN: 5.1.2
(cherry picked from commit 39785a3d80c56dc6e53f301d76c41bcc8ca048c6)

M  +18   -10   kmymoney/plugins/xml/xmlstorage.cpp

https://invent.kde.org/office/kmymoney/commit/646c4cb810a960699b1b659d72bef248992f4e40