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

[ClassLoader] Fix ClassCollectionLoader inlining with declare(strict_types=1) #19859

Merged
merged 1 commit into from Sep 6, 2016

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Sep 5, 2016

Q A
Branch? 2.7
Bug fix? yes
Tests pass? yes
Fixed tickets #19845
License MIT

See diff as https://github.com/symfony/symfony/pull/19859/files?w=1

@fabpot
Copy link
Member

fabpot commented Sep 5, 2016

So, basically, you prevent files with strict types to be inlined (via a require), is that correct?

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Sep 5, 2016

I "inline" them by doing a straight "require" yes. We can't exclude them, because they might be parent classes of non strict classes.

throw new \RuntimeException(sprintf('Class Collection Loader was not able to create directory "%s"', $cacheDir));
}
$cacheDir = rtrim(realpath($cacheDir), '/'.DIRECTORY_SEPARATOR);
$cache = $cacheDir.DIRECTORY_SEPARATOR.$name.$extension;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole section does not look related to the fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is: these are some lines moved from the bottom to the before the below loop.
https://github.com/symfony/symfony/pull/19859/files?w=1

@fabpot
Copy link
Member

fabpot commented Sep 6, 2016

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 647b3d2 into symfony:2.7 Sep 6, 2016
fabpot added a commit that referenced this pull request Sep 6, 2016
…are(strict_types=1) (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[ClassLoader] Fix ClassCollectionLoader inlining with declare(strict_types=1)

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| Tests pass?   | yes
| Fixed tickets | #19845
| License       | MIT

See diff as https://github.com/symfony/symfony/pull/19859/files?w=1

Commits
-------

647b3d2 [ClassLoader] Fix ClassCollectionLoader inlining with declare(strict_types=1)
This was referenced Sep 7, 2016
@nicolas-grekas nicolas-grekas deleted the class-strict branch September 9, 2016 08:41
@fabpot fabpot mentioned this pull request Oct 3, 2016
nicolas-grekas added a commit that referenced this pull request Nov 15, 2016
…lt_compiler (giosh94mhz)

This PR was squashed before being merged into the 2.7 branch (closes #20455).

Discussion
----------

[ClassLoader] Fix ClassCollectionLoader inlining with __halt_compiler

| Q             | A
| ------------- | ---
| Branch?       | 2.7+
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | |
| License       | MIT
| Doc PR        | |

I have some PHP classes which ends with an `__halt_compiler` call which fail on production.

This is due to the ClassCollectionLoader inlining the class code (which is followed by random binary data) which breaks PHP parsing of the following classes.

I found this issue while using ZendGuardLoader module, but this apply to whatever php class using this function call.

I've made my fix, based on #19859, and tested it with symfony 2.7 and 2.8

Commits
-------

a60cd15 [ClassLoader] Fix ClassCollectionLoader inlining with __halt_compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants