Critical bug: back-compatibility broken: Key cannot be empty

[aprokopenko]

Hi,

After updating my composer today - all my application is broken, because of update in your package.

We have Laravel and https://github.com/PHP-Open-Source-Saver/jwt-auth/releases package for JWT auth. This package has your package as a dependency.

After latest release I now just get fatal error like this (didn't do anything at all, just update the composer and downloaded your fresh package):

This is critical!

[aprokopenko]

Version, which works fine: v4.1.5

New versions not working at all: v4.2.0, v4.2.1

[Slamdunk]

This is an intended security fix: after #833 you need InMemory::empty() to use empty keys

[mfn]

I've opened a similar issue related to the recent release -> #878

I guess more ppl will come forward now that the week just started.

[aprokopenko]

This is an intended security fix: after #833 you need InMemory::empty() to use empty keys

But this breaks back-compatibility! If you fixed the security fix and other package has to update their code - this is a MAJOR release according to SemVer! If you just follow SemVer - noone will be affected, because standard composer dependency looks like "^4.1" for example, which will download only bug fixes and features. If you update method signatures - this breaks back-compatibliity and is a MAJOR release

[Slamdunk]

But this breaks back-compatibility! If you fixed the security fix and other package has to update their code - this is a MAJOR release according to SemVer!

I disagree: it was a security bug, hence it has been considered a security bug fix.
BC breaks need to be addressed within the same MAJOR, if they are security bug fixes

[Ocramius]

Closing as per @Slamdunk's comments.

An insecure system is worse than a broken system. Better for it to be broken than compromised, and security takes priority over BC.

[lcobucci]

Just a longer version of @Ocramius' comment...

Introducing behavioural BC-breaks is never fun and it was not an arbitrary decision.
If you look at the statement we had in the documentation, we've tried to point users to the correct path but overlooked the critical details on the RFC, where they clearly request implementors to conform with minimum security guidelines to prevent people from having insecure systems.

I completely understand the sentiment and feel bad about forcing breakages to downstream packages. At the same, I feel responsible for providing abstractions that don't compromise security of end-users, which is why I decided to release the breakage as a minor version and not a patch (giving end-users a migration path) - even though it's a security issue.

When analysing all the trade-offs, I still feel like we've made the correct decision with these behavioural BC-breaks as they will lead to safer solutions across all channels. I really don't like the feeling of overlooking these details and not enforcing the constraints from day 0 but I'm also human and entitled to mistakes.

I look forward to helping the affected users of this package to find solutions and get their systems into the desired state.