-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Translator] avoid serialize unserializable resources. #14705
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
Conversation
aitboudad
commented
May 20, 2015
Q | A |
---|---|
Bug fix? | yes |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Fixed tickets | #14689 |
Tests pass? | yes |
License | MIT |
69758b5
to
047ec82
Compare
@aitboudad Can we add a test for this to avoid future regressions? |
047ec82
to
8b448dc
Compare
@xabbuh test added. |
if (isset($this->resources[$locale])) { | ||
foreach ($this->resources[$locale] as $resource) { | ||
if (is_object($resource[1]) && !$resource[1] instanceof \Serializable) { | ||
$resource[1] = get_class($resource[1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is relevant, but if we are looking for something unique here, it might be better to use spl_object_hash($resource[1])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me too, spl_object_hash($resource[1])
doesn't solve the issue because when object is destroyed, its hash may be changed.
I think it's better to removing that part until we found the right way ??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But using spl_object_hash
is much better. The reuse for an object that is also a resource is probably a very small risk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using spl_object_hash()
means that we get different paths on each request, doesn't it?. Isn't that an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, I reverted that part as I think it should be handled by config cache instead of serializing resources.
8b448dc
to
7220f2c
Compare
ping @iamluc |
👍 |
Thank you @aitboudad. |
…tboudad) This PR was merged into the 2.7 branch. Discussion ---------- [Translator] avoid serialize unserializable resources. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Fixed tickets | #14689 | Tests pass? | yes | License | MIT Commits ------- 7220f2c [Translator] avoid serialize unserializable resources.