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: Using placeholder (:any) before {locale} in routes cause incorrect $localeSegment value. #5997

Closed
kpeu3u opened this issue May 14, 2022 · 7 comments · Fixed by #6003
Closed
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@kpeu3u
Copy link

kpeu3u commented May 14, 2022

PHP Version

7.4

CodeIgniter4 Version

4.1.9

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

macOS

Which server did you use?

apache

Database

MySQL 5.6

What happened?

Incorrect $localeSegment when route is set as: $routes->get('(:any)/lang/{locale}', 'Language::index');

Using placeholder in routes cause problem in getting:

$localeSegment = array_search('{locale}', preg_split('/[\/]*((^[a-zA-Z0-9])|\(([^()]*)\))*[\/]+/m', $key), true);

at line 375.
The $localeSegment suppose to be 3 but returned value is 2

Steps to Reproduce

$routes->get('(:any)/lang/{locale}', 'Language::index');

URL: https://domain.name/test/lang/bg

App Config:

public $defaultLocale = 'en';
public $supportedLocales = ['en', 'bg',];

Expected Output

expect to set locale as 'bg'

Anything else?

No response

@kpeu3u kpeu3u added the bug Verified issues on the current code behavior or pull requests that will fix them label May 14, 2022
@kpeu3u kpeu3u changed the title Bug: Bug: Using placeholder (:any) in routes cause incorrect $localeSegment value. May 14, 2022
@kpeu3u kpeu3u changed the title Bug: Using placeholder (:any) in routes cause incorrect $localeSegment value. Bug: Using placeholder (:any) before {locale} in routes cause incorrect $localeSegment value. May 14, 2022
@iRedds
Copy link
Collaborator

iRedds commented May 14, 2022

Can't reproduce

public $defaultLocale = 'en';
public $supportedLocales = ['en', 'uk', 'bg'];

$routes->get('(:any)/lang/{locale}', fn () => service('request')->getLocale());

test/lang/uk -> uk
test/lang/bg -> bg
test/lang/aaa -> en

(:any)/lang/{locale}
  0     1      2  

@MGatner
Copy link
Member

MGatner commented May 14, 2022

@iRedds Did you test on 4.1.9?

@iRedds
Copy link
Collaborator

iRedds commented May 14, 2022

@MGatner yep

@kpeu3u
Copy link
Author

kpeu3u commented May 14, 2022

Let me post my specific case and some debugging results:
Routes.php

$routes->group('emenu', ['namespace' => 'Emenu\Controllers'], static function ($routes) {
    $routes->get('/', 'Home::index');
    $routes->get('(:any)/lang/{locale}', 'Language::index');
});

App.php

    /**
     * --------------------------------------------------------------------------
     * Default Locale
     * --------------------------------------------------------------------------
     *
     * The Locale roughly represents the language and location that your visitor
     * is viewing the site from. It affects the language strings and other
     * strings (like currency markers, numbers, etc), that your program
     * should run under for this request.
     *
     * @var string
     */
    public $defaultLocale = 'bg';

    /**
     * --------------------------------------------------------------------------
     * Negotiate Locale
     * --------------------------------------------------------------------------
     *
     * If true, the current Request object will automatically determine the
     * language to use based on the value of the Accept-Language header.
     *
     * If false, no automatic detection will be performed.
     *
     * @var bool
     */
    public $negotiateLocale = false;

    /**
     * --------------------------------------------------------------------------
     * Supported Locales
     * --------------------------------------------------------------------------
     *
     * If $negotiateLocale is true, this array lists the locales supported
     * by the application in descending order of priority. If no match is
     * found, the first locale will be used.
     *
     * @var string[]
     */
    public $supportedLocales = ['bg', 'en', 'es', 'ru', 'fr', 'de', 'gr', 'ro'];

Call url: https://domain.dev/emenu/pod-vishnata/lang/en

Debug result:

Router.php:

 // Reset localeSegment
            $localeSegment = null;

            $key = $key === '/'
                ? $key
                : ltrim($key, '/ ');

            $matchedKey = $key; **"emenu/(.*)/lang/{locale}"**

            // Are we dealing with a locale?
            if (strpos($key, '{locale}') !== false) {
                $localeSegment = array_search('{locale}', preg_split('/[\/]*((^[a-zA-Z0-9])|\(([^()]*)\))*[\/]+/m', $key), true); **2**

                // Replace it with a regex so it
                // will actually match.
                $key = str_replace('/', '\/', $key);
                $key = str_replace('{locale}', '[^\/]+', $key); **"emenu\/(.*)\/lang\/{locale}"**
            }

IncomingRequest.php:

 /**
     * Sets the locale string for this request.
     *
     * @return IncomingRequest
     */
    public function setLocale(string $locale) **$locale: "lang"**
    {
        // If it's not a valid locale, set it
        // to the default locale for the site.
        if (! in_array($locale, $this->validLocales, true)) {
            $locale = $this->defaultLocale; **sets default locale back to bg**
        }

        $this->locale = $locale;
        Locale::setDefault($locale);

        return $this;
    }

@iRedds
Copy link
Collaborator

iRedds commented May 17, 2022

Now I confirm that such a bug exists.
A possible solution is to use a named group to get the locale.

@MGatner
Copy link
Member

MGatner commented May 17, 2022

@kpeu3u Have you tested this against the develop branch? There have been so many unreleased changes that 4.1.9 is hard to judge anymore.

@kpeu3u
Copy link
Author

kpeu3u commented May 17, 2022

@MGatner I haven't test against develop branch, i just checked line $localeSegment = array_search('{locale}', preg_split('/[\/]*((^[a-zA-Z0-9])|\(([^()]*)\))*[\/]+/m', $key), true); for regular expression changes. Changes in method that determine locale segment refers to variable name changes only.

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

Successfully merging a pull request may close this issue.

3 participants