Jetty 9.4.x #6473 normalize only once #6474
Conversation
gregw
changed the title
lachlan-roberts
force-pushed
the
jetty-9.4.x-6473-normalize-only-once
branch
from
f6a0630
to
ab49940
Compare
| * <li>{@link #getParam()} - param</li> | ||
| * <li>{@link #getPath()} - /path;param1/%2e/info;param2</li> | ||
| * <li>{@link #getDecodedPath()} - /path/info</li> | ||
| * <li>{@link #getParam()} - param2</li> |
There was a problem hiding this comment.
Needs an explanation of why we don't return param1.
| */ | ||
| public static String canonicalPath(String path) | ||
| public static String canonicalURI(String path) |
There was a problem hiding this comment.
Is the param a path or a uri? If it's a uri, then the param should be called uri. Also, there are so many decodeds, encodeds flying around, that it might be clearer to name the method, rather than just relying on the javadoc of the param. Suggestion: canonicalencodedURI.
There was a problem hiding this comment.
It is a uri. I will rename
| */ | ||
| public static String canonicalEncodedPath(String path) | ||
| public static String canonicalPath(String path) |
There was a problem hiding this comment.
As above, it would be clearer to call this method canonicalDecodedPath - or to keep the api the same, add this method and make canonicalPath deprecated and refer to canonicalDecodedPath.
There was a problem hiding this comment.
I'm not keen to rename this method. There is now only canonicalPath and canonicalUri, with javadoc making it clear what they are.
| return null; | ||
|
|
||
| path = URIUtil.canonicalPath(path); | ||
| throw new MalformedURLException("null path"); |
There was a problem hiding this comment.
Is this change from returning null to MalformedURLException tested?
There was a problem hiding this comment.
yes - The main resource classes do not return null and few calls to addPath ever expect a null return.
So this is just making it more uniform.
| @@ -300,7 +300,8 @@ public Resource addPath(String path) throws IOException | |||
| { | |||
| return new ResourceCollection(resources.toArray(new Resource[0])); | |||
| } | |||
| return null; | |||
|
|
|||
| throw new MalformedURLException(); | |||
There was a problem hiding this comment.
No, because I can't see how this can ever happen, as either resource or resources will be non null.
Thus throwing rather than returning null just make sure it is seen as an error if it error does somehow get there
|
|
||
| Path normal = path.normalize(); | ||
| if (!abs.equals(normal)) | ||
| return normal; |
There was a problem hiding this comment.
What happens if the normalized path is to a symlink? Do we want to do the code below to resolve the target of the symlink?
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts
force-pushed
the
jetty-9.4.x-6473-normalize-only-once
branch
from
bae6d23
to
c2d5a82
Compare
Fix #6473 Reduce multiple canonicalPath calls with single alias check in PathResource