Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix downloading encrypted files to a temporary location #835

Conversation

vkrause
Copy link
Member

@vkrause vkrause commented Nov 21, 2024

This case had two problems:

  • The temporary file we decrypt into wasn't opened.
  • The temporary encrypted file was removed before renaming the decrypted one into its place. Removing a temporary file however resets its file name (for good reasons, that might meanwhile be owned by a different user), and thus renaming fails due to an empty destination.

Instead we just swap the two temporary files now, as the final destination of the download is a temporary location we decide on here anyway which one we return ultimately doesn't matter.

This case had two problems:
- The temporary file we decrypt into wasn't opened.
- The temporary encrypted file was removed before renaming the decrypted
  one into its place. Removing a temporary file however resets its file
  name (for good reasons, that might meanwhile be owned by a different
  user), and thus renaming fails due to an empty destination.

Instead we just swap the two temporary files now, as the final destination
of the download is a temporary location we decide on here anyway which one
we return ultimately doesn't matter.
@KitsuneRal KitsuneRal added the bug/fix The library doesn't work as expected label Nov 21, 2024
@vkrause
Copy link
Member Author

vkrause commented Nov 24, 2024

CI failure seems unrelated to this change, should I merge anyway?

@TobiasFella
Copy link
Member

yes, the CI fails when being run for branches that come from a fork

@vkrause vkrause merged commit e579e0b into quotient-im:dev Nov 24, 2024
4 of 5 checks passed
@vkrause vkrause deleted the work/vkrause/fix-download-to-temporary-location branch November 24, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/fix The library doesn't work as expected
Projects
Status: 0.9 - Done
Development

Successfully merging this pull request may close these issues.

3 participants