Deterministic proxy names #25978
Deterministic proxy names #25978
Conversation
| @@ -108,7 +108,7 @@ public function getProxyCode(Definition $definition) | |||
| */ | |||
| private function getProxyClassName(Definition $definition) | |||
| { | |||
| return preg_replace('/^.*\\\\/', '', $definition->getClass()).'_'.substr(hash('sha256', spl_object_hash($definition).$this->salt), -7); | |||
| return preg_replace('/^.*\\\\/', '', $definition->getClass()).'_'.substr(hash('sha256', serialize($definition).$this->salt), -7); | |||
There was a problem hiding this comment.
I just looked at the current logic. It looks like this would be enough isn't it? (with lazy prefix to easy recognizing them):
return 'lazy'.preg_replace('/^.*\\\\/', '', $definition->getClass()).'_'.substr(hash('sha256', $definition->getClass()), -7);
There was a problem hiding this comment.
Indeed, that should be enough. The only scenario where this could break is if we would have the same class name with a different shape in one process. That is probably achievable through runkit and the like but I guess that’s a case we can ignore.
…ct_hash() which is somewhat random
|
@nicolas-grekas not sure what to do about the resource tracking topic. Probably worth another PR? |
|
@lstrojny we currently don't invalidate the cached container when the class changes, meaning the generated proxy class might not be regenerated properly when needed. |
|
I fixed resource tracking in #25981 |
| @@ -108,7 +108,7 @@ public function getProxyCode(Definition $definition) | |||
| */ | |||
| private function getProxyClassName(Definition $definition) | |||
| { | |||
| return preg_replace('/^.*\\\\/', '', $definition->getClass()).'_'.substr(hash('sha256', spl_object_hash($definition).$this->salt), -7); | |||
| return preg_replace('/^.*\\\\/', '', $definition->getClass()).'_'.substr(hash('sha256', $definition->getClass().$this->salt), -7); | |||
There was a problem hiding this comment.
Does this mean we only have a maximum of one proxy per class? 🤔
There was a problem hiding this comment.
does it mean anything to have more that one proxy per class? do you have an example?
There was a problem hiding this comment.
you will get only one proxy class per class. You can still have multiple instance.
If you look at the code, the only part of the Definition impact the generated class is the class name. So multiple definitions can reuse the same proxy class safely.
There was a problem hiding this comment.
You are right, this makes sense :)
|
Thank you @lstrojny. |
This PR was merged into the 3.4 branch. Discussion ---------- Deterministic proxy names Proxy class names should be deterministic and independent of spl_object_hash() which is somewhat random to better support reproducible builds. See #25958 | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- b173b81 Proxy class names should be deterministic and independent of spl_object_hash() which is somewhat random
Proxy class names should be deterministic and independent of spl_object_hash() which is somewhat random to better support reproducible builds. See #25958