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

Smarty include relative path with 2.0.1 behaviour changed #331

Closed
ophian opened this issue Mar 19, 2015 · 23 comments

Comments

Projects
None yet
6 participants
@ophian
Copy link
Member

commented Mar 19, 2015

to need strict ./ in themes.
http://board.s9y.org/viewtopic.php?f=5&t=20321 and s9y/additional_themes@7c2d995
Old {include file="example.tpl"} fails, new {include file="./example.tpl"} works.

What might have changed (when), which could influence this behaviour in Smarty?

It must be something regarding the template fetch "scope", like the stricter set of serendipity_getTemplateFile() for backend chainings we have touched. (i actually can`t see why this could influence it though)

Any ideas?

@ophian ophian added 2.0 bugs labels Mar 19, 2015

@ophian ophian added this to the 2.0.x milestone Mar 19, 2015

@garvinhicking

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

Don't know, I think this should be smarty related and not something in s9y itself... {include} is a fully internal smarty command, there's no s9y logic in there...?!

@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

I know, but it could be influenced, if the template scope path is not strict and maybe is an array in wrong order or something...

@garvinhicking

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

Hm. Maybe you could ask in the smarty forums on either if they know of an intentional change to this, or they can help debugging/where to look?

@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

I am pretty sure this would have had come up in the smarty forum, since 3.1.21 is "pretty" old. As I said already, I can't remember to have read something related in the smarty forum the last months. When we really do not know where to search for, I will certainly ask for it.
If you know where this is reproducible, enabling debugging at https://github.com/s9y/Serendipity/blob/master/include/functions_smarty.inc.php#L946 ff could help. I once did test this regarding a question by YL with an own include "something" (in 2k11 I think) and could not reproduce this behaviour.

@yellowled

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

could not reproduce this behaviour

That is actually the irritating factor in this. We have had two or three occurrences of this reported as far as I remember, and we have never been able to reproduce it. Smells like some (template) cache thing to me, since I think it always occurred in updated blogs and was tried to reproduce in test environments that were already set up using 2.0.

@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

Yeah, right! Maybe it has to do with the compiled template cleanup on upgrade, which was increased also... (so this would not happen with more current checkouts...?)

@garvinhicking

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

It was reproducible on blog.s9y.org which uses a 2.0.1 github checkout. It occured with completely fresh templates that were never used before.

It is also easily reproducible on any of my installations. Simply edit i.e. index.tpl of 2k11, add a:

{include file="test.tpl"}

and then create test.tpl with dummy content.

It will create a fatal PHP error:

Fatal error: Uncaught --> Smarty: Unable to load template file 'test.tpl' in '/www/cvs/serendipity/recent/templates/2k11/index.tpl' <-- thrown in /home/garvin/cvs/serendipity/git/2.0/Serendipity/bundled-libs/Smarty/libs/sysplugins/smarty_internal_templatebase.php on line 129

I am using symlinks for my installation (blog.s9y.org does as well). Maybe that's an issue, but I don't know if @donchambers has the same setup. I currently have no access to any install that doesn't use symlinks, so I can't test this with a non-symlinked install right now.

Maybe Smarty checks the "realdir" of a file, and gets confused if that path is not in the template list (where the symlinked, not realdir, is configured). In my oppinion that would be a bug with Smarty; maybe it can be circumvented with a smarty option like "use symlinks" or whatever)

@donchambers

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

I'm not using symlinks, and as @garvinhicking mentioned, he can easily reproduce the problem on any of his installations. Did we change smarty versions between s9y 1.7.8 and 2.x? Because this problem does NOT exist in 1.7.8.

@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

I am sorry, I cannot reproduce this! In my local setup with a current Serendipity v.2.0.1 in index.tpl

{include "mytest.tpl"}[1]
{include file="mytest.tpl"}[2]

mytest.tpl with <h2>mytest.tpl</h2>

Both works in templates/next/index.tpl.
Of course you get an error, if you have misspelled the include file.

@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

The realdir path thing may be a valid issue though.

@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

I think it must be the not correctly purged compiles thing, maybe having changed path to relpath with Smarty 3.1.21 update, for Don, YL and liebeszeit and the realdir path thing for symlinked directories users like Garvin.
I for myself cannot reproduce this in any tpl file of a template though.
It would be interesting for the three mentioned, when they had changed the errored include to use ./ instead to get out of this exception, what would happen if they just revert their changes, since they now are on current 2.0.x versions. ?!?

@yellowled

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

when they had changed the errored include to use ./ instead to get out of this exception

I did not do that, I just added the contents of the included file (Piwik snippet) to my index.tpl.

@donchambers

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

maybe having changed path to relpath with Smarty 3.1.21 update, for Don, YL and liebeszeit

What exactly do you mean by that?

Here is what I did....

  • Site running 1.7.8.
  • Upgraded to 2.0.1.
    • did NOT purge templates_c
    • observed file not found error
    • modified to {include file="./filename.tpl"}
    • this modification corrected file not found error
  • reverted to 1.7.8
    • did NOT purge templates_c
    • noted that modification of "./"filename.tpl also works in 1.7.8 (as it should - "./" is the current dir)
    • reverted to {include ="filename.tpl"}
    • observed that it continues to work in 1.7.8
    • re-upgraded to 2.0.1
    • file not found error returned
  • re-upgraded to 2.0.1.
    • did NOT purge templates_c
    • again observed file not found error
    • again modified to {include file="./filename.tpl"}
    • again - this modification corrected file not found error
@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

I am not too sure you actually see a live result, since that is a compile time process. (Upgrading should purge the compiles itself, but this was buggy and was fixed by onli with a 2.0-beta version.)

But my question simply was: now that you are on 2.0.1 and you change back to the open relative path, then purge the compiles via maintenance section, what does it say after recompiling?

@donchambers

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

But my question simply was: now that you are on 2.0.1 and you change back to the open relative path, then purge the compiles via maintenance section, what does it say after recompiling?

It worked! I removed "./" leaving {include file="filename.tpl"}, saved, purged templates_c, and it now works again as before.

So, what do we do going forward? Do we use file="./filename.tpl" because it is the same as file="filename.tpl" or do we need to do something about recompiling/purging templates_c?

@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

I does not hurt to use it the strict way for the public. But before getting busy on that I would think to wait for 2.0.2 and see if that happens again to you, now that you are using the open one.

@donchambers

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

Did something change from 2.0 - > 2.0.1 that I should expect to change again in 2.0.2?

I am a little uncomfortable waiting to see "if something happens again"... do we have any idea why it happened? It didn't happen only to me. Garvin modified several themes because of this issue.

@ophian

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2015

No. Garvins issue is a symlinked issue, which is something different!

@donchambers

This comment has been minimized.

Copy link
Member

commented Mar 19, 2015

I guess from this point forward I will use ./filename.tpl just to avoid any potential conflict, but will report back here when 2.0.2 is released to see if it happens again for my one demo.

@ophian ophian removed their assignment Mar 19, 2015

@yellowled

This comment has been minimized.

Copy link
Member

commented Jun 13, 2015

What about this? Is it a Smarty issue? Is it an issue at all?

@yellowled

This comment has been minimized.

Copy link
Member

commented Dec 21, 2015

Xmas issue cleanup – this issue has been open, but without interaction for a while now. Is it still an issue? Has anyone tested or reproduced it? Should/can we close it?

@onli

This comment has been minimized.

Copy link
Member

commented Jan 15, 2016

Seems like this is no issue

@onli onli closed this Jan 15, 2016

@igoltz

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2017

years later...
I updated a s9y (symlinked) installation from 1.7.8 to 2.0.5 to 2.1.1 and hit this bug(?) in blogs using the kinetic theme.

Prefixing the tpl files included tpl files with ./ solves it. But digging deeper through enabling $serendipity['smarty']->testInstall(); in functions_smarty.inc.php I get

    [template_dir:Smarty:private] => Array
        (
            [0] => /var/www.serendipity/html/jugendkurse/templates/default/
            [1] => /var/www.serendipity/html/jugendkurse/templates/2k11/
            [2] => /var/www.serendipity/html/jugendkurse/templates/2k11/
            [3] => /var/www.serendipity/html/jugendkurse/plugins/
            [4] => /var/www.serendipity/html/jugendkurse/templates/default/
        )

which does not include the template path.
Looking at serendipity_smarty_class.inc.php (from version 2.1.1) shows that the path $template_dirs[] = $serendipity['serendipityPath'] . $serendipity['templatePath'] . $serendipity['template']; is only included when $template_engine is not definded. However it is set to "default".
If I patch serendipity_smarty_class.inc.php so that the templatePath is always included all the kintetic based blogs work again without modifying the tpl files. Non kinetic blogs work too.
As I have no knowledge of s9y interna my question is: is this patch a bad hack? Why is the template path not included, which would make sense I guess.

--- serendipity_smarty_class.inc.php.org	2017-04-09 09:35:14.000000000 +0200
+++ serendipity_smarty_class.inc.php.igo	2017-06-09 15:26:09.922142184 +0200
@@ -131,6 +131,7 @@
             // this is tested without need actually, but it makes the directory setter fallback chain a little more precise
             $template_dirs[] = $serendipity['serendipityPath'] . $serendipity['templatePath'] . $serendipity['template'];
         }
+        $template_dirs[] = $serendipity['serendipityPath'] . $serendipity['templatePath'] . $serendipity['template'];
         $template_dirs[] = $serendipity['serendipityPath'] . $serendipity['templatePath'] . $serendipity['defaultTemplate'];
         $template_dirs[] = $serendipity['serendipityPath'] . $serendipity['templatePath'] . $serendipity['template_backend'];

    [template_dir:Smarty:private] => Array
        (
            [0] => /var/www.serendipity/html/jugendkurse/templates/default/
            [1] => /var/www.serendipity/html/jugendkurse/templates/kinetic_jugendkurse2/
            [2] => /var/www.serendipity/html/jugendkurse/templates/2k11/
            [3] => /var/www.serendipity/html/jugendkurse/templates/2k11/
            [4] => /var/www.serendipity/html/jugendkurse/plugins/
            [5] => /var/www.serendipity/html/jugendkurse/templates/default/
        )`

igoltz added a commit to igoltz/Serendipity that referenced this issue Jun 13, 2017

th-h added a commit to th-h/Serendipity that referenced this issue Aug 2, 2017

Add template path as first entry to template_dirs.
Fixes s9y#331 and s9y#516.

Cherry-picked from master.

Signed-off-by: Thomas Hochstein <thh@inter.net>

robelix added a commit to robelix/Serendipity that referenced this issue Mar 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.