nextcloudpi icon indicating copy to clipboard operation
nextcloudpi copied to clipboard

Prevent backup restoration if datadir within ncdir (#1400)

Open mackncheesiest opened this issue 2 years ago • 3 comments

Follow up to #1400, this is a quick changeset that attempts to fix the bug described there.

Perhaps it would be better in general to try to move the datadir somewhere else if this ever happens. Given that there is already similar logic elsewhere in the script that I'd prefer not to try to coordinate with (since they'd need to know where it gets moved to), this seems like a reasonable way of making script execution quite a bit safer at the cost of being slightly more inconvenient for people who were already planning to replace their datadir.

mackncheesiest avatar Dec 12 '21 04:12 mackncheesiest

Thanks for your feedback.

This is not a bug in the sense that you are restoring a dataless backup, so your instance will have no data.

Now that being said, we did add some logic for the case where the user had moved the datadir before and if it finds it when restoring a dataless backup, then it will use it. We can add something similar for this case.

  • move the datadir at the beginning of the script
  • restore it in the case of dataless
  • in any case, remove the moved data -rf at the end

Do you want to give that a try?

nachoparker avatar Dec 13 '21 19:12 nachoparker

I'll give it a shot

mackncheesiest avatar Dec 13 '21 20:12 mackncheesiest

Since the existing logic for restoring a with-data backup seemed to be leaving the user's data intact in $DATADIR-$( date "+%m-%d-%y" ) after restoration, I figured that in that case, this new backup folder can fill that role. I'm fine with actually performing the deletion that was mentioned above if it's desired, though

However, in a no-data backup restoration I move the folder's contents and remove the folder. Now that I think of it, it might be nice to have an expression there that doesn't rely on glob expansion since i.e. dotfiles are probably missed here

mackncheesiest avatar Dec 13 '21 21:12 mackncheesiest

This should now be fixed (or rather: the default behavior changed in #1601). Sorry for not considering your PR. Frankly, I had missed it while working on the issue and nacho has left the project as developer

theCalcaholic avatar Jan 07 '23 19:01 theCalcaholic

No worries, I'm just happy to see it eventually got fixed! I've migrated away from Nextcloud in general due to a lack of free time to handle issues that came up, but I hope to return to the ecosystem in the future

mackncheesiest avatar Jan 26 '23 21:01 mackncheesiest