Skip to content

Fix: Routing::loadRoutes() in windows do not validate correctly $routesFiles - #7930

Merged
kenjis merged 1 commit into
codeigniter4:developfrom
pjsde:fix-routing
Sep 15, 2023
Merged

kenjis merged 1 commit into
codeigniter4:developfrom
pjsde:fix-routing

Conversation

@pjsde

@pjsde pjsde commented Sep 12, 2023

Copy link
Copy Markdown
Contributor

This occours because the argument definition is string $routesFile = APPPATH . 'Config/Routes.php' (using an slash /) and in the constructor $this->routeFiles is initialized with realpath($routesFile).

When validating if $routesFile exist in the $routeFiles array it do not exist, so add it to the $routeFiles[] duplicating the path.

expected:

$routeFiles = [
    0 => 'D:\projects\tests\app\Config\Routes.php',
];

result:

$routeFiles = [
    0 => 'D:\projects\tests\app\Config\Routes.php',
    1 => 'D:\projects\tests\app\Config/Routes.php',
];

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Comment thread system/Router/RouteCollection.php Outdated
@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Sep 12, 2023
@MGatner

MGatner commented Sep 12, 2023

Copy link
Copy Markdown
Member

PHPStan
1687 Short ternary operator is not allowed.

Since when? That doesn't seem right.

@samsonasik

Copy link
Copy Markdown
Member

@MGatner that ignored message to be removed from baseline

Comment thread system/Router/RouteCollection.php Outdated
@pjsde
pjsde force-pushed the fix-routing branch 2 times, most recently from 00b4aff to 193e14d Compare September 14, 2023 08:48
@kenjis
kenjis merged commit 689c933 into codeigniter4:develop Sep 15, 2023
@kenjis

kenjis commented Sep 15, 2023

Copy link
Copy Markdown
Member

@pjsde Thank you!

@pjsde
pjsde deleted the fix-routing branch September 16, 2023 11:10
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 this pull request may close these issues.

5 participants