Does this module still work with latest Caddy?

mkscs commented 1 year ago

I've built this with xcaddy and am able to pull the maxmind database. However the next thing in the example fails to work:

header geoip-country "{geoip2.country_code}"

outputs only

{geoip2.country_code}

styxlab commented 1 year ago

Any updates on this cas? Wanted to make sure package is working before trying it out.

zhangjiayin commented 1 year ago

Hi @mkscs

Apologies for the delayed response. Could you please provide the complete configuration details?

do you add "geoip2_vars strict" as described in readme ?

zhangjiayin commented 1 year ago

@styxlab We have already conducted tests with caddy v2.6.4 . Should any issues arise, feel free to submit an issue. It would be helpful if you could include comprehensive configuration details when doing so.

As for scenarios where certain variables may pose problems, I have recently updated them. The initialization now defaults to empty strings, which I hope will be helpful for you.

styxlab commented 1 year ago

Thanks, @zhangjiayin. I can confirm that the module works with latest caddy as expected. Thanks a lot for providing it!

Also, the default to empty strings is very helpful, however it does not seem to work for the variables like {geoip2.subdivisions_1_iso_code}, {geoip2.subdivisions_2_iso_code} and possibly some others. Could you check that? This leads to the originally reported error when testing on localhost and that can be a bit confusing.

I also noticed that caddy will crash, when the "lockFile" variable is not provided in the config. This is just a note, in case you want to improve your module (obviously there is a rather simple workaround by providing the parameter in the config).

zhangjiayin commented 1 year ago

Hi @styxlab,

I've made some changes in the caddy-geoip2 plugin's code:

I've set the default values for certain variables, specifically {geoip2.subdivisions_1_iso_code} and {geoip2.subdivisions_2_iso_code}. However, I've chosen not to set a default value for more indexed values.
I've also changed the default value for lockFile. Previously, it defaulted to geoip2.lock in the current directory. Now, I've changed it to /tmp/geoip2.lock. Despite setting these defaults, I would still recommend explicitly configuring these variables to maintain control over your configuration.

You can review these changes in the pull request I've submitted on GitHub: PR #11

Thank you!

styxlab commented 1 year ago

Perfect, thanks so much!

apxcarter commented 4 months ago

Edit: It was an issue with using the directory and edition ID. Mine were different than the defaults, so it couldn't find the file. Nothing to solve now :smile:

Hi there, I'm seeing a similar issue as the original poster here on Caddy 2.7.6 using the example configuration in the readme. Except that I'm getting an empty string for {geoip2.country_code}.

Is there anything that would prevent this plugin from working with this version of Caddy?

zhangjiayin commented 4 months ago

If you change the geoip2_vars to wild

geoip2_vars wild

you could test it and got as below

curl xxxxxx -H'X-Forwarded-For:23.229.22.148' -i
HTTP/1.1 200 OK
Content-Type: text/plain; charset=utf-8
Geoip-Country: US
Geoip-Subdivision: NY
Server: Caddy
Date: Fri, 10 May 2024 05:44:56 GMT
Content-Length: 281

hello everyone except Ohioan:
    geoip2.country_code:US
    geoip2.country_name:United States
    geoip2.city_geoname_id:5110629
    geoip2.city_name:Buffalo
    geoip2.location_latitude:42.8856
    geoip2.location_longitude:-78.8736
    geoip2.location_time_zone:America/New_York%

curl xxxxx  -i
HTTP/1.1 200 OK
Geoip-Country:
Geoip-Subdivision:
Server: Caddy
Date: Fri, 10 May 2024 05:45:58 GMT
Content-Length: 0

it is works as expected. and it is with version caddy_v2.7.6

zhangjiayin / caddy-geoip2

Does this module still work with latest Caddy? #9