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

fix: routing behavior when $uriProtocol is QUERY_STRING #7199

Merged
merged 10 commits into from
Feb 7, 2023

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jan 30, 2023

Description
From https://forum.codeigniter.com/showthread.php?tid=86284&pid=406192#pid406192

When Config\App::$uriProtocol is QUERY_STRING and accessing baseURL:

  • (1) Fix: the defined route / is ignored, and routed to the default controller
  • (2) Fix: Debug/Toolbar/Collectors/Routes.php throws "ReflectionException: Class "Home" does not exist"

How to Reproduce

--- a/app/Config/App.php
+++ b/app/Config/App.php
@@ -59,7 +59,7 @@ class App extends BaseConfig
      *
      * WARNING: If you set this to 'PATH_INFO', URIs will always be URL-decoded!
      */
-    public string $uriProtocol = 'REQUEST_URI';
+    public string $uriProtocol = 'QUERY_STRING';
 
     /**
      * --------------------------------------------------------------------------
diff --git a/app/Config/Routes.php b/app/Config/Routes.php
index c251ec22c4..520078ff91 100644
--- a/app/Config/Routes.php
+++ b/app/Config/Routes.php
@@ -29,7 +29,7 @@ $routes->set404Override();
 
 // We get a performance increase by specifying the default
 // route since we don't have to scan directories.
-$routes->get('/', 'Home::index');
+$routes->get('/', 'Index::index');
 
 /*
  * --------------------------------------------------------------------

(1) set CI_ENVIRONMENT = production in .env, and run spark serve.
Navigate to http://localhost:8080/.
You will see the Welcome page. But it should be 404 Not Found.
Because Index::index does not exist.

(2) set CI_ENVIRONMENT = development in .env, and run spark serve.
Navigate to http://localhost:8080/.

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

@kenjis kenjis marked this pull request as draft January 30, 2023 05:07
@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 30, 2023
Comment on lines 69 to 73
public function testPathRequestURINested()
{
$this->request->uri = '/ci/index.php/woot?code=good#pos';
// /ci/index.php/woot?code=good#pos
$_SERVER['REQUEST_URI'] = '/index.php/woot';
$_SERVER['SCRIPT_NAME'] = '/index.php';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what is Nested (in the method name), and why
the URI path is /ci/index.php/woot?code=good#pos but $_SERVER['REQUEST_URI'] is /index.php/woot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure either! Was that my code? It probably just means "in a subdirectory".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not your code.

@kenjis kenjis marked this pull request as ready for review February 2, 2023 07:16
@kenjis kenjis merged commit 315f8c6 into codeigniter4:develop Feb 7, 2023
@kenjis kenjis deleted the fix-uriProtocol-QUERY_STRING branch February 7, 2023 10:12
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.

2 participants