Skip to content

Mark matchingRequestParameterName as @Nullable in HttpSessionRequestCache and WebSessionServerRequestCache #19153

@ternbusty

Description

@ternbusty

Current Behavior

The package org.springframework.security.web.savedrequest is @NullMarked, so by contract setMatchingRequestParameterName(String) does not accept null.

https://github.com/spring-projects/spring-security/blob/8c4c5fe91f/web/src/main/java/org/springframework/security/web/savedrequest/HttpSessionRequestCache.java#L177-L179

However, the field is already null-checked at runtime. getMatchingRequest skips the query-parameter requirement entirely when the field is null.

https://github.com/spring-projects/spring-security/blob/8c4c5fe91f/web/src/main/java/org/springframework/security/web/savedrequest/HttpSessionRequestCache.java#L100-L111

So the implementation supports null, but the JSpecify type contract advertises non-null. The two disagree.

The reactive counterpart WebSessionServerRequestCache has a partial mismatch in the same direction. The field is already declared @Nullable String, but the setter parameter is not.

https://github.com/spring-projects/spring-security/blob/8c4c5fe91f/web/src/main/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCache.java#L63

https://github.com/spring-projects/spring-security/blob/8c4c5fe91f/web/src/main/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCache.java#L124-L126

Expected Behavior

Annotate the field and the setter parameter as @Nullable so the type contract matches the implementation. The change applies to HttpSessionRequestCache#matchingRequestParameterName (field), HttpSessionRequestCache#setMatchingRequestParameterName(String) (parameter), and WebSessionServerRequestCache#setMatchingRequestParameterName(String) (parameter; the field is already @Nullable).

The Javadoc should also state that passing null disables the query-parameter requirement, following the convention used by BasicAuthenticationEntryPoint#setCharset ("Set to null to ...").

Context

Aside from fixing the contract/implementation mismatch, this also legitimizes the only existing way to disable the ?continue optimization (introduced in #11454). The optimization is enabled by default (the field default is "continue") and reduces unnecessary HttpSession reads. However, appending ?continue to post-login redirect URLs can break client-side routing in SPAs. Applications hitting this currently have to call setMatchingRequestParameterName(null), which is supported at runtime but warns under JSpecify/NullAway.

Happy to send a PR.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions