Skip to content

Optionally set notes (IFF MaxMindDBSetNotes is On) instead of only env#76

Closed
mfontani wants to merge 2 commits intomaxmind:masterfrom
mfontani:optional_set_notes
Closed

Optionally set notes (IFF MaxMindDBSetNotes is On) instead of only env#76
mfontani wants to merge 2 commits intomaxmind:masterfrom
mfontani:optional_set_notes

Conversation

@mfontani
Copy link
Contributor

The old "mod_geoip" used to set both environment and notes.
This commit makes the setting of the notes optional.

This enables an Apache mod_perl running with:

<Location "/">
  SetHandler perl-script
  PerlOptions -ParseHeaders -SetupEnv -GlobalRequest
  PerlResponseHandler ...
</Location>

... and with a MaxMind configuration of:

LoadModule maxminddb_module  /usr/lib/apache2/modules/mod_maxminddb.so
<IfModule mod_maxminddb.c>
  # See https://github.com/maxmind/mod_maxminddb
  MaxMindDBEnable On
  MaxMindDBSetNotes On
  MaxMindDBFile COUNTRY_DB /path/to/GeoLite2-Country.mmdb
  MaxMindDBEnv MM_GEOIP_COUNTRY_CODE COUNTRY_DB/country/iso_code
  MaxMindDBEnv MM_GEOIP_CONTINENT_CODE COUNTRY_DB/continent/code
</IfModule>

... to get the client's country code & continent code as an Apache
"note", without having to involve the environment.

The reason one usually wants to disable SetupEnv (via "-SetupEnv") for
mod_perl handlers is one of performance; from the docs at:
https://perl.apache.org/docs/2.0/user/config/config.html#C_SetupEnv_c

But %ENV population is expensive. Those who have moved to the Perl
Apache API no longer need this extra %ENV population, and can gain
by disabling it.

The old "mod_geoip" used to set _both_ environment _and_ notes.
This commit makes the setting of the notes optional.

This enables an Apache mod_perl running with:

    <Location "/">
      SetHandler perl-script
      PerlOptions -ParseHeaders -SetupEnv -GlobalRequest
      PerlResponseHandler ...
    </Location>

... and with a MaxMind configuration of:

    LoadModule maxminddb_module  /usr/lib/apache2/modules/mod_maxminddb.so
    <IfModule mod_maxminddb.c>
      # See https://github.com/maxmind/mod_maxminddb
      MaxMindDBEnable On
      MaxMindDBSetNotes On
      MaxMindDBFile COUNTRY_DB /path/to/GeoLite2-Country.mmdb
      MaxMindDBEnv MM_GEOIP_COUNTRY_CODE COUNTRY_DB/country/iso_code
      MaxMindDBEnv MM_GEOIP_CONTINENT_CODE COUNTRY_DB/continent/code
    </IfModule>

... to get the client's country code & continent code as an Apache
"note", without having to involve the environment.

The reason one usually wants to disable SetupEnv (via "-SetupEnv") for
mod_perl handlers is one of performance; from the docs at:

    https://perl.apache.org/docs/2.0/user/config/config.html#C_SetupEnv_c

    > But %ENV population is expensive. Those who have moved to the Perl
    > Apache API no longer need this extra %ENV population, and can gain
    > by disabling it.
A little refactoring, but the function might want a better name..
@mfontani
Copy link
Contributor Author

Note both commits might want to get squashed prior to merging if you wish to merge both; I'm happy to do so if wanted but am unsure about a nice, proper name for the newly introduced function. I'm happy to rename it to whatever you think would be best; rebase & squash as required.

@horgh horgh closed this in #85 Jan 27, 2020
horgh added a commit that referenced this pull request Jan 27, 2020
Rebase #76 and add tests and change log
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant