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

[Config] ConfigCache::isFresh() should return false when unserialize() fails #20671

Merged
merged 1 commit into from Nov 29, 2016

Conversation

Projects
None yet
3 participants
@nicolas-grekas
Copy link
Member

commented Nov 28, 2016

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

Removes some Warning: Class __PHP_Incomplete_Class has no unserializer failures when clearing the cache.

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:unser-safe branch 2 times, most recently from 36e4ec6 to 65b89c2 Nov 28, 2016

$meta = unserialize(file_get_contents($metadata));
$unserializeCallbackHandler = ini_set('unserialize_callback_func', '');
$signalingException = new \UnexpectedValueException();
set_error_handler(function () use ($signalingException) { throw $signalingException; });

This comment has been minimized.

Copy link
@stof

stof Nov 28, 2016

Member

what about other errors happening there due to bugs in the unserialize implementation of Serializable resources ? They would be completely swallowed currently. Shouldn't we try to detect only the case of undefined classes ?

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 29, 2016

Author Member

updated to return false only when hitting not found classes

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:unser-safe branch 3 times, most recently from c2a5991 to 09132bf Nov 29, 2016

@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:unser-safe branch from 09132bf to 609245e Nov 29, 2016

@nicolas-grekas nicolas-grekas merged commit 609245e into symfony:2.7 Nov 29, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Nov 29, 2016

bug #20671 [Config] ConfigCache::isFresh() should return false when u…
…nserialize() fails (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[Config] ConfigCache::isFresh() should return false when unserialize() fails

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

Removes some `Warning: Class __PHP_Incomplete_Class has no unserializer` failures when clearing the cache.

Commits
-------

609245e [Config] ConfigCache::isFresh() should return false on __PHP_Incomplete_Class

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:unser-safe branch Nov 29, 2016

@fabpot fabpot referenced this pull request Nov 30, 2016

Merged

Release v3.2.0 #20693

This was referenced Dec 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.