Make an encrypted export importable again - #735
Merged
Conversation
An encrypted export could not be restored. Not "failed with an error" — it died on a PHP fatal, for any export containing so much as one category, which is every real backup anybody has taken. The decrypted fragment was parsed into a document that had not been told to drop whitespace, unlike the one XmlFile builds for the file itself, and the export is written pretty-printed. So the indentation arrived as text nodes and was grafted into the main tree; the first one reached a callback typed for elements, and the TypeError that raised is an Error, which neither the importer's own catch nor the transaction's catches. One line, matching what the unencrypted path has always done. Comes with the test that found it: an account, its category, client and tags are created through the real services, exported, imported back, and every field — including the decrypted password — is required to match. Plain and encrypted both, and a wrong password still refused.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
An encrypted export could not be restored. Not "failed with an error" — it died on an uncaught PHP fatal, for any export containing so much as one category, client, tag or account. That is every real backup anybody has taken.
SyspassImport::processEncrypted()parsed the decrypted fragment into aDOMDocumentwithout settingpreserveWhiteSpace = false— unlikeXmlFile, which does set it for the file itself.XmlExportwrites withformatOutput = true, so the fragment is pretty-printed: the indentation arrives asDOMTextnodes and is grafted into the main tree byimportNode(…, true).The first one then reaches a
CallbackFilterIteratorcallback typedDOMElement $element, and theTypeErrorthat raises is anError, not anException— so neitherSyspassImport::doImport()'scatch (Exception)nor the repository transaction's catches it.Reproduced standalone before touching anything:
The fix
One line: the same
preserveWhiteSpace = falsethe unencrypted path has always had.The test that found it
A new export→import round trip, which is the property neither half's tests covered: both were tested against themselves, so nothing said what the export writes is what the import can read.
It creates a category, a client, two tags and an account with a real password through the real services, exports, imports back, and requires every field to match — name, login, url, notes, category and client by name, tags, and the decrypted password. Not "no exception was thrown".
Three cases: plain, encrypted with the right password, and encrypted with the wrong one (still refused cleanly). The encrypted case fails against the old code with exactly the
TypeErrorabove.It builds a real container against a real database by hand, the way
CliTestCasedoes, becauseIntegrationTestCasemocks the database away and nothing would actually persist to read back.Testing
Integration suite green: 855. Import unit tests: 54. PHPStan and PHPCS clean.