[
https://issues.apache.org/jira/browse/WICKET-1627?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12597175#action_12597175
]
donohoedigital edited comment on WICKET-1627 at 5/15/08 11:15 AM:
----------------------------------------------------------------
The RFC says that a ":" in the query string is reserved. So, technically any
wicket URL that passed a ":" in the query string that is not encoded is in
violation of the RFC. Why the ":" is reserved is not clear (the RFC is not
specific, but I suspect it is due to the use of colon earlier in the path like
http://foo:8080).
What happens in practice is obviously another matter. It clearly seems to work
not encoded. We could perhaps mark the ':' as not needing encoding in the
WicketURLEncoder.QUERY_INSTANCE. Will this cause problems in the future?
Unlikely. However, does Wicket wish to violate the RFC purposefully?
Regardless of the disposition of the colon, there are clearly places where
encoding is done inconsistently, incorrectly or perhaps the worst violation:
double-encoding. There is a very real need to encode path components
differently from query string components. My patch attempts to lay the
framework for the correct encoding logic by clearly having path and query
encoding instances.
Perhaps other Wicket developers can comment here. I've looked at the code a
little more in-depth and there doesn't seem to be a centralized place that
path/query encoding is done. What is the history here?
was (Author: donohoedigital):
The RFC says that a ":" in the query string is reserved. So, technically
any wicket URL that passed a ":" in the query string that is not encoded is in
violation of the RFC. Why the ":" is reserved is not clear (the RFC is not
specific, but I suspect it is due to the use of colon earlier in the path like
http://foo:8080).
What happens in practice is obviously another matter. It clearly seems to work
not encoded. We could perhaps mark the ':' as not needing encoding in the
WicketURLEncoder.QUERY_INSTANCE. Will this cause problems in the future?
Unlikely. However, does Wicket wish to violate the RFC purposefully?
Regardless of the disposition of the colon, there are clearly places where
encoding is done inconsistently, incorrectly or perhaps the worst violation:
double-encoding. There is a very real need to encode path components
differently from query string components. My patch attempts to lay the
framework for the correct encoding logic by clearly having path and query
encoding instances.
Perhaps other Wicket developers can comment here. I've looked at the code a
little more in-depth and there doesn't seem to be a centralized place that
path/query encoding is done. What is the history here?
> AbstractRequestTargetUrlCodingStrategy improper user of URLEncoder.encode
> -------------------------------------------------------------------------
>
> Key: WICKET-1627
> URL: https://issues.apache.org/jira/browse/WICKET-1627
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 1.3.1, 1.3.2, 1.3.3, 1.4-M1
> Environment: Tomcat or Jetty (probably others)
> Reporter: Doug Donohoe
> Fix For: 1.4-M2
>
> Attachments: 1627and1624.v2.patch
>
>
> The use of URLEncoder.encode is incorrect in this scenario. The URLEncoder
> is meant for encoding query string values - not values that appear in the
> path portion of a URI.
> Because the AbstractRequestTargetUrlCodingStrategy is used by other classes
> to encode values that appear in the path, problems can occur when that path
> has spaces. For example, the parameter "message with spaces
> and+some+pluses" is encoded as follows in a URL:
> http://localhost:8080/bugs/home/message/message+with+spaces+and%2Bsome%2Bpluses/
> However, the resulting request.getServletPath() call returns this:
> /home/message/message+with+spaces+and+some+plusses=bug/
> Note that the + in the path are not turned back into spaces. This is the
> correct behavior and is seen in both Tomcat and Jetty.
> See the RFC (http://www.ietf.org/rfc/rfc2396.txt) for a full description of
> what should or should not be encoded.
> /**
> * Url encodes a string
> *
> * @param string
> * string to be encoded
> * @return encoded string
> */
> protected String urlEncode(String string)
> {
> try
> {
> return URLEncoder.encode(string,
> Application.get().getRequestCycleSettings()
> .getResponseRequestEncoding());
> }
> catch (UnsupportedEncodingException e)
> {
> log.error(e.getMessage(), e);
> return string;
> }
> }
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
