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

RoutingContext.normalizedPath not decoding as documented #2708

Closed
ia3andy opened this issue Jan 22, 2025 · 6 comments
Closed

RoutingContext.normalizedPath not decoding as documented #2708

ia3andy opened this issue Jan 22, 2025 · 6 comments
Assignees
Milestone

Comments

@ia3andy
Copy link
Contributor

ia3andy commented Jan 22, 2025

Version

4.5.11 and current master

Context

I encountered an this issue which looks suspicious while working on Quarkus, here is the downstream issues:
quarkusio/quarkus#45775

Do you have a reproducer?

I created a failing test:
ia3andy@f3b111e

@ia3andy ia3andy added the bug label Jan 22, 2025
@ia3andy
Copy link
Contributor Author

ia3andy commented Jan 22, 2025

It seems the doc of this method has changed, now it is:

  /**
   * Normalizes a path as per <a href="http://tools.ietf.org/html/rfc3986#section-5.2.4>rfc3986</a>.
   *
   * There are 2 extra transformations that are not part of the spec but kept for backwards compatibility:
   *
   * double slash // will be converted to single slash and the path will always start with slash.
   *
   * Null paths are normalized to {@code /}.
   *
   * @return normalized path
   */
  String normalizedPath();

I don't know if the RFC spec states that it should decode.

Here is the previous doc for this which was more clear on the decoding part (but not working either AFAIK):

  /**
   * Return the normalized path for the request.
   * <p>
   * The normalized path is where the URI path has been decoded, i.e. any unicode or other illegal URL characters that
   * were encoded in the original URL with `%` will be returned to their original form. E.g. `%20` will revert to a space.
   * Also `+` reverts to a space in a query.
   * <p>
   * The normalized path will also not contain any `..` character sequences to prevent resources being accessed outside
   * of the permitted area.
   * <p>
   * It's recommended to always use the normalized path as opposed to {@link HttpServerRequest#path()}
   * if accessing server resources requested by a client.
   *
   * @return the normalized path
   */
  String normalizedPath();

@ia3andy
Copy link
Contributor Author

ia3andy commented Jan 22, 2025

Ok it seems it should decode it if it follows the spec: https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.2.2

@tsegismont tsegismont self-assigned this Jan 22, 2025
@tsegismont tsegismont added this to the 4.5.12 milestone Jan 22, 2025
@tsegismont
Copy link
Contributor

Ok it seems it should decode it if it follows the spec: https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.2.2

Section 6.2.2.2 reads:

These URIs should be normalized by decoding any percent-encoded octet that corresponds to an unreserved character, as described in Section 2.3

Section 2.3 reads:

Characters that are allowed in a URI but do not have a reserved purpose are called unreserved. These include uppercase and lowercase letters, decimal digits, hyphen, period, underscore, and tilde.

 unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"

So space is not an unreserved character

@tsegismont tsegismont removed this from the 4.5.12 milestone Jan 22, 2025
@tsegismont tsegismont added invalid and removed bug labels Jan 22, 2025
@tsegismont tsegismont closed this as not planned Won't fix, can't repro, duplicate, stale Jan 22, 2025
@ia3andy
Copy link
Contributor Author

ia3andy commented Jan 22, 2025

@tsegismont then try the same request with a space :)

(400 BAD REQUEST)

@tsegismont
Copy link
Contributor

For the record, the doc issue is known since at least 3.8.5 #1549

I will backport the docfix to the 4.x branch

@tsegismont
Copy link
Contributor

Doc fixed by #2711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants