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

Geolocation wrong on internal network even with enable_language_to_country_guess=0 #18625

Closed
brandtmark opened this issue Jan 14, 2022 · 4 comments · Fixed by #18634
Closed
Assignees
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Milestone

Comments

@brandtmark
Copy link

brandtmark commented Jan 14, 2022

Our websites are accessed externally and internally (with an IP from the local network range).
GeoIp is installed and enabled.

Despite having set enable_language_to_country_guess=0, the geolocation information for users from the local network is set based on their browser language and therefore totally wrong.

This seems to happen in Common::extractCountryCodeFromBrowserLanguage and I’ve got the feeling that this function does not respect the config value correctly.

public static function extractCountryCodeFromBrowserLanguage($browserLanguage, $validCountries, $enableLanguageToCountryGuess)
{
	/** @var LanguageDataProvider $dataProvider */
	$dataProvider = StaticContainer::get('Piwik\Intl\Data\Provider\LanguageDataProvider');

	$langToCountry = $dataProvider->getLanguageToCountryList();

	if ($enableLanguageToCountryGuess) {
		if (preg_match('/^([a-z]{2,3})(?:,|;|$)/', $browserLanguage, $matches)) {
			// match language (without region) to infer the country of origin
			if (array_key_exists($matches[1], $langToCountry)) {
				return $langToCountry[$matches[1]];
			}
		}
	}

        // the following code should run only if $enableLanguageToCountryGuess is true

	if (!empty($validCountries) && preg_match_all('/[-]([a-z]{2})/', $browserLanguage, $matches, PREG_SET_ORDER)) {
		foreach ($matches as $parts) {
			// match location; we don't make any inferences from the language
			if (array_key_exists($parts[1], $validCountries)) {
				return $parts[1];
			}
		}
	}
	return self::LANGUAGE_CODE_INVALID;
}

Expected Behavior

When setting "enable_language_to_country_guess=0" in config, the country code should never be guessed from the browser language. It should be "unknown".

Current Behavior

Despite having enable_language_to_country_guess=0 set in config, the geolocation for users with local network IP addresses is extracted from the browser language resulting in completely wrong countries.

Possible Solution

The function Common::extractCountryCodeFromBrowserLanguage should respect the setting correctly

@brandtmark brandtmark added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Jan 14, 2022
@sgiehl
Copy link
Member

sgiehl commented Jan 17, 2022

Hi @brandtmark
Thanks for creating this issue. The setting might be a bit misleading. Disabling the country guess by language, actually only disables using the language to country mapping as defined here:

return array(
'bg' => 'bg', // Bulgarian => Bulgaria
'ca' => 'es', // Catalan => Spain
'cs' => 'cz', // Czech => Czech Republic
'da' => 'dk', // Danish => Denmark
'de' => 'de', // German => Germany
'el' => 'gr', // Greek => Greece
'es' => 'es', // Spanish => Spain
'et' => 'ee', // Estonian => Estonia
'fa' => 'ir', // Farsi => Iran
'fi' => 'fi', // Finnish => Finland
'fr' => 'fr', // French => France
'he' => 'il', // Hebrew => Israel
'hr' => 'hr', // Croatian => Croatia
'hu' => 'hu', // Hungarian => Hungary
'id' => 'id', // Indonesian => Indonesia
'is' => 'is', // Icelandic => Iceland
'it' => 'it', // Italian => Italy
'ja' => 'jp', // Japanese => Japan
'ko' => 'kr', // Korean => South Korea
'lt' => 'lt', // Lithuanian => Lithuania
'lv' => 'lv', // Latvian => Latvia
'mk' => 'mk', // Macedonian => Macedonia
'ms' => 'my', // Malay => Malaysia
'nb' => 'no', // Bokmål => Norway
'nl' => 'nl', // Dutch => Netherlands
'nn' => 'no', // Nynorsk => Norway
'no' => 'no', // Norwegian => Norway
'pl' => 'pl', // Polish => Poland
'pt' => 'pt', // Portuguese => Portugal
'ro' => 'ro', // Romanian => Romania
'ru' => 'ru', // Russian => Russia
'sk' => 'sk', // Slovak => Slovakia
'sl' => 'si', // Slovene => Slovenia
'sq' => 'al', // Albanian => Albania
'sr' => 'rs', // Serbian => Serbia
'sv' => 'se', // Swedish => Sweden
'th' => 'th', // Thai => Thailand
'bo' => 'cn', // Tibetan => China
'tr' => 'tr', // Turkish => Turkey
'uk' => 'ua', // Ukrainian => Ukraine
);

But it still uses the country if it's defined in the language. Like en-US indicates the US, while en-GB would indicate England, de-CH would be Swiss,... If someone uses a language like fr only, that wouldn't be mapped to France without the language guess.

I guess this could be clarified a bit better.

Nevertheless, I guess you are actually looking for an option to disable falling back to the default provider if the GeoIP provider didn't have results, right?

@brandtmark
Copy link
Author

Hi @sgiehl

Thanks for the explanation.

Exactly. I would prefer that clients who access our websites from the internal network don't to show up as visitors from US, UK, etc. just because of their browser language.

@sgiehl
Copy link
Member

sgiehl commented Jan 17, 2022

Should be quite simple to introduce a new config setting that makes this possible. I'll set up a pull request to do that. But can't promise when this will be included in Matomo. Likely not before 4.8.0, as 4.7.0-rc1 was already released.

@sgiehl sgiehl added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Jan 17, 2022
@sgiehl sgiehl self-assigned this Jan 17, 2022
@brandtmark
Copy link
Author

Wow - i didn't expect this issue to be addressed that quickly.
Thanks a lot! Amazing!

@sgiehl sgiehl added this to the 4.8.0 milestone Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants