Skip to content

fix: never leave plaintext when encryption fails - #1171

Open
nkuprins wants to merge 1 commit into
apache:mainfrom
nkuprins:fix/encryption-failure-plaintext
Open

nkuprins wants to merge 1 commit into
apache:mainfrom
nkuprins:fix/encryption-failure-plaintext

Conversation

@nkuprins

Copy link
Copy Markdown
Contributor

Closed: #1135

Purpose of the pull request

As title
Related: #1168 fixes the separate temp-file leak on the same path (#1137).

What's changed?

  • OutputStream: when doOutputStreamEncrypt07() fails, finish() sets writeExcel = false, so the unencrypted workbook is no longer written to the caller's stream.
  • File: when doFileEncrypt07() fails, finish() deletes the target file.
  • If deleting the target file fails, the reported error is "Can not delete unencrypted file: ", with the encryption error as its cause. It is recorded like the other failures in finish() rather than thrown on the spot, so the remaining cleanup still runs.

Tests

  • xlsxStreamPasswordWrite_encryptionFails_writesNothing: the temp directory path runs through a regular file, so it cannot be created. Asserts the caller's stream is empty; on main it holds the full unencrypted workbook.
  • xlsxFilePasswordWrite_encryptionFails_leavesNoFile: the target is made read-only after its output stream is open, so encrypting it in place fails. Asserts the file no longer exists; on main it is left as a plain xlsx. Skipped where file permissions are not enforced (root).

This conflicts with #1168 only at the end of EncryptDataTest, where both add tests. I'll rebase whichever lands second; after #1168 is merged, the stream test here can use its writeWithPassword helper.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] xlsx write with password() leaves an unencrypted workbook when encryption fails

1 participant