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

[Translator] avoid serialize unserializable resources. #14705

Merged
merged 1 commit into from Jun 5, 2015

Conversation

Projects
None yet
5 participants
@aitboudad
Copy link
Contributor

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
@xabbuh

This comment has been minimized.

Copy link
Member

commented May 20, 2015

@aitboudad Can we add a test for this to avoid future regressions?

@xabbuh xabbuh added the Translator label May 20, 2015

@aitboudad aitboudad force-pushed the aitboudad:translation_serialize_fix branch from 047ec82 to 8b448dc May 22, 2015

@aitboudad

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2015

@xabbuh test added.

@aitboudad aitboudad added the Ready label May 22, 2015

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]);

This comment has been minimized.

Copy link
@fabpot

fabpot May 25, 2015

Member

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])

This comment has been minimized.

Copy link
@aitboudad

aitboudad May 25, 2015

Author Contributor

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 ??

This comment has been minimized.

Copy link
@fabpot

fabpot May 26, 2015

Member

But using spl_object_hash is much better. The reuse for an object that is also a resource is probably a very small risk

This comment has been minimized.

Copy link
@xabbuh

xabbuh May 31, 2015

Member

Using spl_object_hash() means that we get different paths on each request, doesn't it?. Isn't that an issue?

This comment has been minimized.

Copy link
@aitboudad

aitboudad May 31, 2015

Author Contributor

right, I reverted that part as I think it should be handled by config cache instead of serializing resources.

@aitboudad aitboudad removed the Ready label May 25, 2015

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 4, 2015

ping @iamluc

@stof

This comment has been minimized.

Copy link
Member

commented Jun 5, 2015

👍

@fabpot

This comment has been minimized.

Copy link
Member

commented Jun 5, 2015

Thank you @aitboudad.

@fabpot fabpot merged commit 7220f2c into symfony:2.7 Jun 5, 2015

1 of 2 checks passed

fabbot.io Not Found
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

fabpot added a commit that referenced this pull request Jun 5, 2015

bug #14705 [Translator] avoid serialize unserializable resources. (ai…
…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.

@aitboudad aitboudad deleted the aitboudad:translation_serialize_fix branch Jun 5, 2015

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.