Skip to content

Conversation

sairohith2605
Copy link

This PR introduces a tiny patch for the method StringUtils.pathEquals() that short-circuits into a false result if any or both of the parameters of path are null. This aims to prevent a NullPointerException when cleanPath() returns null when the supplied path String does not have any length.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 2, 2025
Signed-off-by: Sai Rohith Reddy <[email protected]>
@sairohith2605 sairohith2605 force-pushed the patch/nullsafe-pathequals branch from 2477431 to 6c6ee7f Compare October 2, 2025 16:40
Signed-off-by: Sai Rohith Reddy <[email protected]>
@sairohith2605 sairohith2605 marked this pull request as ready for review October 2, 2025 16:47
Signed-off-by: Sai Rohith Reddy <[email protected]>
@sbrannen sbrannen self-assigned this Oct 3, 2025
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Oct 3, 2025
@sbrannen
Copy link
Member

sbrannen commented Oct 3, 2025

Hi @sairohith2605,

Congratulations on submitting your first PR for the Spring Framework! 👍

Unfortunately, I need to decline the proposal for the following reasons.

  1. Supplying a null path to StringUtils.pathEquals(...) is not a supported use case.
  2. The org.springframework.util package (see package-info.java) is annotated with @NullMarked.
  3. Consequently, null is not a supported argument value unless a given parameter is annotated with @Nullable.
  4. Neither path1 nor path2 is annotated with @Nullable.

It is therefore the responsibility of the caller to ensure that pathEquals(...) is not invoked with a null argument.

Note that we rely on the NullAway build plugin for Gradle as well as IDE support to ensure that we do not supply a null value for one of those arguments. For example, in WebUtils you can see that we verify that both arguments are not null.

if (root == null) {
throw new IllegalStateException(
"Cannot set web app root system property when WAR file is not expanded");
}
String param = servletContext.getInitParameter(WEB_APP_ROOT_KEY_PARAM);
String key = (param != null ? param : DEFAULT_WEB_APP_ROOT_KEY);
String oldValue = System.getProperty(key);
if (oldValue != null && !StringUtils.pathEquals(oldValue, root)) {
throw new IllegalStateException("Web app root system property already set to different value: '" +
key + "' = [" + oldValue + "] instead of [" + root + "] - " +
"Choose unique values for the 'webAppRootKey' context-param in your web.xml files!");
}

In light of the above, I am closing this PR.

Thanks anyway

@sbrannen sbrannen closed this Oct 3, 2025
@sbrannen sbrannen added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants