PHP globals passed by reference in hive() result

[xfra35]

The hive() method returns a copy of the F3's hive, except for PHP globals which are passed by reference:

$hive=$f3->hive();
echo $f3->get('SERVER.HTTP_HOST');//localhost
$hive['SERVER']['HTTP_HOST']='whatever';
echo $f3->get('SERVER.HTTP_HOST');//whatever

A consequence of this behaviour is that the HTML escaping performed in View & Template classes overwrites PHP globals (including session variables) as described by Andrew on Google groups.

[bcosca]

This is expected behavior of assigning values to PHP globals. It's a bug in the escaping and raw conversion that you discovered. Nice spot. Fixed in latest commit.

[xfra35]

Why is this expected behaviour? In the example above $hive is a copy of the original hive. Therefore, any modification on this copy should not affect the original hive.

The effect of commit fba00ea is to prevent escaping of PHP globals in views/templates. I don't think that's desired. Check this example:

<form method="post" action="">
<input type="text" name="var" value="{{@@POST.var}}"/>
<input type="submit" value="OK"/>
</form>

Potential double quotes in @POST.var are not escaped anymore..

[bcosca]

How would you synchronize $_POST and the framework's POST (so you can use them interchangeably) without using references then?

[xfra35]

Can't we force hive() to return a read-only copy of the original hive? That is, to replace references to PHP globals with values?

[bcosca]

hive() isn't the culprit. It's render() that's causing the trouble. Fixed.

[xfra35]

Now we're back to the original issue: PHP globals are modified at rendering time:

$f3->set('SESSION.str','a&b');
echo $f3->get('SESSION.str');//a&b
\Template::instance()->render('whichever-template.html');
echo $f3->get('SESSION.str');//a&b

I insist that this is linked to the fact that hive() returns direct references to PHP globals instead of values.

If hive() was returning a real copy of the hive, this would not happen. One way to force it to return values would be to pass it through a dummy function:

function hive() {
  return array_map(function($val){return $val;},$this->hive);
}

UPDATE: the modified function above doesn't exactly perform a deep copy of F3's hive. It works with SESSION.str but not with SERVER.HTTP_HOST which itself is a reference. If we want to perform a real deep copy of the hive, we need to use a recursion:

function hive() {
  return $this->aclone($this->hive);
}
function aclone($arr) {
  $clone=array();
  foreach($arr as $key=>$val)
    $clone[$key]=is_array($val)?$this->aclone($val):$val;
  return $clone;
}

That's becoming heavy..

[bcosca]

hive() should always return the hive itself, not a copy. Some plugins require this be passed by reference. It's render() that's at fault. Fixed (hopefully the last).

[ikkez]

hm.. no ... still fails. i tested now by myself.
i think the best would be to backup the GLOBALS before escaping the whole hive in the render function, and sync the references back into it afterwards. in case for the template class i altered the render function like this:

$globals_bkp = array();
foreach (explode('|','GET|POST|COOKIE|REQUEST|SESSION|FILES|SERVER|ENV') as $global)
  $globals_bkp[$global] = $fw->get($global);
$out = $this->sandbox($fw->get('ESCAPE')?
  $fw->esc($hive):$hive);
$fw->mset($globals_bkp);
return $out;

works good

[bcosca]

Shallow-copy of hive used in this third debug iteration.

[xfra35]

I'm a bit confused now. What's escaped, what's not, what's passed by reference, what's not..
Check out this new example:

  $obj=new myClass();
  $obj->prop='été';
  echo $obj->prop;//été
  $f3->set('OBJ',$obj);
  \Template::instance()->render('anything.html');
  echo $obj->prop;//été

[MINORITYmaN]

and what do get using raw at the end?

[xfra35]

It probably decodes the string but the example is there to underline the fact that template rendering overwrites the hive.

[ikkez]

what happens if we render two templates? Does it double encode the already encoded hive references?

[xfra35]

No because F3 uses htmlentities with the double_encode option set to false.

[bcosca]

Deep-copy of hive now implemented. It was a bit tricky because not only are PHP globals involved but user-defined objects too. Additionally, not all objects in PHP are cloneable, hence the (object)(array) typecasting. There's a test case now added to the code.

[xfra35]

It looks better but still fails with PHP globals:

$f3->set('string','<test>');
$obj=new \stdclass;
$obj->content='<ok>';
$f3->set('object',$obj);
$f3->set('SESSION.content',$obj->content);
$test->expect(
  $f3->get('string')=='<test>' &&
  $f3->get('object')->content=='<ok>' &&
  $tpl->render('templates/test12.htm')==
    '&lt;test&gt;&lt;ok&gt;' &&
  $f3->get('string')=='<test>' &&
  $f3->get('object')->content=='<ok>' &&
  $f3->get('SESSION.content')=='<ok>',//<=====  &lt;ok&gt;
  'Escaped values'
);

[bcosca]

Done

[KOTRET]

question: what happens to the object then? methods are removed? and what about visibility modificators (private / protected etc)

[xfra35]

@bcosca thanks it works now

[xfra35]

I mean it works for PHP globals. But after @KOTRET's question, I made a test with objects and it appears that since they are converted to stdClass, they become unuseable inside a template (loss of methods + private/protected are exposed):

$db=new \DB\SQL('etc..');
$dbm=new \DB\SQL\Mapper($db,'mytable');
$dbm->load();
echo $dbm->data;//"abcdef"
$f3->set('DBM',$dbm);
echo Template::instance()->render('test.html');

test.html:

{{@DBM->data}} //Undefined property: stdClass::$data
{{@DBM->fields.data.value}} //"abcdef"
{{@DBM->engine}} //"mysql"

[ikkez]

well, that is even more worse than the very first issue.
if we can't deref or copy objects, maybe we should consider of skipping them from default escaping (which probably introduces a security issue in backward compatibility). Or do we need to copy/factory the data-mappers? (and would that be good or bad). I also want to mention here, that there are cases, where neither using esc() nor raw() is optimal: i.e. if i want to render a template, that includes a markdown-rendered section, which includes a code block, you must not use esc or raw, as it would break the code-block or the whole markdown-html... the only way i could make it working was with an own custom <markdown> tag

[bcosca]

Well since it's not possible to clone all objects, we just simplify the code and clone whatever the framework can. This way templates still have access to the object's methods. But users should be mindful that uncloneable objects are always passed by reference.

[xfra35]

There's still a small issue: since ArrayAccess objects are not iterable, they should be treated like other objects in esc and raw functions. Otherwise they end up empty.

[KOTRET]

imho this is the wrong place to fiddle around with, for me the bug is in template.php and not in base.php. is there a reason why the whole escaping and duplicating is done in base?
To clearify for me: this is not done in template.php#L253 already!?

[ikkez]

@KOTRET: no this does only take effect when you set your template token like {{ @title | esc }}

[KOTRET]

yea, and what we are working on?

[bcosca]

@xfra35 Fixed

[bcosca]

@KOTRET

fatfree/lib/template.php

Line 253 in 48df64b

if (preg_match('/^(.+?)\h*\|\h*(raw|esc|format)$/',

only does the parsing.

fatfree/lib/template.php

Line 386 in 48df64b

return $this->sandbox($hive);

is where the action is.

[KOTRET]

don't understand why $hive gets escaped, passed to the sandbox where the built file includes echo Base::instance()->esc(...);. it is done twice?

[bcosca]

It's a recursive call.

[xfra35]

@bcosca I think you forgot to update the repository. You wrote "Fixed" but nothing has changed.

[bcosca]

@xfra35 I noticed when I pushed to the dev branch a few minutes ago