From 060226d9a917dfc2a77c6232d7d04c7be9ca6165 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Thu, 23 Apr 2020 13:54:36 -0500 Subject: [PATCH 1/2] Better locale matching against broad groups. Fixes #2774 --- system/HTTP/Negotiate.php | 54 +++++++++++++++++++---- tests/system/HTTP/IncomingRequestTest.php | 27 ++++++++++-- tests/system/HTTP/NegotiateTest.php | 11 +++++ 3 files changed, 80 insertions(+), 12 deletions(-) diff --git a/system/HTTP/Negotiate.php b/system/HTTP/Negotiate.php index 6ac2d06b7bc4..74aa4706227d 100644 --- a/system/HTTP/Negotiate.php +++ b/system/HTTP/Negotiate.php @@ -179,7 +179,7 @@ public function encoding(array $supported = []): string */ public function language(array $supported): string { - return $this->getBestMatch($supported, $this->request->getHeaderLine('accept-language')); + return $this->getBestMatch($supported, $this->request->getHeaderLine('accept-language'), false, false, true); } //-------------------------------------------------------------------- @@ -198,10 +198,11 @@ public function language(array $supported): string * @param boolean $enforceTypes If TRUE, will compare media types and sub-types. * @param boolean $strictMatch If TRUE, will return empty string on no match. * If FALSE, will return the first supported element. + * @param boolean $matchLocales If TRUE, will match locale sub-types to a broad type (fr-FR = fr) * * @return string Best match */ - protected function getBestMatch(array $supported, string $header = null, bool $enforceTypes = false, bool $strictMatch = false): string + protected function getBestMatch(array $supported, string $header = null, bool $enforceTypes = false, bool $strictMatch = false, bool $matchLocales = false): string { if (empty($supported)) { @@ -232,7 +233,7 @@ protected function getBestMatch(array $supported, string $header = null, bool $e // If an acceptable value is supported, return it foreach ($supported as $available) { - if ($this->match($accept, $available, $enforceTypes)) + if ($this->match($accept, $available, $enforceTypes, $matchLocales)) { return $available; } @@ -337,12 +338,14 @@ public function parseHeader(string $header): array /** * Match-maker * - * @param array $acceptable - * @param string $supported - * @param boolean $enforceTypes + * @param array $acceptable + * @param string $supported + * @param boolean $enforceTypes + * @param boolean $matchLocales + * * @return boolean */ - protected function match(array $acceptable, string $supported, bool $enforceTypes = false): bool + protected function match(array $acceptable, string $supported, bool $enforceTypes = false, $matchLocales = false): bool { $supported = $this->parseHeader($supported); if (is_array($supported) && count($supported) === 1) @@ -363,6 +366,12 @@ protected function match(array $acceptable, string $supported, bool $enforceType return $this->matchTypes($acceptable, $supported); } + // Do we need to match locales against broader locales? + if ($matchLocales) + { + return $this->matchLocales($acceptable, $supported); + } + return false; } @@ -409,8 +418,14 @@ protected function matchParameters(array $acceptable, array $supported): bool */ public function matchTypes(array $acceptable, array $supported): bool { - list($aType, $aSubType) = explode('/', $acceptable['value']); - list($sType, $sSubType) = explode('/', $supported['value']); + [ + $aType, + $aSubType, + ] = explode('/', $acceptable['value']); + [ + $sType, + $sSubType, + ] = explode('/', $supported['value']); // If the types don't match, we're done. if ($aType !== $sType) @@ -429,4 +444,25 @@ public function matchTypes(array $acceptable, array $supported): bool } //-------------------------------------------------------------------- + + /** + * Will match locales against their broader pairs, so that fr-FR would + * match a supported localed of fr + * + * @param array $acceptable + * @param array $supported + * + * @return boolean + */ + public function matchLocales(array $acceptable, array $supported): bool + { + $aBroad = mb_strpos($acceptable['value'], '-') > 0 + ? mb_substr($acceptable['value'], 0, mb_strpos($acceptable['value'], '-')) + : $acceptable['value']; + $sBroad = mb_strpos($supported['value'], '-') > 0 + ? mb_substr($supported['value'], 0, mb_strpos($supported['value'], '-')) + : $supported['value']; + + return strtolower($aBroad) === strtolower($sBroad); + } } diff --git a/tests/system/HTTP/IncomingRequestTest.php b/tests/system/HTTP/IncomingRequestTest.php index 6457f92da6a6..ef1b4f6a8de1 100644 --- a/tests/system/HTTP/IncomingRequestTest.php +++ b/tests/system/HTTP/IncomingRequestTest.php @@ -202,22 +202,43 @@ public function testSetBadLocale() //-------------------------------------------------------------------- + /** + * @see https://github.com/codeigniter4/CodeIgniter4/issues/2774 + */ public function testNegotiatesLocale() { - $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'es; q=1.0, en; q=0.5'; + $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'fr-FR; q=1.0, en; q=0.5'; $config = new App(); $config->negotiateLocale = true; $config->supportedLocales = [ + 'fr', 'en', - 'es', ]; $config->baseURL = 'http://example.com'; $request = new IncomingRequest($config, new URI(), null, new UserAgent()); $this->assertEquals($config->defaultLocale, $request->getDefaultLocale()); - $this->assertEquals('es', $request->getLocale()); + $this->assertEquals('fr', $request->getLocale()); + } + + public function testNegotiatesLocaleOnlyBroad() + { + $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'fr; q=1.0, en; q=0.5'; + + $config = new App(); + $config->negotiateLocale = true; + $config->supportedLocales = [ + 'fr', + 'en', + ]; + $config->baseURL = 'http://example.com'; + + $request = new IncomingRequest($config, new URI(), null, new UserAgent()); + + $this->assertEquals($config->defaultLocale, $request->getDefaultLocale()); + $this->assertEquals('fr', $request->getLocale()); } // The negotiation tests below are not intended to exercise the HTTP\Negotiate class - diff --git a/tests/system/HTTP/NegotiateTest.php b/tests/system/HTTP/NegotiateTest.php index 1c8ffb204f3c..edeabd6503a1 100644 --- a/tests/system/HTTP/NegotiateTest.php +++ b/tests/system/HTTP/NegotiateTest.php @@ -138,6 +138,17 @@ public function testAcceptLanguageBasics() } //-------------------------------------------------------------------- + + /** + * @see + */ + public function testAcceptLanguageMatchesBroadly() + { + $this->request->setHeader('Accept-Language', 'fr-FR,fr;q=0.9,en-US;q=0.8,en;q=0.7'); + + $this->assertEquals('fr', $this->negotiate->language(['fr', 'en'])); + } + public function testBestMatchEmpty() { $this->expectException(Exceptions\HTTPException::class); From cf5bf7cb56f78941ab402fbc936b7752077d5bce Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Thu, 23 Apr 2020 14:32:17 -0500 Subject: [PATCH 2/2] Added link for @see --- tests/system/HTTP/NegotiateTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/HTTP/NegotiateTest.php b/tests/system/HTTP/NegotiateTest.php index edeabd6503a1..1db5cdae4cbd 100644 --- a/tests/system/HTTP/NegotiateTest.php +++ b/tests/system/HTTP/NegotiateTest.php @@ -140,7 +140,7 @@ public function testAcceptLanguageBasics() //-------------------------------------------------------------------- /** - * @see + * @see https://github.com/codeigniter4/CodeIgniter4/issues/2774 */ public function testAcceptLanguageMatchesBroadly() {