Skip to content
This repository

Check if 404_override controller is in a subfolder #939

Closed
wants to merge 3 commits into from

5 participants

pickupman Matt Soltani Dekadence Will Power Andrey Andreev
pickupman

If $route['404_override'] is set to a controller that is in a subfolder it will not be found. This changeset will also check if controller filename exists in application/controllers/class/class.php.

Per the user guide, controllers maybe placed in a subfolder of the same name inside of the application/controllers folder.

pickupman

I refactored some more of the code to match the first two segments to see if it can find the class/method using the pattern show in the user guide.
404_override routes like will now load as:
products -> application/controllers/products.php (method: index)
products/sitemap ->application/controllers/(products/)products.php (method: sitemap)
products/shoes -> application/controllers/products/shoes.php (method: index)
products/shoes/sitemap -> application/controllers/products/shoes.php (method: sitemap)

Matt Soltani

I agree with CroNiX. I actually also added a pull request fixing this, but just used a redirect to the path specified in the 404_override parameter. This was because HMVC can involve routing to be done foreach module as well, so for instance a module has config/routes.php, plus it also adds a more flexibility. But I closed it since a redirection is not sufficient and you have no chance of making a fall back if the specified path does not exist, resulting in an endless loop.

pickupman

I know there is HMVC lite in one of the branches, but until that is merged into the develop branch the 404 controller path should be the same as all other controllers or documented as such. When that feature becomes available this code will need one more path check to for the controller.
I stumbled upon this by a lengthy forum thread trying to help a user figure out why their 404 controller was not being called. It was because of this issue of the controllers being organized in subfolders, CI was not finding it /application/controllers.

Matt Soltani

But what about checking if config/routes.php exists in the subfolder, and if so check if there is a route for this controller/method? This way there is no need to update the code when the feature becomes avaliable. Also, people using this already will have a benifit of this - I know I would.
I noticed you count the segments of the path and cut off if it's more than 2. Instead, you could add a limit to your explode.
Also, how come your last commit code is in the "Load the local application controller" section?

pickupman

The code only changes the behaviour of the the default 404 override. The routes are already handled by code above this block. If a routes file is being loaded from a subfolder then that should be handled in that section of code. This block is for finding the controller file from was is determined by the Router class.
The last commit is still in the same block around line 248 and the explode needs the third segment in case a folder/class/method is given the 404 route. I removed the third value after referencing it so it didn't cause any issues later. The block of code is being added to that section because that is where the error is thrown by CI if the file path cannot be determined.

Dekadence

This bug persists :S

Dekadence

With commits 1530187 and 6217e72 work fine, thanks @pickupman

Will Power

Ignore what I said, I copied it wrong. Thanks for this, hope it gets committed soon!

Andrey Andreev narfbg referenced this pull request from a commit
Andrey Andreev narfbg URI Routing overhaul
 - Allow multiple levels of controller directories (supersedes PRs #390, #2439)
 - Add support for per-directory 'defaul_controller' and '404_override' (resolves issue #2611; supersedes PR #939)
 - Fixed a bug where default_controller was called instead of triggering 404 if the current route is inside a directory
 - Removed a few calls from CI_Router to CI_URI that made a necessity for otherwise internal CI_URI methods to be public:

    - Removed CI_URI::_fetch_uri_string() and moved its logic into CI_URI::__construct()
    - Removed CI_URI::_remove_url_suffix, CI_URI::_explode_segments() and moved their logic into CI_URI::_set_uri_string()
    - Removed CI_URI::_reindex_segments() altogether ( doesn't need further manipulation, while  is
      public anyway and can be properly (and more effectively) replaced on the spot)
30d5324
Andrey Andreev
Collaborator

Please test this: 30d5324

Andrey Andreev narfbg closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 24 additions and 2 deletions. Show diff stats Hide diff stats

  1. +24 2 system/core/CodeIgniter.php
26 system/core/CodeIgniter.php
@@ -240,7 +240,29 @@ function &get_instance()
240 240 // If this include fails it means that the default controller in the Routes.php file is not resolving to something valid.
241 241 if ( ! file_exists(APPPATH.'controllers/'.$RTR->fetch_directory().$RTR->fetch_class().'.php'))
242 242 {
243   - show_error('Unable to load your default controller. Please make sure the controller specified in your Routes.php file is valid.');
  243 + // Check if 404_override controller is in a subfolder named different than parent folder
  244 + if (file_exists(APPPATH.'controllers/'.$RTR->fetch_class().'/'.$RTR->fetch_method().'.php'))
  245 + {
  246 + $segments = explode('/',$RTR->routes['404_override']);
  247 + // If we have more than 3 segments we need just two
  248 + if (count($segments) > 2)
  249 + {
  250 + $RTR->method = $segments[2];
  251 + array_pop($segments);
  252 + }
  253 + $RTR->directory = $segments[0].'/';
  254 + $RTR->class = $segments[1];
  255 + }
  256 + // Check if 404_override controller is in a subfolder named same as the class
  257 + elseif (file_exists(APPPATH.'controllers/'.$RTR->fetch_class().'/'.$RTR->fetch_class().'.php'))
  258 + {
  259 + $RTR->directory = $RTR->fetch_class().'/';
  260 + }
  261 + else
  262 + {
  263 + show_error('Unable to load your default controller. Please make sure the controller specified in your Routes.php file is valid.');
  264 + }
  265 +
244 266 }
245 267
246 268 include(APPPATH.'controllers/'.$RTR->fetch_directory().$RTR->fetch_class().'.php');
@@ -383,4 +405,4 @@ function &get_instance()
383 405 $EXT->call_hook('post_system');
384 406
385 407 /* End of file CodeIgniter.php */
386   -/* Location: ./system/core/CodeIgniter.php */
  408 +/* Location: ./system/core/CodeIgniter.php */

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.