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

WebFlux Cookie Path with Trailing Slash Causes Inconsistent Behavior with Browser Path Matching #34091

Open
guqing opened this issue Dec 14, 2024 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team status: waiting-for-triage An issue we've not yet triaged or decided on

Comments

@guqing
Copy link

guqing commented Dec 14, 2024

Description

When using Spring WebFlux with a base-path configuration, such as /base, the session_id cookie's Path attribute is set to /base/ (with a trailing slash). However, when the browser accesses the path /base (without a trailing slash), the cookie is not sent, as browsers treat /base and /base/ as two different paths when matching cookies.

This results in the following issues:

  1. Inconsistent behavior with browser path matching rules
    Browsers consider /base and /base/ as distinct paths, but WebFlux defaults to setting the cookie Path to /base/, which causes the cookie not to be sent when accessing /base.

  2. Inconsistency with Spring MVC behavior
    In Spring MVC, the Path for the session_id cookie is set to the base-path without a trailing slash. This allows the cookie to be sent correctly for both /base and /base/ paths.

  3. Issues with accessing the base path
    When the base-path is configured, users expect that accessing /base will send the session cookie. However, due to the trailing slash in the cookie path, the session cookie is not sent unless the path includes the trailing slash, i.e., /base/.

Phenomenon Description

When the base-path is configured as /base, and session-based login is implemented, if a user is already logged in, accessing /base is treated as an unauthenticated state. This occurs because the browser does not send the session_id cookie to the backend when accessing /base. The reason is that the cookie's scope is set to /base/, and since the path /base does not match the cookie's path, the browser does not send the cookie, causing the backend to create a new session. However, when accessing paths like /base/**, the cookie's path matches, so the session_id cookie is sent correctly, and the backend recognizes the user as logged in.

Steps to Reproduce

  1. Configure application.yml:
    spring:
      webflux:
        base-path: /base
  2. Start the WebFlux application and access /base.
  3. The browser does not send the session_id cookie, but when accessing /base/, the cookie is sent.

Problem Analysis

The issue arises in the CookieWebSessionIdResolver when setting the session_id cookie. Specifically, the initCookie method in CookieWebSessionIdResolver adds a trailing slash to the Path attribute of the cookie:

private ResponseCookie.ResponseCookieBuilder initCookie(ServerWebExchange exchange, String id) {
    ResponseCookie.ResponseCookieBuilder builder = ResponseCookie.from(this.cookieName, id)
            .path(exchange.getRequest().getPath().contextPath().value() + "/")  // Trailing slash added here
            .maxAge(getCookieMaxAge())
            .httpOnly(true)
            .secure("https".equalsIgnoreCase(exchange.getRequest().getURI().getScheme()))
            .sameSite("Lax");

    if (this.initializer != null) {
        this.initializer.accept(builder);
    }

    return builder;
}

This causes the cookie path to be /base/ (with a trailing slash), which does not match when the browser accesses /base (without the slash).

Further Explanation

If the issue is addressed by redirecting requests from /base to /base/, this would make the paths match and the cookie would be sent. However, this approach conflicts with the default behavior of PathMatcher in WebFlux, which treats /base and /base/ as distinct paths. Automatically redirecting to /base/ would interfere with normal path matching behavior and could cause issues with routing in other parts of the application.

Expected Behavior

  1. WebFlux should allow developers to configure whether the cookie Path should include a trailing slash.
  2. Alternatively, WebFlux should behave like Spring MVC and not append a trailing slash to the base-path by default, preventing the path matching issue.

If I misunderstood, please correct me 🫡 and I look forward to your response♥️

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 14, 2024
@rstoyanchev
Copy link
Contributor

This was added in #20579 which was defined as setting the path to context path + "/", but there is no specific explanation for the trailing slash. @rwinch do you see any reason why that would matter, or can we change CookieWebSessionIdResolver to use the context path only?

@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jan 2, 2025
@rstoyanchev rstoyanchev self-assigned this Jan 2, 2025
@rstoyanchev rstoyanchev added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Jan 2, 2025
@rwinch
Copy link
Member

rwinch commented Jan 3, 2025

@rstoyanchev There are tradeoffs around adding the trailing slash vs not that are well explained by Tomcat's sessionCookiePathUsesTrailingSlash documentation:

Some browsers, such as Internet Explorer, Safari and Edge, will send a session cookie for a context with a path of /foo with a request to /foobar in violation of RFC6265. This could expose a session ID from an application deployed at /foo to an application deployed at /foobar. If the application deployed at /foobar is untrusted, this could create a security risk. However, it should be noted that RFC 6265, section 8.5 makes clear that path alone should not be view as sufficient to prevent untrusted applications accessing cookies from other applications. To mitigate this risk, this attribute may be set to true and Tomcat will add a trailing slash to the path associated with the session cookie so, in the above example, the cookie path becomes /foo/. However, with a cookie path of /foo/, browsers will no longer send the cookie with a request to /foo. This should not be a problem unless there is a servlet mapped to /*. In this case this attribute will need to be set to false to disable this feature. The default value for this attribute is false.

At the time I sent the Pull Request, the default value of sessionCookiePathUsesTrailingSlash was true which meant that the trailing slash was included. This meant that at that time the PR was consistent with Tomcat

Tomcat's default was changed in part as a response to this issue.

I'd probably pick performing a redirect from /context to /context/ before any application logic is invoked as this ensures that the session id is always seen and avoids leaking the session id. However, I'm not sure that there is a right or a wrong answer here, but rather a few options with tradeoffs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team status: waiting-for-triage An issue we've not yet triaged or decided on
Projects
None yet
Development

No branches or pull requests

4 participants