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

Use a random name when creating intermediate tmp archive for teleporter backup #2242

Merged
merged 1 commit into from Jul 4, 2022

Conversation

yubiuser
Copy link
Member

@yubiuser yubiuser commented Jul 2, 2022

  • What does this PR aim to accomplish?:

Fixes an issue reported here:
https://discourse.pi-hole.net/t/fatal-php-error-with-teleporter/56264/

The user created teleporter backups via cron. The script was running under /tmp. During the backup creation, PHP compresses the data and creates a temporary copy within /tmp with the same filename as the backup. This leads to

PHP Fatal error: Uncaught BadMethodCallException: phar "/tmp/pi-hole-pihole-wf-inside-teleporter_2022-06-30_11-21-35.tar.gz" exists and must be unlinked prior to conversion in /var/www/html/admin/scripts/pi-hole/php/teleporter.php:634 Stack trace: #0 /var/www/html/admin/scripts/pi-hole/php/teleporter.php(634): PharData->compress() #1 {main} thrown in /var/www/html/admin/scripts/pi-hole/php/teleporter.php on line 634
  • How does this PR accomplish the above?:

Use a random name when creating intermediate tmp archive for teleporter backup with tempnam(sys_get_temp_dir())


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

  • I have read the above and my PR is ready for review. Check this box to confirm

@yubiuser yubiuser requested a review from a team July 2, 2022 15:25
@yubiuser yubiuser added the PR: Code Review Required Open Pull Request, needs code reviewed label Jul 2, 2022
rdwebdesign
rdwebdesign previously approved these changes Jul 2, 2022
@dschaper
Copy link
Member

dschaper commented Jul 2, 2022

Can you use mktemp and not have to worry about changing modes and owners?

@yubiuser
Copy link
Member Author

yubiuser commented Jul 3, 2022

Do not merge atm. I'll see if I can get this working with tempnam()

@yubiuser yubiuser marked this pull request as draft July 3, 2022 11:50
@yubiuser
Copy link
Member Author

yubiuser commented Jul 3, 2022

Using tempnam() now.

rockpi@rockpi-4b:/tmp$ pihole -a -t
rockpi@rockpi-4b:/tmp$ pihole -a -t
rockpi@rockpi-4b:/tmp$ pihole -a -t
rockpi@rockpi-4b:/tmp$ ls -la
....
-rw-r--r--  1 root   root   6045 Jul  3 23:26 pi-hole-rockpi-4b-teleporter_2022-07-03_23-26-22.tar.gz
-rw-r--r--  1 root   root   6045 Jul  3 23:26 pi-hole-rockpi-4b-teleporter_2022-07-03_23-26-23.tar.gz
-rw-r--r--  1 root   root   6047 Jul  3 23:26 pi-hole-rockpi-4b-teleporter_2022-07-03_23-26-24.tar.gz

No issues occurred.

Signed-off-by: Christian König <ckoenig@posteo.de>
@yubiuser yubiuser changed the title Create teleporter tmp file in /tmp/pihole Use a random name when creating intermediate tmp archive for teleporter backup Jul 3, 2022
@yubiuser yubiuser marked this pull request as ready for review July 3, 2022 21:38
@yubiuser yubiuser merged commit 7acc7d8 into devel Jul 4, 2022
@yubiuser yubiuser deleted the teleporter_tmp branch July 4, 2022 06:42
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-16-web-v5-13-and-core-v5-11-1-released/56384/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Code Review Required Open Pull Request, needs code reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants