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

require statement breaks local variable scope #1621

Closed
zacek opened this issue Mar 2, 2018 · 9 comments
Closed

require statement breaks local variable scope #1621

zacek opened this issue Mar 2, 2018 · 9 comments
Assignees
Labels

Comments

@zacek
Copy link

zacek commented Mar 2, 2018

There is some problem in the code zephir generates for requirement statement in PHP 7. Local variable values overwrite global variables when require statement is used (the required file can be empty).

Minimalistic example is attached. The render method in the example extension class is inspired by Phalcon\Mvc\View\Engine::render(). Thus, this bug causes problems with volt template rendering in Phalcon.

Components versions

  • zephir builder: v1.1.2 (latest development version f3afc60 tested too)
  • zephir: 0.10.7 (latest development version 8059e66 tested too)
  • PHP: 7.0.25 (latest ubuntu 17.10 version 7.1.11 tested too)

How to reproduce:

  1. create simple extension (zephir init mira) and copy this mira.zep file in it:
namespace Mira;

class Engine
{
    public function render(string! templatePath, var params)
    {
	var key, value;

	if typeof params == "array" {
	    for key, value in params {
		let {key} = value;
	    }
	}

        require templatePath;
    }
}

build it (zephir build), install mira.so, create mira.ini and enable it (phpenmod mira).

  1. create file mira.php:
<?php
$engine = new \Mira\Engine();

$a = 1;
$engine->render('mira2.php', ['a' => 2]);

// expected output: $a == 1
print 'a == ' . $a;

and an empty file mira2.php

  1. run
    php mira.php
    The result displayed is 'a=2' which means that the $a variable has been overwritten in the Engine::render() call.

Note: If you comment out the line with the require statement in the Engine extension the variable $a is not affected.

All the example files can be found in the attached demo.zip.


Refs

@zacek
Copy link
Author

zacek commented Jul 16, 2018

I've added workaround for \Phalcon\Mvc\View\Engine\Volt to phalcon/cphalcon#12176

@sergeyklay
Copy link
Member

@zacek Could you please explain a bit more. Is there any code example?

@zacek
Copy link
Author

zacek commented Jul 16, 2018

This defect affects the Phalcon volt template engine - variables assigned in the template, e.g.

$this->view->param = 'value'; 

overwrite local $param variable when the page gets rendered.

As this defect seems to be quite difficult to fix I've published my workaround in the related defect for those who need the fix ASAP. The fix is just to move the problematic code (method render()) from Volt class in zephir to PHP. This seems to work properly. So whoever needs to use the volt templating and cannot wait for the fix can simply extend the Volt class as proposed in the workaround and use the extended class.

@zacek
Copy link
Author

zacek commented Jul 16, 2018

the code of the extended class is in my last comment to this defect: phalcon/cphalcon#12176
I've mentioned it here just for completeness as this defect (in zephir) is the root cause of the problem in the Volt class (in cphalcon).

@sergeyklay
Copy link
Member

As I know @AlexNDRmac tries to sort out in these days.

@Jurigag
Copy link
Contributor

Jurigag commented Aug 7, 2018

@AlexNDRmac any progress ?

@AlexNDRmac
Copy link
Member

Yes... I have some little progress. Now I trying to implement symbol table.

@dschissler

This comment was marked as abuse.

@sergeyklay
Copy link
Member

Fixed here:

Thank you for the report, and for helping us make Zephir better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants