Let OpenSslCipherMethodsProvider contribute its own result cache meta - #6338
Conversation
|
I don't love this solution. It'd make more sense to me if the extension implemented ResultCacheMetaExtension too. |
b9962c2 to
40d9ba6
Compare
|
Both addressed, thanks. The provider implements There is a test as well, I also dropped the mbstring fingerprint from the first version. I could not find an environment where |
just curios: how did you find out this is a problem? which problem you experienced triggered the PR? |
40d9ba6 to
489fcc1
Compare
|
Agreed on both, removed. The sha1 one was re-implementing the production code in the assertion, and the key one only asserted a literal that a deliberate rename would update anyway. What is left is the two that pin behaviour: a different set of ciphers has to produce a different hash, and the order the runtime reports them in must not. Both still error without the change.
Nothing in production, so worth being straight about that. It came out of the result cache sharing thread you started. I was going through what actually ends up in the cache metadata to understand why a CI-built cache cannot be reused on a developer machine, and noticed that That could have been theoretical, so I measured it before writing anything: a probe across the runners and docker images, printing the cipher and encoding hashes per environment. The ciphers differ at the same So: found by reading, confirmed by measuring, not by being bitten. If you would rather wait until someone actually reports a stale type, that is a fair call. |
489fcc1 to
1585c96
Compare
|
do we have the same problem with e.g. |
1585c96 to
31d485d
Compare
|
Good question, and it turned out to be worth measuring rather than reasoning about. Same shape, but not reachable. The difference is that the algorithm list does not actually vary. I re-ran the environment probe with
60 algorithms with the same fingerprint everywhere, not just at a given While I was in there I swept for the same pattern across Also applied your other note: |
it seems there is a api to register new hash algos from a php extension. I think depending on php-extensions implementation details algos might show up and disappear. |
31d485d to
ed57f30
Compare
fb1cf35 to
a22cb36
Compare
The provider builds its list of supported ciphers by asking openssl_get_cipher_methods() and then probing every algorithm with openssl_cipher_iv_length(), and the inferred type follows it: openssl_cipher_iv_length() is int for a supported algorithm and false for an unsupported one. That list is a property of the PHP build rather than of the PHP version, so two hosts on the same PHP_VERSION_ID disagree. Measured across CI runners and docker images: PHP 8.4.25, ubuntu-latest, OpenSSL 3.0.13 -> 212 ciphers PHP 8.4.25, macos-latest, OpenSSL 3.6.3 -> 208 ciphers The four missing on macOS are aes-128-cbc-hmac-sha1, aes-128-cbc-hmac-sha256, aes-256-cbc-hmac-sha1 and aes-256-cbc-hmac-sha256, so openssl_cipher_iv_length() for one of them is int on the first and false on the second. phpExtensions records extension names only, so the metadata is identical and the cache is reused across the difference. Implementing ResultCacheMetaExtension on the provider puts the fingerprint where the knowledge already lives, and it costs nothing for projects that never touch openssl, since the list is only computed when something asks for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
a22cb36 to
0e6182a
Compare
|
The API is exported, so you're right that an extension can add algos: Two things narrow it though:
What is left is narrower than the openssl case: an extension that keeps its name and changes its algo set across its own versions or ini settings, or a patched build of ext/hash. The meta key holds extension names, not versions, so that would slip through. I couldn't find a published extension that calls the API, which is also why the probe saw one fingerprint across all 14 environments. Agreed on leaving it out of this PR. If a real case turns up, the fix is the same |
|
thank you |
OpenSslCipherMethodsProviderbuilds its list of supported ciphers by askingopenssl_get_cipher_methods()and then probing every algorithm withopenssl_cipher_iv_length(), and the inferred type follows it:openssl_cipher_iv_length()isintfor a supported algorithm andfalsefor an unsupported one.That list is a property of the PHP build rather than of the PHP version, so two hosts on the same
PHP_VERSION_IDdisagree. Measured across CI runners and docker images:PHP_VERSION_IDAt PHP 8.4.25 on both sides, ubuntu-latest has four ciphers macos-latest does not:
aes-128-cbc-hmac-sha1,aes-128-cbc-hmac-sha256,aes-256-cbc-hmac-sha1,aes-256-cbc-hmac-sha256. Soopenssl_cipher_iv_length('aes-256-cbc-hmac-sha1')isinton one andfalseon the other, at the same patch version.phpExtensionsrecords extension names only, so the metadata is identical and the cache is reused across the difference.Changed since the first version
Both review points, thank you.
ResultCacheMetaExtensioninstead ofResultCacheManagerreaching out to it. That is clearly the better home: the fingerprint sits where the knowledge already lives,ResultCacheManageris untouched, and thebuild/baseline-8.0.neonentry the first version needed is gone. The diff went from two files and 60 lines to one file and 25.OpenSslCipherMethodsProviderTestcovers that the key is stable, that a different set of ciphers produces a different hash, and that the order the runtime reports them in does not. It errors on all four cases without the change.I also dropped the mbstring half of the first version. I could not find any environment where
mb_list_encodings()differs at a given PHP version, so it was unmotivated; only the openssl list is evidenced.Checks
make tests(21266 tests, 96360 assertions),make phpstanand phpcs on both touched files pass. Also verified end to end by patching the provider to drop one cipher: without this change the restored cache still reportedint, with it the metadata no longer matches and the file is re-analysed tofalse.