-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix for: Exception occurring when encoding the path containing unwise characters (i.e space) #364
Fix for: Exception occurring when encoding the path containing unwise characters (i.e space) #364
Conversation
return null; | ||
} | ||
try { | ||
return new URI(context.path.replaceAll(" ", "%20")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question out of curiosity: shouldn't we escape whole path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what other options we have? Only path segment after last '/' ?
Even if - is there any harm to keep it simple with replaceAll
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As usual I'm referring to standards ;)
In RFC2396 (URI spec) you can find unwise
characters that required escaping.
unwise = "{" | "}" | "|" | "\" | "^" | "[" | "]" | "`"
Might not be worth doing it in this scenario though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for details @pazkooda, very good idea to properly handle all of them (i.e by using UrlDecoder/Encoder classes), please re-review, unfortunately the changes got bigger as I needed to adjust problem-to-json serializers as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handling of unwise
char will be nice bonus but otherwise look OK.
Fixes #358