-
Notifications
You must be signed in to change notification settings - Fork 468
pkp/pkp-lib#11975 Allow mix of blade/smarty templates #11980
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
base: main
Are you sure you want to change the base?
Conversation
ab8d72d to
35a5369
Compare
| $this->registerPlugin('function', 'load_menu', $this->smartyLoadNavigationMenuArea(...)); | ||
|
|
||
| // load a blade view in smarty templates | ||
| $this->registerPlugin('function', 'load_blade_view', $this->smartyLoadBladeView(...)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just last name adjustments - include_blade ? I think it helps to have expectation that it does work same as {include ...}, just loading blade template instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done .
| throw new Exception("Invalid file path: path traversal or absolute paths not allowed in given blade file {$file}"); | ||
| } | ||
|
|
||
| $file = str_replace(['/', '.blade.php', '.blade'], ['.', '', ''], $file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't be coding for .blade.php files anywhere; they should always be just .blade (at least until we move the codebase outside the web root).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asmecher this is not providing any support for .blade.php but sanitizing the path in proper format by removing any extension and / . But now that you mentioning this, I think rather than we sanitizing, we should be strict and throws exception so that developer is forced to maintain/pass proper path format .
@jardakotesovec do you think it will be too strict if we force developer to have like this
{ include_blade file='path.to.blade.template' params1=$params1 params2=$params2 ...}e.g. no / and no extension for file like we have for .tpl files. I was trying to be bit relax here but probably better to be strict here .
BTW, if we fully stop the support of .blade.php , we need to handled it in service provider and have throws exception with file extension that has .blade.php . I will see what I can do .
db765cd to
bea8182
Compare
for #11975 .