joerghoh commented on code in PR #101:
URL:
https://github.com/apache/sling-org-apache-sling-engine/pull/101#discussion_r4143375950
##########
src/main/java/org/apache/sling/engine/impl/request/RequestData.java:
##########
@@ -218,13 +218,31 @@ public Resource initResource(ResourceResolver
resourceResolver) {
StringBuffer requestURL = servletRequest.getRequestURL();
String path = request.getPathInfo();
- if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
+ if (requestURL.indexOf(";") > -1 && path != null &&
!path.contains(";")) {
+ // The container stripped path parameters (';...') from the path
info. Re-derive
+ // the path from the raw request URL to preserve them, but only
use the re-derived
+ // path if it is equivalent to the container provided path info
modulo the path
+ // parameters. Otherwise every upstream path based decision
(container
+ // normalization, authentication requirements, filter patterns,
access rules)
+ // would have been made on a different path than the one resolved
here.
try {
- final URL rUrl = new URL(requestURL.toString());
+ // java.net.URI#getPath decodes percent escapes, so the
derived path is in
+ // the same canonical (decoded) form as the container provided
path info
+ final String rawPath = new
URI(requestURL.toString()).getPath();
final String prefix =
request.getContextPath().concat(request.getServletPath());
- path = rUrl.getPath().substring(prefix.length());
- } catch (final MalformedURLException e) {
- // ignore
+ if (rawPath != null && rawPath.startsWith(prefix)) {
Review Comment:
The null check is redundant and I will remove it.
The ``startsWith`` check is not redundant, as the prefix can be different;
see this (obscure) example:
| Value | Result | Decoded? |
|---|---|---|
| `request.getContextPath()` | `/my%20app` | **No** — per
[spec](https://jakarta.ee/specifications/servlet/6.0/apidocs/jakarta.servlet/jakarta/servlet/http/httpservletrequest#getContextPath()),
the container does *not* decode this |
| `request.getServletPath()` | `""` (Sling registers at `/*`) | n/a |
| `request.getPathInfo()` | `/foo bar` (container strips `;v=1.1`) | Yes |
| `new URI(requestURL).getPath()` | `/my app/foo bar;v=1.1` | Yes —
`URI.getPath()` decodes the *whole* string uniformly |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
