Possible discrepancy in FileLocator between Windows and Linux

Hi all,

I think I stumbled upon a small discrepancy between the handling/output of the FileLocator but wanted to confirm if this is useful to be discussed on GitHub before posting there.

Summary
When trying to load a view, the use of '\' instead of '/' will load the correct file on Windows machines but throw a file not found exception on Linux systems (whereof the latter should be intended behaviour as far as I understand).

Test Case
Setup CodeIgniter 4.0.4 with everything working fine on a Windows developement environment (XAMPP).
Install lonnieezell\myth-auth latest developement version. ( This is not needed, as the error should be reproducible with any view, however, I want to tell the whole story here).
Publish myth-auth configuration and views via spark.
Rename view in published myth-auth config from 'App\Views\Auth\login' to 'Auth\Login'. (Please don't ask why, it was a stupid idea)

Expected Result
Upon trying to load the view, the FileLocator will check whether the requested file name is namespaced or not. As 'Auth\Login' appears to be namespaced it will search for the file that way but not find it and will throw a file not found exception.

Actual Result
Surprisingly, on Windows systems, the view file is still correctly located and displayed.
On the Linux production system the exception is thrown as expected.

Suggested Action
While I know that changing the view file path the way I did was just wrong, I think this might happen to other beginners, too (be it due to lack of caution or understanding or simple misspelling).
As far as I understand the principle of the FileLocator, the behaviour on the Windows machine is wrong here, as the view file should not be detected. I think this should be related to the different directory seperators on Windows/Linux, but would need to take a closer look where exactly the detection wrongly succeeds.
The problem I see with this discrepancy is (as it happened to me), that the system works nicely in the developement environment, but instantly crashes when pushed to production, such that I needed to do some debugging in production.

Do you think this is an issue to be adressed?

Cheers
Alex

Can you try also your test case using the latest develop branch of CI4? I believe many changes are added to FileLocator since 4.0.4. Please confirm if it is still a case.

I just tried the same behaviour with the current development branch, but the file still gets detected in the Windows environment.
Therefore I tried to trace the behaviour back a litte and found that the issue is not the FileLocator but rather the view renderer.

In 'View\View.php' on line 218

PHP Code:

$this->renderVars['file'] 


is set. This attribute is checked via 'is_file' and the file locator is only loaded if this check fails.

On Windows the results are:

• 'C:\XAMPP\htdocs\ci-ttv\app\Config/../Views/Auth\login.php' when using 'Auth\login' as the view name which evaluates as 'true' in 'is_file'.
  

• 'C:\XAMPP\htdocs\ci-ttv\app\Config/../Views/App\Views\Auth\login.php' when using 'App\Views\Auth\login' which evaluates as 'false' in 'is_file'. Hence, the FileLocator is triggered and finds the correct view from the namespace.
  

On Linux:

• The first case (using 'Auth\login' evaluates as 'false' in 'is_file' such that the FileLocator is called here to but cannot resolve the file, as 'Auth' is not a valid namespace. This should be the desired behavior.
  

My first impression is that the 'is_file' check on line 220 in the View class needs to be extended to not let namespaced values pass on windows (e.g. strings containing '\'). In the FileLocator, we have a

PHP Code:

strpos($file, '\\') === false 


test to check whether the filename is namespaced. Maybe this could be reused in the View here.

My impression is that is_file behaves differently on Linux and Windows. is_file will not care about namespaces as it treats its argument as a file path to check. The value $this->renderVars['file'] is composed of the concatenation of the default $viewPath and the view name passed to view(). The default viewPath value is technically APPPATH . 'Views' . DIRECTORY_SEPARATOR. If I will have the view written as view('Auth\login.php') then technically is_file should still return true as APPPATH . 'View/Auth\login.php' is a file (although the namespace inside is wrong and not conforming to PSR4).

So I believe Linux is wrong here. It should return is_file as true. Searching further the net, this may be caused by permissions?

I think I see your point, however, maybe we want to avoid this behaviour as it can lead to misinterpretations.
Here ist the result of the view->render() function for both environments:

Windows:
$view: Auth\login
$renderVars['file']: C:\XAMPP\htdocs\ci-ttv\app\Config/../Views/Auth\login.php
is_file: bool(true)

Debian:
$view: Auth\login
$renderVars['file']: /var/www/xxx/html/intern/app/Config/../Views/Auth\login.php
is_file: bool(false) (hence, FileLocator is called)

While I agree that the result of is_file is legit here, as Windows does not really care about mixed separators, I believe we should prohibit loading view this way. I think so because usually, if someone references a view in the form 'Auth\login', he is supposedly trying to address a namespaced view. If one want to adress a view by path, 'Auth/login' should be used. Thats why I think we should check whether a $view name is namespaced in the view render function instead of just relying on the is_file function.

Overall, I think its more an issue of philosophy that we want to project on the user, but as displayed in my case, unexperienced users might run into problems when pushing their systems to live environment.
What do you think?

Yes I see your point and I think we need to normalize things here. I don't have a Linux machine to test on so if you don't mind can you test my conjecture? Im thinking that if you will be writing the view name as Auth/login, then is_file will return true on Linux. If so then I think we need to have strtr replacements before the is_file check.

Okay, I did some further tests to cover most use cases I think (would be nice to have tables here ):

View name: 'Auth/login'
  Expected Result:   loaded via file path
  Windows:             loaded via file path
  Debian:                loaded via file path

View name: 'Auth\login'
  Expected Result:   File-not-Found-Exception
  Windows:             loaded via file path
  Debian:                File-not-Found-Exception

View name: 'App\Views\Auth\login'
  Expected Result:   loaded via FileLocator
  Windows:             loaded via FileLocator
  Debian:                loaded via FileLocator

View name: 'Myth\Auth\Views\login'
  Expected Result:   loaded via FileLocator
  Windows:             loaded via FileLocator
  Debian:                loaded via FileLocator

As long as nobody is using mixed separators in a single call, this should be it.

Like you said, we could strstr before calling is_file or I would suggest to change line 220 in View\View.php from

if (! is_file($this->renderVars['file']))

to

if (strpos($file, '\\') !== false && ! is_file($this->renderVars['file'])).

That is the way FileLocator checks if a filename is namespaced.
Thinking about is, the strpos check might even be executed at the beginning of the render function, as I think backslashed should always indicate a namespaced filename, in which case we would not need to waste time by checking the concatenated folder path before using FileLocator. Am I overseeing something here?