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

Fix #1885: Check return value before creating dynamic class. #1887

Merged
merged 3 commits into from
Jul 18, 2019

Conversation

davidofferman
Copy link
Contributor

Hello!

In raising this pull request, I confirm the following:

  • I have checked that another pull request for this purpose does not exist
  • I wrote some tests for this PR
  • I updated the CHANGELOG

Small description of change:
Code to check the $classEntry value before calling the method Zephir\BaseBackend::initObject() inside of Zephir\Operators\Other\NewInstanceOperator.

Without this check, it's possible that NULL could be passed through and end up causing a segment fault. Happens when auto-loading a class with a syntax error.

Thanks

@davidofferman
Copy link
Contributor Author

I'll try to think of a better unit-test so we can pass the travis-ci build. Worst case scenario I can just remove the php extension from the file.

Give me a few days to get around to this unless you have something in mind.
Thanks.

@sergeyklay
Copy link
Contributor

Anyway you can exclude any file/directory from code style testing using https://github.com/phalcon/zephir/blob/development/.php_cs.dist

@davidofferman
Copy link
Contributor Author

Thanks for that information. I'll keep that in mind for the future.

The latest commit resolves the problem and now passes the automated tests and works as expected.

@sergeyklay sergeyklay merged commit 2ba6306 into zephir-lang:development Jul 18, 2019
@sergeyklay
Copy link
Contributor

Thank you

dreamsxin pushed a commit to dreamsxin/zephir that referenced this pull request Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants