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

Bug: CodeIgniter\Router\Router hasLocale returns true even if {locale} is absent #3386

Closed
tomaluca95 opened this issue Jul 23, 2020 · 5 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@tomaluca95
Copy link

Describe the bug
When using $this->request->getLocale() if the route don't have the {lang} segment i get always the default language because the $this->router->hasLocale() returns true in system/CodeIgniter.php line 798

CodeIgniter 4 version
4.0.4

Affected module(s)
Router

Expected behavior, and steps to reproduce if appropriate

The router should try to match the locale only if {locale} is present

Context

  • OS: CentOS 7 / Debian 10
  • Web server 2.4.6 / 2.4.38-3+deb10u3
  • PHP 7.4.8 / 7.3+69
@tomaluca95 tomaluca95 added the bug Verified issues on the current code behavior or pull requests that will fix them label Jul 23, 2020
@michalsn
Copy link
Member

Maybe I don't understand you correctly, but I couldn't reproduce the behavior you're describing here.

Please check out the methods that are available in the framework to set Locale dynamically: https://codeigniter.com/user_guide/outgoing/localization.html?highlight=locale#locale-detection

I'm closing this but feel free to continue the conversation if you can provide some better example for this issue.

@tomaluca95
Copy link
Author

tomaluca95 commented Jul 24, 2020

In Routes.php i have

$routes->get('/code/(:any)', 'DefaultController::code/$1');
$routes->get('/menu/(:any)', 'DefaultController::menu/$1');

$routes->get('/{locale}', 'MainSite::index');
$routes->get('/{locale}/menu/(:any)', 'ShowMenu::index/$1');

In App.php I have

public $defaultLocale = 'it';
public $negotiateLocale = true;
public $supportedLocales = ['it','en','es','de','fr'];

i see the setLocale called with menu as parameter in the locale in the DefaultController::menu

Running $targetLocale = $this->request->negotiate("language", (new \Config\App())->supportedLocales); gives back the right value

@michalsn
Copy link
Member

Sorry, but I followed your example and was not able to reproduce the error. Here are my results:

When we use {locate} in the URL it's always respected.

When I was checking $routes->get('/menu/(:any)', 'DefaultController::menu/$1'):

  • my default accept language is en and I got this value when calling $this->request->getLocale()
  • when I changed Accept-Language value in my browser to it it also worked fine (I got it)
  • when I changed Accept-Language value to something not supported, like pl, I got the default value it

This is all working as expected. If I'm still missing something, please don't hesitate to provide another example so I can reproduce the error.

@tomaluca95
Copy link
Author

I managed to reproduce the issue running

  • composer create-project codeigniter4/appstarter ci-lang-test
  • cd ci-lang-test
diff --git a/app/Config/App.php b/app/Config/App.php
index b91c058..5f6a130 100644
--- a/app/Config/App.php
+++ b/app/Config/App.php
@@ -76,7 +76,7 @@ class App extends BaseConfig
 	| If false, no automatic detection will be performed.
 	|
 	*/
-	public $negotiateLocale = false;
+	public $negotiateLocale = true;
 
 	/*
 	|--------------------------------------------------------------------------
@@ -88,7 +88,7 @@ class App extends BaseConfig
 	| found, the first locale will be used.
 	|
 	*/
-	public $supportedLocales = ['en'];
+	public $supportedLocales = ['en','it','es','de'];
 
 	/*
 	|--------------------------------------------------------------------------
diff --git a/app/Config/Routes.php b/app/Config/Routes.php
index 56839ca..aaeca6f 100644
--- a/app/Config/Routes.php
+++ b/app/Config/Routes.php
@@ -20,7 +20,7 @@ $routes->setDefaultController('Home');
 $routes->setDefaultMethod('index');
 $routes->setTranslateURIDashes(false);
 $routes->set404Override();
-$routes->setAutoRoute(true);
+$routes->setAutoRoute(false);
 
 /**
  * --------------------------------------------------------------------
@@ -32,6 +32,23 @@ $routes->setAutoRoute(true);
 // route since we don't have to scan directories.
 $routes->get('/', 'Home::index');
 
+$routes->get('/logout', 'Home::index');
+
+
+$routes->group('/admin',function(\CodeIgniter\Router\RouteCollection $routes) {
+    $routes->get('', 'Home::index');
+});
+$routes->group('/{locale}/admin', function(\CodeIgniter\Router\RouteCollection $routes) {
+    $routes->get('', 'Home::index');
+});
+
+$routes->get('/code/(:any)', 'Home::index');
+$routes->get('/menu/(:any)', 'Home::index');
+
+$routes->get('/{locale}', 'Home::index');
+$routes->get('/{locale}/menu/(:any)', 'Home::index');
+
+
 /**
  * --------------------------------------------------------------------
  * Additional Routing
diff --git a/app/Controllers/Home.php b/app/Controllers/Home.php
index 8798cdd..0c3996a 100644
--- a/app/Controllers/Home.php
+++ b/app/Controllers/Home.php
@@ -4,7 +4,7 @@ class Home extends BaseController
 {
 	public function index()
 	{
-		return view('welcome_message');
+		return $this->request->getLocale()."\n";
 	}
 
 	//--------------------------------------------------------------------
  • curl -H "Accept-Language: es" http://localhost:8080/menu/id1
  • curl -H "Accept-Language: es" http://localhost:8080

I'm thinking that the issue is with the $routes->group with the {locale} route

@michalsn
Copy link
Member

Okay, now I see it. It's not related to the $routes->group but just to the {locale} itself. If the route with {locale} is defined before "normal" route it will be "remembered" as $localeSegment in the Router::checkRoutes() will not have the opportunity to be reset.

@michalsn michalsn reopened this Jul 25, 2020
@michalsn michalsn mentioned this issue Jul 25, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

2 participants