On Tue, Sep 13, 2016 at 9:07 AM, Edward Burns <edward.burns_at_oracle.com> wrote:
>>>>>> On Thu, 8 Sep 2016 15:24:59 +1000, Greg Wilkins <gregw_at_webtide.com> said:
>
> GW> According to the javadoc
> GW> <http://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest.html#getContextPath()>,
> GW> the container does not decode the string, so it would not return "/context
> GW> path", rather it should return "/content%20path", or even "
> GW> /c%6Fntent%20path" if that is what the client sent!
>
> Considering the fundamental nature of this one, I want to be sure I am
> understanding you correctly. Greg, prior to your discovery of this
> issue, Jetty *was* performing decoding before returning the result from
> HttpServletRequest.getContextPath(). Is that correct?
>
> GW> Luckily such encoded context paths are very rare!
>
> GW> However, we would like to be compliant, so we are fixing this, but I'd just
> GW> like to check regarding ServletContext.getContextPath(), as its javadoc
> GW> does not mention if it is encoded or not. I would assume that it
> GW> is?
>
> Indeed it does not, and given that the rest of the text is verbatim the
> same as HttpServletRequest.getContextPath(), I agree it probably
> should. I have filed SPEC-163 for this.
I don't think this actually makes sense. Say we have a web app that we
have deployed at
'/my app'
This is the canonical context path that the deployer has set, it is
not determined by the contents of a request, there is really no need
for it to be decoded/not decoded as it has not been provided by a HTTP
request. If we added this text IMHO it would just cause confusion, as
decoded/encoded does not mean anything in this case. If you really
want both methods to return the same result you would actually have to
add a requirement to encode this string (assuming the deployer has not
provided it in encoded form).
Clients can then make requests against this using URL's of the following form:
/my+app
/my%20app
So with the way the spec is currently written
HttpServletRequest.getContextPath() and
ServletContext.getContextPath() will never return the same result if
the mapped context path contains characters that must be encoded,
which IMHO is not great.
Stuart
>
>>>>>> On Thu, 8 Sep 2016 17:01:08 +1000, Stuart Douglas <sdouglas_at_redhat.com> said:
>
> SD> Looks like Undertow also has the same issue. If it turns out that
> SD> everyone has implemented it the same (wrong) way maybe we should look
> SD> at changing the spec instead.
>
> I looked at what we do in GlassFish and we just return what was set, no
> decoding is performed.
>
> SD> Personally I don't think this makes much sense that this is not
> SD> decoded while getServletPath and getPathInfo are not. The only
> SD> possible reason I can think of is that maybe it is to allow you to
> SD> select the URL encoding on a per application basis (basically you
> SD> could hold off on URL decoding till the request has been mapped to an
> SD> application), but I think that is a fairly weak reason.
>
>>>>>> On Thu, 8 Sep 2016 11:53:47 +0100, Mark Thomas <markt_at_apache.org> said:
>
> MT> I'd dearly like to be able to ditch that implementation and replace it
> MT> with the one liner that would return the decoded path.
>
> MT> I have seen the method used to build encoded URIs. Tomcat does this in
> MT> its own examples and the Manager application.
>
> MT> I think either option (fix the other containers or change the spec) is
> MT> going to lead to breakage somewhere. Given the minimal reaction there
> MT> was to Tomcat fixing this, I suspect that breakage will be small either
> MT> way. I'm very much on the fence as to which way to go on this.
>
> The only spec change Greg is requesting is to make the text in
> ServletContext.getContextPath() be the same as in
> HttpServletRequest.getContextPath() by including the statement: "The
> container does not decode this string." I have captured just this bit
> in SPEC-163. This change will not lead to any breakage.
>
> Ed
>
> --
> | edward.burns_at_oracle.com | office: +1 407 458 0017