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

Permission denied error when shlink tries to download same GeoLite db more than once #899

Closed
schoeppe opened this issue Nov 13, 2020 · 7 comments · Fixed by #909
Closed
Labels
Milestone

Comments

@schoeppe
Copy link

How Shlink is set-up

  • Shlink Version: 2.4.0
  • PHP Version: 7.4.12
  • How do you serve Shlink: Self-hosted Apache
  • Database engine used: MySQL

Summary

We have setup multiple installations on one server each served by a different vhost.
For geolocation we have configured cronjobs which run in the context of the user/group of the corresponding vhost.

Current behavior

When a Geolite2 database download is available and needs to be downloaded there is a conflict extracting the file Geolite2-City.mmdb file into the temporary directory. The first run of the task ./cli visit:locate creates a directory in the configured temp_dir "GeoLite2-City_[timestamp]" owned by the corresponding user. The second run of the task, started by another user, cannot extract the file to the already existing directory because of the directory ownership.

Expected behavior

I would expect multiple runs of the task visit:locate to be independent of each other, not conflicting in the described way.

How to reproduce

Just run the task ./cli visit:locate for two different installations with two different users.
Make sure there is no GeoLite2-City.mmdb file in the corresponding data-directory.

The first run succeeds, the second run fails with:

 31546360/31546360 [============================] 100%PHP Warning:  PharData::extractTo(/tmp/GeoLite2-City_20201110/GeoLite2-City.mmdb): failed to open stream: Permission denied in /home/user2/htdocs/vendor/shlinkio/shlink-ip-geolocation/src/GeoLite2/DbUpdater.php on line 67


                                                                                                                        
 [ERROR] GeoLite2 database download failed. It is not possible to locate visits.                                        
                                                                                                                        

                                                                                                                        
 [ERROR] An error occurred while updating geolocation database, and an older version could not be found```
@schoeppe schoeppe added the bug label Nov 13, 2020
@schoeppe
Copy link
Author

Related to #621

@acelaya
Copy link
Member

acelaya commented Nov 13, 2020

Thanks for reporting.

I did that command in a way that it sets up a lock that prevents multiple executions in parallel, but I never considered having multiple instances of shlink where each one of them will have it's own lock, but a shared filesystem.

I think this should be easy to fix, by either using a more randomized temp name, or by using some folder inside shlink's instance dir.

Any is those approaches would prevent this collision.

@schoeppe
Copy link
Author

Thanks for the quick reply!

To use the instance directory is a good idea. I updated the config in /home/user2/htdocs/config/params/generated_config.php and added:

 'geolite2' => array(
        'license_key' => 'somelicense',
        'temp_dir' => '/home/user2/htdocs/data',
    ),

This solves the shared filesystem problem.
Is this the place to go for this configuration value?

@acelaya
Copy link
Member

acelaya commented Nov 13, 2020

Yep. I actually didn't remember that config option even existed, haha.

It's probably as easy as that.

@schoeppe
Copy link
Author

Ok, good. For me this issue is solved with the configuration change. Thank you!

Yep. I actually didn't remember that config option even existed, haha.

I had a look at the repos and the code you implemented for this project... quite impressive.
The code is really well structured and easy to read - no need to keep every option in mind! 😉
Good work! 👍

@acelaya
Copy link
Member

acelaya commented Nov 14, 2020

Thanks for the kind words 🙂

@acelaya acelaya added this to the 2.4.2 milestone Nov 15, 2020
acelaya added a commit to acelaya-forks/shlink that referenced this issue Nov 21, 2020
acelaya added a commit to acelaya-forks/shlink that referenced this issue Nov 21, 2020
@acelaya
Copy link
Member

acelaya commented Nov 21, 2020

I have just merged a PR changing the default value for temp_dir so that it points to the data folder of the shlink instance itself, which should solve this problem.

The fix will be part of the upcoming v2.4.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants