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

Parser crashes when encountering property call inside string #17

Closed
nevadascout opened this issue Jun 17, 2016 · 11 comments
Closed

Parser crashes when encountering property call inside string #17

nevadascout opened this issue Jun 17, 2016 · 11 comments
Assignees
Labels
Milestone

Comments

@nevadascout
Copy link
Contributor

nevadascout commented Jun 17, 2016

So I have some code: echo "test: $this->prop->foo".

When trying to parse this code, the parser crashes without an error, however adding { } around the property call resolves the issue and it parses successfully.

So echo "test: {$this->prop->foo}" works.

Even if this behaviour is not allowed in PHP, the parser should throw an exception so it can be caught on the user side. Currently this code crashes my application without warning, regardless if I use a try/catch block or not.

@ichiriac
Copy link
Member

Hi @nevadascout,

Seems to work with this :

Z:\php-parser\bin>node test.js -e "echo ""test: $this->prop"""

*** START TESTING ***

-- TOKENS :
T_ECHO " T_ENCAPSED_AND_WHITESPACE T_VARIABLE T_OBJECT_OPERATOR T_STRING "
-- AST :
[ 'program',
  [ [ 'sys',
      'echo',
      [ [ 'bin',
          '.',
          [ 'string', 'test: ' ],
          [ 'prop', [ 'var', '$this' ], [ 'string', 'prop' ] ] ] ] ] ] ]

But not with this :

Z:\php-parser\bin>node test.js -e "echo ""test: $this->prop->foo"""

*** START TESTING ***

-- TOKENS :
T_ECHO " T_ENCAPSED_AND_WHITESPACE T_VARIABLE T_OBJECT_OPERATOR T_STRING T_OBJECT_OPERATOR T_STRING "
Z:\php-parser\src\parser.js:218
        throw new Error(this.lastError.message);
              ^
Error: Parse Error : unexpected T_OBJECT_OPERATOR, expecting T_VARIABLE, T_CURLY_OPEN, T_DOLLAR_OPEN_CURLY_BRACES, T_ENCAPSED_AND_WHITESPACE at line 1
    at Object.api.error (Z:\php-parser\src\parser.js:218:15)
    at Object.api.expect (Z:\php-parser\src\parser.js:319:18)
    at Object.read_encapsed_string_item (Z:\php-parser\src\parser\scalar.js:142:14)
    at Object.read_encapsed_string (Z:\php-parser\src\parser\scalar.js:171:39)
    at Object.read_scalar (Z:\php-parser\src\parser\scalar.js:54:32)
    at Object.read_expr_item (Z:\php-parser\src\parser\expr.js:270:21)
    at Object.read_expr (Z:\php-parser\src\parser\expr.js:12:23)
    at Object.api.read_list (Z:\php-parser\src\parser.js:375:28)
    at Object.read_statement (Z:\php-parser\src\parser\statement.js:187:35)
    at Object.read_top_statement (Z:\php-parser\src\parser\statement.js:60:23)

It should pass and I'll fix it, but for me the error behaviour is normal, it's throwing an syntax error as expected. I can't reproduce the same crash error, could you isolate this behaviour in a self-contained js file ?

@nevadascout
Copy link
Contributor Author

Hey thanks for the quick response, I just realised that my example was wrong.

It should have been echo "test: $this->foo->prop"

@ichiriac
Copy link
Member

ichiriac commented Jun 17, 2016

ok, i've got it, my implementation of this grammar was too simplified :
https://github.com/php/php-src/blob/493524454d66adde84e00d249d607ecd540de99f/Zend/zend_language_parser.y#L1228

@nevadascout
Copy link
Contributor Author

Great! How easy is it to fix?

@ichiriac
Copy link
Member

it's easy, but i wait that travis do it's job :
https://travis-ci.org/glayzzle/php-parser/builds/138464108

Since I've validate tokens over php7 I'm compiling php from sources and it takes a long time but I'm confident it will pass

@ichiriac ichiriac reopened this Jun 17, 2016
@ichiriac
Copy link
Member

I'm re-openning this issue until we figure out the crash error

@nevadascout
Copy link
Contributor Author

I'll get you a code sample shortly

@ichiriac
Copy link
Member

@nevadascout
Copy link
Contributor Author

Nice one! Do you still want a sample of broken code?

@ichiriac
Copy link
Member

No, it's ok, maybe it's a problem with the silent error handler, if so it's not about encapsed strings parsing.

If you get another parsing problem with a crash you could open another issue.

I will close this issue, thanks for your reporting,

Best Regards,
Ioan

@nevadascout
Copy link
Contributor Author

Thanks for fixing it so quickly!

@ichiriac ichiriac added this to the 0.1.2 milestone Nov 23, 2016
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

2 participants