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

Fix ArrayIndexOutOfBoundsException in XorCsrfTokenRequestAttributeHan… #14902

Closed
wants to merge 0 commits into from

Conversation

kratosmy
Copy link

To fix gh-#13310, I make a hotfix to ensure that System.arraycopy() won't cause a ArrayIndexOutOfBoundsException.

@pivotal-cla
Copy link

@kratosmy Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@kratosmy Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 13, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Apr 18, 2024

Thanks, @kratosmy! Will you please add a unit test that would fail without your change and pass with your change in place?

@jzheaux jzheaux self-assigned this Apr 18, 2024
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 18, 2024
@kratosmy
Copy link
Author

Thanks, @kratosmy! Will you please add a unit test that would fail without your change and pass with your change in place?

sure!let me add it

@kratosmy
Copy link
Author

Thanks, @kratosmy! Will you please add a unit test that would fail without your change and pass with your change in place?

Hi, @jzheaux , I think it is hard to write a unit test for this change because it is regarding to a call to System.arraycopy(), which is a base native function in the middle of a static function.

@jzheaux
Copy link
Contributor

jzheaux commented Apr 25, 2024

@kratosmy, I think it's okay to proceed without mocking System.arraycopy. It seems like a test where the csrf byte array is longer than the random byte array will suffice. Does that help you to proceed?

@kratosmy
Copy link
Author

@kratosmy, I think it's okay to proceed without mocking System.arraycopy. It seems like a test where the csrf byte array is longer than the random byte array will suffice. Does that help you to proceed?

Sure will do.

@kratosmy
Copy link
Author

@jzheaux Hi, I have opened another PR (#14976) using a new branch name to align with contribution guideline.

@kratosmy
Copy link
Author

And I also fix the same code in another package: org.springframework.security.messaging.web.csrf, please kindly help review, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants