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

Remove deprecated constantTimeEquals methods from CSRFTokenSigner #12519

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rtyley
Copy link
Contributor

@rtyley rtyley commented Apr 4, 2024

Purpose

This PR removes deprecated constantTimeEquals() methods on the Scala implementation of CSRFTokenSigner - these methods were deprecated with #6429 back in August 2016 with Play v2.6.

This PR does break binary compatibility (hence the failing test), because it's removing deprecated methods/objects.

There are no constantTimeEquals() or deprecated methods on the Java play.libs.crypto.CSRFTokenSigner implementation, so no methods changes are occurring there.

Background Context

I encountered these deprecated methods while addressing guardian/play-secret-rotation#445 and thought they may as well be removed.

I also opened this PR to see how receptive you'd be to PRs that break binary compatibility, because I'm interested in #12520!

@rtyley rtyley changed the title Remove deprecated methods on CSRFTokenSigner Remove CSRFTokenSigner methods deprecated by Play v2.6 Apr 4, 2024
@rtyley rtyley changed the title Remove CSRFTokenSigner methods deprecated by Play v2.6 Remove deprecated constantTimeEquals methods from CSRFTokenSigner Apr 4, 2024
@rtyley rtyley marked this pull request as ready for review April 4, 2024 16:33
Copy link
Member

@mkurz mkurz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The MiMa checks are failing. Can you please add the three needed MiMa filters to the already existing ones (appending)? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants