patch the Preview->render() method #998

Merged
merged 1 commit into from Feb 15, 2017

Projects

None yet

3 participants

@RickOrchard
Contributor

Ran into a 'Unable to open ...' error using the Template render method that was not generated using the View render method. There appeared to be a small inconsistency in parsing the UI global. So i added the small addition .';./' that was present on the View method, now they're both working fine.

@RickOrchard RickOrchard Update base.php
Ran into a 'Unable to open ...' error using the Template render method that was not generated using the View render method. There appeared to be a small inconsistency in parsing the UI global. So i added the small addition .';./' that was present on the View method, now they're both working fine.
8406e37
@bcosca bcosca merged commit e9a8e91 into bcosca:master Feb 15, 2017
@ikkez
Collaborator
ikkez commented Feb 16, 2017 edited

I think it should be solved the other way round and remove the ;./-addition to the UIvar in View and / Preview render method. I would be very surprised to see the template engine trying to load files from the project root directory, if nothing else was found. Since this is completly configurable with the UI var, we don't really need it here. IMO this addition could add potential security issues, which can be overseen fast. (like dynamic including of templates or such)

I didn't noticed this before, because I haven't used the raw View class a lot yet. But this should be removed there and not applied to the other template engines.

@RickOrchard
Contributor

Hi @ikkez thanks for your reply.

This addition just prepends the ./ in front of a given path, so that if the variable $file contains a template including its relative path, everything still works. How does that jeopardize app-security?

@ikkez
Collaborator
ikkez commented Feb 16, 2017

No this addition always appends ./ to the UI path, which is then exploded by f3->split

foreach (  $fw->split(  $fw->get('UI').';./'  )   as $dir) {

That means, if the file was not found within the UI path scope (with can be a split-able string), then it goes through another loop and check if a matching file can be found in the ./ folder, which is most likly the app root path or public web folder.

The actual solution to your problem is to set UI to a sub-folder like templates and do not put this path into your controllers where you render your templates - same for <include> within template files. Imagine the UI var is like mounting different template "drives" to the render engine. This ./ addition adds the another "drive" that is now used as fallback.

@RickOrchard
Contributor
RickOrchard commented Feb 17, 2017 edited

I see your point. In my case i call out to several different sub-templates in my main template, using their full relative path. But i guess i could do the same by hardcoding these paths in the UI variable. Either case, those two functions should work identically, dont you think...

@ikkez
Collaborator
ikkez commented Feb 17, 2017

those two functions should work identically, dont you think

Yes I think they should. I'll patch this so they use the "other" way.

@ikkez ikkez added a commit to bcosca/fatfree-core that referenced this pull request Feb 17, 2017
@ikkez ikkez remove base dir addition from UI on render, bcosca/fatfree#998 ea27848
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment