Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Ability to manually specify geoip database file to use #2

Closed
damianb opened this Issue · 7 comments

2 participants

@damianb

Are there any plans to allow script authors to set the geoip database file to use via a php function? I know this wasn't present in the original extension, but it would certainly be a welcome feature.

@Zakay
Owner

I like the idea at first, but when thinking about it a bit further, if you have the access to change the path for the geoip data, or install a custom geoip extension, you also have the access to change the ini file. The extension already has support for changing the directory of all geoip databasefiles.

@Zakay Zakay was assigned
@damianb

What of the possibility where the extension is installed by a system administrator/hosting provider, and the end user supplies their own geoip database such as the full, licensed GeoIP database. As part of the license agreement for the per-site license of the geoip database, you're agreeing not to "re-assigned, resold, or made available to any 3rd parties without MaxMind's written consent". (See also: http://www.maxmind.com/app/faq#hostingprovider ). By that license, you can't just globally install that database, you have to have it only available to the one client that owns it - this throws a common geoip database directory out the window. Also, most shared hosting providers also do not run separate instances of PHP for each vict...errr, client, and also don't allow php.ini to be altered by the client either.
While they might allow the end-user to tweak the value using .htaccess, it's not an elegant solution nor a reliable one.

I apologize if it seems I'm being difficult, but I'd like to be able to rely on a geoip extension for readily-deployed code, without having to subject users to requirements of having to modify php.ini, getting their host to modify php.ini, etc. just so they can use their own database file. If they want to use the licensed one, or be able to update it themselves, I think that functionality should be available.

@Zakay
Owner

I see your point, the way the geoip library is built is making it a little bit uncomfortable. The directory is set on a global variable. Changing the directory path of the geo ip database will not only change it for the current running script btu also change it for the next running script on the same fastcgi/fpm process. So if the user doesn't change it back the next user will have someone else database.
Creating two functions: geoip_change_directory and geoip reset_directory function.

The other way would be creating a copy of every function defined and adding an extra argument for which database directory to use, and every function changes the directory and then resets this. Luckily PHP doesn't execute scripts in parallel so we won't have any issues if you change it back.
This option requires the developers to make changes to their code and they need to be informed of it being less portable.

Third option is to let the users change php.ini with .htaccess.

What do you think about our options?

@damianb

Quick thought regarding the first idea - geoip_change_directory could just do both, have a default value which triggers a reset of the directory (similar to the behavior of another internal php function, can't remember which it is at the moment).
Also, I'm not sure if this is possible, but couldn't you hook onto script termination (similar to how app developers can with register_shutdown_function()) to trigger the code necessary for the directory to be reset?

Additionally, there is the possibility of moving towards a different setup, say a file resource handler, similar to how the curl extension behaves. Open a geoip resource handler with a function, use functions and pass the resource through the first function, and go on with your life. Thing is, this is an older style for PHP - newer stuff uses objects instead, similar to how DateTime works (constructor for the object takes the args, stores the resource handler internally, and you manipulate via object methods instead of handlers and functions).
Both of those options though would break code significantly, and at that point it becomes a whole new extension, and any "odd" behaviors could be addressed then and there with the excuse of "we're breaking everything". If that were the path taken, it'd probably be best to go with how PHP5+ behaves and use objects instead of passing resources to functions.

@Zakay
Owner

Yeah, that would be a quick solution registering a shutdown function to restore its value.

I could probably do that first, then I have also had the idea to create a more PHP5+ style of extension with a GeoIP object instead. That could take a custom directory by the constructor and resetting it on the destructor.

Considering my current situation I could release the first solution pretty quickly but on the second I would have to push that for a much later release. I welcome pull request on this if someone else does this before I do.

I'll go ahead and create a 1.0 milestone for this project containing a GeoIP Object goal.

@Zakay
Owner

Just commited the changes: geoip_setup_custom_directory is now available in PHP and it resets when the script is complete.

@Zakay Zakay closed this
@damianb

alright, I'll compile this on my box and give it a shot when I get a chance, might be in a few days. Busy busy. (Curse you, calculus.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.