Closed hostep closed 3 years ago
While I think we all love strict types (it's definitely nice to see this bubble non-compliant warnings up across the ecosystem), issuing semver non-compliant changes makes dependents' lives harder and should be avoided if possible.
If a change results in user programs breaking, it's a bug in the kernel. We never EVER blame the user programs.
Sorry folks, but this interface always required a string
as input.
On Fri, Jun 25, 2021, 15:50 Damien Retzinger @.***> wrote:
While I think we all love strict types (it's definitely nice to see this bubble non-compliant warnings up across the ecosystem), issuing semver non-compliant changes makes dependents' lives harder and should be avoided if possible.
3fbe665
diff-45a996ba81e262b6b47a807220ac2d11c1824463bcfb30d1d25a43af0642a113R3
If a change results in user programs breaking, it's a bug in the kernel. We never EVER blame the user programs.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/laminas/laminas-escaper/issues/21#issuecomment-868514103, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABFVEHAPLGWH3RNECUUUMDTUSCSXANCNFSM47J5LDFA .
It's definitely a bug in Magento, I totally agree, but Magento has a super slow release schedule and all Magento version that were released after 28 April 2020 (Magento 2.3.5 and higher) will crash when upgrading laminas/laminas-escaper
to 2.7.1
Hence the request to revert the changes and only introduce them in a new major version.
It's the quickest way of fixing Magento shops, besides telling all Magento developers not to upgrade laminas/laminas-escaper
to 2.7.1
You can lock onto a prior release until this is fixed in upstream.
On Fri, Jun 25, 2021, 15:54 Pieter Hoste @.***> wrote:
It's definitely a bug in Magento, I totally agree, but Magento has a super slow release schedule and all Magento version that were released after 28 April 2020 (Magento 2.3.5 and higher) will crash when upgrading laminas/laminas-escaper to 2.7.1
Hence the request to revert the changes and only introduce them in a new major version.
It's the quickest way of fixing Magento shops, besides telling all Magento developers not to upgrade laminas/laminas-escaper to 2.7.1
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/laminas/laminas-escaper/issues/21#issuecomment-868516844, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABFVEBHUJGSWFZB3O3QGQ3TUSDCJANCNFSM47J5LDFA .
@weierophinney, @ghostwriter, can we get a second opinion here please? Thanks! 🙂
An alternative could be to add string
to the type declarations in
parameters: shouldn't affect subtypes as per PHP variance rule changes in
newer 7.x releases.
On Fri, Jun 25, 2021, 16:00 Pieter Hoste @.***> wrote:
@weierophinney https://github.com/weierophinney, @ghostwriter https://github.com/ghostwriter, can we get a second opinion here please? Thanks! 🙂
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/laminas/laminas-escaper/issues/21#issuecomment-868520629, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABFVEBRCUKHILCIP7DR5BLTUSDXXANCNFSM47J5LDFA .
Closing as duplicate of #20 meanwhile.
@hostep I'm with @Ocramius on this one. Pin to an earlier version in your application, or wait for the fix to bubble into the Magento source code. While code calling these methods might have worked previously, that code was still calling it incorrectly. Had we done any actual checking for types manually in the code to ensure the values we received were actually strings, we would have been throwing our exceptions; now we rely on the engine to do this. At some point, this was going to break for callers using invalid values.
Note: adding string
type declarations would also be somewhat mitigate this, if the caller-side does not use strict types.
Note: adding
string
type declarations would also be somewhat mitigate this, if the caller-side does not use strict types.
So would that mitigate the issue? That would cast any int to string in your function, so the integer values from Magento would be converted to a string automatically?
Thanks!
BC Break Report
Summary
See https://github.com/magento/magento2/issues/33346 & https://github.com/magento/magento2/pull/33353
Magento sometimes send a
bool
instead of astring
toEscaper::escapeUrl
This worked fine in version 2.7.0, but now no longer in 2.7.1 due to the addition ofdeclare(strict_types=1);
Request is to revert your changes and release 2.7.2 and re-introduce those changes in a new major version: 3.0.0
Does this makes sense?
Thanks! 🙂
Previous behavior
See above
Current behavior
See above
How to reproduce
See above