Skip to content

Make copy non-blocking for the encryption wrapper - #28925

Closed
artonge wants to merge 2 commits into
masterfrom
fix/make_transfert_ownership_non_blocking
Closed

Make copy non-blocking for the encryption wrapper #28925
artonge wants to merge 2 commits into
masterfrom
fix/make_transfert_ownership_non_blocking

Conversation

@artonge

@artongeartonge commented Sep 22, 2021

Copy link
Copy Markdown
Collaborator

Issue

During an ownership transfer, if a file fails to be moved, then the transfer stops. This PR partly changes this behaviour for encryption error.

What this PR changes:

  1. When called from the CLI, ignore the exceptions in copyBetweenStorage method of Files/Storage/Wrapper/Encryption.php
  2. Log, in both the CLI and the log file to let the admin know which files are problematic.

Testing steps from a fresh install with encryption enabled:

  • add a new file 1.md to Alice
  • disable encryption php occ encryption:disable
  • add a new file 2.md to Alice
  • enable encryption php occ encryption:enable
  • set encryption=1 in oc_filecache for 2.md
  • add a new file 3.md to Alice
  • run php occ files:transfer-ownership alice bob
  • run file:scan bob

Without this PR:

  • The transfer folder contains only 1.md
  • An error message is logged in the CLI but without any information about the failing files
  • The files still exist in the original location
  • The command return 1

With this PR (depending on the chosen solution below):

  • The transfer folder exists and contains all the files besides 2.md
  • All the files have been deleted from the original location
  • An error message with the problematic file names is logged in the CLI
  • A log with the problematic file names and the exception is added to the server logs
  • The command return 1

Problems

  • If we ignore the encryption errors, the problematic files are not copied, but are still removed.
  • If we do not ignore the encryption errors, the copy is stopped halfway through.

Alternative solutions

  • In the handling of the encryption error, we can copy the encrypted file "as is" without checking the encryption, and continue with the copy.

Decision

We won't merge this as it is too hacky.

@artonge
artongeforce-pushed the fix/make_transfert_ownership_non_blocking branch 3 times, most recently from 74bc442 to e8cbbf4CompareSeptember 22, 2021 10:31
@artongeartonge self-assigned this Sep 22, 2021
@artonge
artongeforce-pushed the fix/make_transfert_ownership_non_blocking branch from aff5ee0 to 4b18e42CompareSeptember 22, 2021 13:51
@artongeartonge changed the title Make transfer ownership non-blockingMake copy non-blocking for the encryption wrapper Sep 22, 2021
@artonge
artongeforce-pushed the fix/make_transfert_ownership_non_blocking branch 3 times, most recently from 8e7a101 to 527c5a7CompareSeptember 24, 2021 07:03
Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge
artongeforce-pushed the fix/make_transfert_ownership_non_blocking branch from 527c5a7 to 2da4989CompareSeptember 24, 2021 07:05
@artonge
artongeforce-pushed the fix/make_transfert_ownership_non_blocking branch from fcfb785 to 2da4989CompareOctober 28, 2021 11:12
Signed-off-by: Louis Chemineau <louis@chmn.me>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@artonge