Skip to content

Conversation

@ronodhirSoumik
Copy link
Contributor

@ronodhirSoumik ronodhirSoumik commented Nov 14, 2025

No description provided.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 14, 2025
@ronodhirSoumik ronodhirSoumik force-pushed the bugfix/fixed-npe-in-FilterChainProxy branch from 539fa3e to c8bd4d4 Compare November 14, 2025 19:39
Comment on lines 338 to 339
String requestMethod = request.getMethod();
return requestMethod != null && this.method.name().equals(requestMethod);
Copy link
Contributor

@ngocnhan-tran1996 ngocnhan-tran1996 Nov 15, 2025

Choose a reason for hiding this comment

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

We don’t need to check for null because this.method.name() will return a String type and String#equals(null) will return false, so the test passes without any changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but this fixes the NPE case - ig that is our expectation of getting false for invalid

Copy link
Contributor

Choose a reason for hiding this comment

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

this.method.name().equals(request.getMethod()) will return false if request.getMethod() == null and won’t throw an NPE unless request is null or method is null. The test passes without any changes.

Image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh got it - thanks for the explanation

@rwinch
Copy link
Member

rwinch commented Nov 21, 2025

Thank you for your submission @ronodhirSoumik

As pointed out, the test will pass without changes to the code. Additionally, the issue you are linking to is caused by ConcurrentHashMap not supporting a null key (which is of type ServletContext) and is unrelated to the method being null.

I think that the test would still provide value by ensuring that we do not get a NullPointerException later. If you would modify the PR to just have the test and update the PR subject, I can merge it.

@rwinch rwinch added status: waiting-for-feedback We need additional information before we can continue in: build An issue in the build in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 21, 2025
@rwinch rwinch self-assigned this Nov 21, 2025
@ronodhirSoumik ronodhirSoumik force-pushed the bugfix/fixed-npe-in-FilterChainProxy branch from c8bd4d4 to e292d4c Compare November 22, 2025 15:30
Signed-off-by: Soumik Sarker <ronodhirsoumik@gmail.com>
@ronodhirSoumik ronodhirSoumik force-pushed the bugfix/fixed-npe-in-FilterChainProxy branch from d8a7001 to d3e138c Compare November 23, 2025 16:16
@ronodhirSoumik ronodhirSoumik changed the title Fixed NPE for unknown request in FilterChainProxy Added test scope for handling NPE scenario in RequestMethod Nov 24, 2025
@ronodhirSoumik
Copy link
Contributor Author

@rwinch I have updated the PR with only test file (also unlinked the previous issue number) - plz review

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: build An issue in the build in: web An issue in web modules (web, webmvc) status: feedback-provided Feedback has been provided

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants