Katie Horn has uploaded a new change for review.
https://gerrit.wikimedia.org/r/87492
Change subject: Adding validation for country code Also cleaning up a little in
DonationData.
......................................................................
Adding validation for country code
Also cleaning up a little in DonationData.
Change-Id: Ia7a20e63ea1ff23fd3faeb81c3f084854a236a8f
---
M gateway_common/DataValidator.php
M gateway_common/DonationData.php
2 files changed, 97 insertions(+), 21 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/92/87492/1
diff --git a/gateway_common/DataValidator.php b/gateway_common/DataValidator.php
index 101e456..b24df29 100644
--- a/gateway_common/DataValidator.php
+++ b/gateway_common/DataValidator.php
@@ -1106,4 +1106,44 @@
}
return true;
}
+
+ /**
+ * Checks to see if $country is a valid iso 3166-1 country code.
+ * DOES NOT VERIFY THAT WE FUNDRAISE THERE. Only that the code makes
sense.
+ * @param string $country the code we want to check
+ * @return boolean
+ */
+ public static function is_valid_iso_country_code( $country ) {
+ /**
+ * List of valid iso 3166 country codes, regenerated on
1380836686
+ * Code generated by a happy script at
+ *
https://gerrit.wikimedia.org/r/#/admin/projects/wikimedia/fundraising/tools,branches
+ */
+ $iso_3166_codes = array (
+ 'AF', 'AX', 'AL', 'DZ', 'AS', 'AD', 'AO', 'AI', 'AQ',
'AG', 'AR', 'AM', 'AW', 'AU',
+ 'AT', 'AZ', 'BS', 'BH', 'BD', 'BB', 'BY', 'BE', 'BZ',
'BJ', 'BM', 'BT', 'BO', 'BQ',
+ 'BA', 'BW', 'BV', 'BR', 'IO', 'BN', 'BG', 'BF', 'BI',
'KH', 'CM', 'CA', 'CV', 'KY',
+ 'CF', 'TD', 'CL', 'CN', 'CX', 'CC', 'CO', 'KM', 'CG',
'CD', 'CK', 'CR', 'CI', 'HR',
+ 'CU', 'CW', 'CY', 'CZ', 'DK', 'DJ', 'DM', 'DO', 'EC',
'EG', 'SV', 'GQ', 'ER', 'EE',
+ 'ET', 'FK', 'FO', 'FJ', 'FI', 'FR', 'GF', 'PF', 'TF',
'GA', 'GM', 'GE', 'DE', 'GH',
+ 'GI', 'GR', 'GL', 'GD', 'GP', 'GU', 'GT', 'GG', 'GN',
'GW', 'GY', 'HT', 'HM', 'VA',
+ 'HN', 'HK', 'HU', 'IS', 'IN', 'ID', 'IR', 'IQ', 'IE',
'IM', 'IL', 'IT', 'JM', 'JP',
+ 'JE', 'JO', 'KZ', 'KE', 'KI', 'KP', 'KR', 'KW', 'KG',
'LA', 'LV', 'LB', 'LS', 'LR',
+ 'LY', 'LI', 'LT', 'LU', 'MO', 'MK', 'MG', 'MW', 'MY',
'MV', 'ML', 'MT', 'MH', 'MQ',
+ 'MR', 'MU', 'YT', 'MX', 'FM', 'MD', 'MC', 'MN', 'ME',
'MS', 'MA', 'MZ', 'MM', 'NA',
+ 'NR', 'NP', 'NL', 'NC', 'NZ', 'NI', 'NE', 'NG', 'NU',
'NF', 'MP', 'NO', 'OM', 'PK',
+ 'PW', 'PS', 'PA', 'PG', 'PY', 'PE', 'PH', 'PN', 'PL',
'PT', 'PR', 'QA', 'RE', 'RO',
+ 'RU', 'RW', 'BL', 'SH', 'KN', 'LC', 'MF', 'PM', 'VC',
'WS', 'SM', 'ST', 'SA', 'SN',
+ 'RS', 'SC', 'SL', 'SG', 'SX', 'SK', 'SI', 'SB', 'SO',
'ZA', 'GS', 'SS', 'ES', 'LK',
+ 'SD', 'SR', 'SJ', 'SZ', 'SE', 'CH', 'SY', 'TW', 'TJ',
'TZ', 'TH', 'TL', 'TG', 'TK',
+ 'TO', 'TT', 'TN', 'TR', 'TM', 'TC', 'TV', 'UG', 'UA',
'AE', 'GB', 'US', 'UM', 'UY',
+ 'UZ', 'VU', 'VE', 'VN', 'VG', 'VI', 'WF', 'EH', 'YE',
'ZM', 'ZW',
+ );
+
+ if ( in_array( $country, $iso_3166_codes ) ) {
+ return true;
+ }
+ return false;
+ }
+
}
diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php
index 3bc8a0d..721db66 100644
--- a/gateway_common/DonationData.php
+++ b/gateway_common/DonationData.php
@@ -504,21 +504,64 @@
/**
* normalize helper function
- * Setting the country correctly.
- * If we have no country, we try to get something rational through
GeoIP
- * lookup.
+ * Setting the country correctly. Country is... kinda important.
+ * If we have no country, or nonsense, we try to get something rational
+ * through GeoIP lookup.
*/
protected function setCountry() {
- if ( !$this->isSomething('country') ){
- // If no country was passed, try to do GeoIP lookup
- // Requires php5-geoip package
- if ( function_exists( 'geoip_country_code_by_name' ) ) {
- $ip = $this->getVal( 'user_ip' );
- if ( IP::isValid( $ip ) ) {
- $country = geoip_country_code_by_name(
$ip );
- $this->setVal('country', $country);
+ static $lookup_result = NULL; //keep us from doing the lookup
more than once.
+ $regen = true;
+ $country = '';
+
+ if ( $this->isSomething( 'country' ) ) {
+ $country = strtoupper( $this->getVal( 'country' ) );
+ if ( DataValidator::is_valid_iso_country_code( $country
) ) {
+ $regen = false;
+ } else {
+ //check to see if it's one of those other codes
we grudgingly accept, for the logs
+ //If this logs annoying quantities of nothing
useful, go ahead and kill this whole else block later.
+ //we're still going to try to regen.
+ $near_countries = array ('XX', 'A1',);
+ if ( !in_array( $country, $near_countries ) ) {
+ $this->log(
$this->getLogMessagePrefix() . __FUNCTION__ . ": $country is not a country, or
a recognized placeholder.", LOG_WARNING );
}
}
+ } else {
+ $this->log( $this->getLogMessagePrefix() . __FUNCTION__
. ': Country not set.', LOG_WARNING );
+ }
+
+ //try to regenerate the country if we still don't have a valid
one yet
+ if ( $regen ) {
+ if ( is_null( $lookup_result ) ) {
+ // If no valid country was passed, try to do
GeoIP lookup
+ // Requires php5-geoip package
+ if ( function_exists(
'geoip_country_code_by_name' ) ) {
+ $ip = $this->getVal( 'user_ip' );
+ if ( IP::isValid( $ip ) ) {
+ //I hate @suppression at least
as much as you do, but this geoip function is being genuinely horrible.
+ //try/catch did not help me
suppress the notice it emits when it can't find a host.
+ //The goggles; They do
*nothing*.
+ $country =
@geoip_country_code_by_name( $ip );
+ if ( !$country ) {
+ $this->log(
$this->getLogMessagePrefix() . __FUNCTION__ . ": GeoIP lookup function found
nothing for $ip! No country available.", LOG_WARNING );
+ }
+ }
+ } else {
+ $this->log(
$this->getLogMessagePrefix() . 'GeoIP lookup function is missing! No country
available.', LOG_WARNING );
+ }
+
+ //still nothing good? Give up.
+ if ( !DataValidator::is_valid_iso_country_code(
$country ) ) {
+ $country = 'XX';
+ }
+ $lookup_result = $country;
+ } else {
+ $country = $lookup_result;
+ }
+ }
+
+ if ( $country != $this->getVal( 'country' ) ) {
+ $this->setVal( 'country', $country );
}
}
@@ -536,18 +579,18 @@
if ( $this->isSomething( 'currency' ) ) {
$currency = $this->getVal( 'currency' );
$this->expunge( 'currency' );
- $this->log( $this->getLogMessagePrefix() . "Got
currency from 'currency', now: $currency" );
+ $this->log( $this->getLogMessagePrefix() . "Got
currency from 'currency', now: $currency", LOG_DEBUG );
}
if ( $this->isSomething( 'currency_code' ) ) {
$currency = $this->getVal( 'currency_code' );
- $this->log( $this->getLogMessagePrefix() . "Got
currency from 'currency_code', now: $currency" );
+ $this->log( $this->getLogMessagePrefix() . "Got
currency from 'currency_code', now: $currency", LOG_DEBUG );
}
//TODO: This is going to fail miserably if there's no country
yet.
if ( !$currency ){
require_once( dirname( __FILE__ ) .
'/nationalCurrencies.inc' );
$currency =
getNationalCurrency($this->getVal('country'));
- $this->log( $this->getLogMessagePrefix() . "Got
currency from 'country', now: $currency" );
+ $this->log( $this->getLogMessagePrefix() . "Got
currency from 'country', now: $currency", LOG_DEBUG );
}
$this->setVal( 'currency_code', $currency );
@@ -1160,13 +1203,6 @@
if( $this->isSomething( 'ffname' ) ){
$tracking_data['payments_form'] .= '.' .
$this->getVal( 'ffname' );
}
- }
-
- // @todo remove if-block after some period of time
- if( !$wgContributionTrackingAnalyticsUpgrade ){
- unset( $tracking_data['country'] );
- unset( $tracking_data['form_amount'] );
- unset( $tracking_data['payments_form'] );
}
return $tracking_data;
--
To view, visit https://gerrit.wikimedia.org/r/87492
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia7a20e63ea1ff23fd3faeb81c3f084854a236a8f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Katie Horn <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
