You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Justin Edelson <ju...@justinedelson.com> on 2014/04/01 20:24:01 UTC
ResourceResolver.map() always adds the context path
Hi,
I'm seeing an issue related to ResourceResolver.map(). In the
two-argument version, if the HttpServletRequest object passed has a
context path, that context path is *always* prepended to the mapped
resource path. In SLING-3338[1], I made a change to
SlingHttpServletResponseImpl so that the two-argument map() method was
called. As described in SLING-3338, this was necessary because without
it, the request's domain was not taken into account when determining
the path mapping.
However, it turns out that this is problematic because the API
contract of HttpServletResponse.encodeURL(String) and
HttpServletResponse.encodeRedirectURL(String) requires that the
context path *not* be prepended. Meaning that callers of these method
typically manually add the context path, i.e
String url = response.encodeURL(request.getContextPath() + "/foo");
The simplest approach I can think of is to add code in
SlingHttpServletResponseImpl to remove the context path from the
parameter passed to encodeURL() and encodeRedirectURL(). This,
however, is potentially problematic as it would fail to handle
correctly the (edge) case where the context path and the first path
segment were the same.
Does anyone have a better suggestion?
Regards,
Justin
[1] https://issues.apache.org/jira/browse/SLING-3338
Re: ResourceResolver.map() always adds the context path
Posted by Carsten Ziegeler <cz...@apache.org>.
I've created SLING-3504 to fix this - I think the initial idea by Justin is
so far the best one. Dev's who directly use the encodeXX methods on the
response usually path in the context path as well as this is what they're
used to from the servlet spec.
Carsten
2014-04-03 23:34 GMT+02:00 Alexander Klimetschek <ak...@adobe.com>:
> On 02.04.2014, at 05:51, Justin Edelson <ju...@justinedelson.com> wrote:
>
> > Hi Carsten,
> > Just curious - why do you prefer to remove the context path from the
> > *result* of ResourceResolver.map() rather than removing it from the
> > path passed *to* ResourceResolver.map()?
> >
> > Since ResourceResolver.map() does a resolve() call, it seems to make
> > more sense to me that the path passed into it is the real resource
> > path, not the context path + resource path.
>
> I think one problem with the sling mapping is that the configuration can
> make an important difference: you can just do simple path mappings such as
> vanity paths, that are restricted to the resource tree if you will. But you
> can also configure domains & ports, so that mapping "/site1" will give you "
> http://site1.com/foo" and "/site2" will give you "http://site2.com/foo".
>
> And afaik, this can happen for the map(path) one as well - it would be an
> absolute URL if such a mapping is present.
>
> The map(request, path) one will do the same, but if there is no domain
> mapping config, it will take the schemehostport from the passed request and
> ensure an absolute URL is returned.
>
> As part of that you have to add the context path from that passed request.
> I guess if you have a mapping config with a domain, you are expected to
> include the right context path.
>
> That's what I know, but I don't really have an idea how to fix the issue
> at hand...
>
> Cheers,
> Alex
>
>
--
Carsten Ziegeler
cziegeler@apache.org
Re: ResourceResolver.map() always adds the context path
Posted by Alexander Klimetschek <ak...@adobe.com>.
On 02.04.2014, at 05:51, Justin Edelson <ju...@justinedelson.com> wrote:
> Hi Carsten,
> Just curious - why do you prefer to remove the context path from the
> *result* of ResourceResolver.map() rather than removing it from the
> path passed *to* ResourceResolver.map()?
>
> Since ResourceResolver.map() does a resolve() call, it seems to make
> more sense to me that the path passed into it is the real resource
> path, not the context path + resource path.
I think one problem with the sling mapping is that the configuration can make an important difference: you can just do simple path mappings such as vanity paths, that are restricted to the resource tree if you will. But you can also configure domains & ports, so that mapping "/site1" will give you "http://site1.com/foo" and "/site2" will give you "http://site2.com/foo".
And afaik, this can happen for the map(path) one as well - it would be an absolute URL if such a mapping is present.
The map(request, path) one will do the same, but if there is no domain mapping config, it will take the schemehostport from the passed request and ensure an absolute URL is returned.
As part of that you have to add the context path from that passed request. I guess if you have a mapping config with a domain, you are expected to include the right context path.
That's what I know, but I don't really have an idea how to fix the issue at hand...
Cheers,
Alex
Re: ResourceResolver.map() always adds the context path
Posted by Carsten Ziegeler <cz...@apache.org>.
Hi Justin,
ah right - got it. Yes, I wasn't thinking that case through...so it seems
your suggestion is better than :)
Regards
Carsten
2014-04-02 14:51 GMT+02:00 Justin Edelson <ju...@justinedelson.com>:
> Hi Carsten,
> Just curious - why do you prefer to remove the context path from the
> *result* of ResourceResolver.map() rather than removing it from the
> path passed *to* ResourceResolver.map()?
>
> Since ResourceResolver.map() does a resolve() call, it seems to make
> more sense to me that the path passed into it is the real resource
> path, not the context path + resource path.
>
> Regards,
> Justin
>
> On Wed, Apr 2, 2014 at 12:54 AM, Carsten Ziegeler <cz...@apache.org>
> wrote:
> > I always have the feeling that our map() method does too much magic,
> > especially the prepending of the context path is too much imho.
> >
> > Anyways, for the issue at hand, yes, I think we should change
> > SlingHttpServletResponseImpl to not add the context path in front. I
> think
> > this can safely be done by comparing the beginning of the passed in path
> > with the one returned from the map method. So the implementation can know
> > whether the context path has been added again by map() or not.
> >
> > Carsten
> >
> >
> > 2014-04-01 20:24 GMT+02:00 Justin Edelson <ju...@justinedelson.com>:
> >
> >> Hi,
> >> I'm seeing an issue related to ResourceResolver.map(). In the
> >> two-argument version, if the HttpServletRequest object passed has a
> >> context path, that context path is *always* prepended to the mapped
> >> resource path. In SLING-3338[1], I made a change to
> >> SlingHttpServletResponseImpl so that the two-argument map() method was
> >> called. As described in SLING-3338, this was necessary because without
> >> it, the request's domain was not taken into account when determining
> >> the path mapping.
> >>
> >> However, it turns out that this is problematic because the API
> >> contract of HttpServletResponse.encodeURL(String) and
> >> HttpServletResponse.encodeRedirectURL(String) requires that the
> >> context path *not* be prepended. Meaning that callers of these method
> >> typically manually add the context path, i.e
> >>
> >> String url = response.encodeURL(request.getContextPath() + "/foo");
> >>
> >> The simplest approach I can think of is to add code in
> >> SlingHttpServletResponseImpl to remove the context path from the
> >> parameter passed to encodeURL() and encodeRedirectURL(). This,
> >> however, is potentially problematic as it would fail to handle
> >> correctly the (edge) case where the context path and the first path
> >> segment were the same.
> >>
> >> Does anyone have a better suggestion?
> >>
> >> Regards,
> >> Justin
> >>
> >>
> >> [1] https://issues.apache.org/jira/browse/SLING-3338
> >>
> >
> >
> >
> > --
> > Carsten Ziegeler
> > cziegeler@apache.org
>
--
Carsten Ziegeler
cziegeler@apache.org
Re: ResourceResolver.map() always adds the context path
Posted by Justin Edelson <ju...@justinedelson.com>.
Hi Carsten,
Just curious - why do you prefer to remove the context path from the
*result* of ResourceResolver.map() rather than removing it from the
path passed *to* ResourceResolver.map()?
Since ResourceResolver.map() does a resolve() call, it seems to make
more sense to me that the path passed into it is the real resource
path, not the context path + resource path.
Regards,
Justin
On Wed, Apr 2, 2014 at 12:54 AM, Carsten Ziegeler <cz...@apache.org> wrote:
> I always have the feeling that our map() method does too much magic,
> especially the prepending of the context path is too much imho.
>
> Anyways, for the issue at hand, yes, I think we should change
> SlingHttpServletResponseImpl to not add the context path in front. I think
> this can safely be done by comparing the beginning of the passed in path
> with the one returned from the map method. So the implementation can know
> whether the context path has been added again by map() or not.
>
> Carsten
>
>
> 2014-04-01 20:24 GMT+02:00 Justin Edelson <ju...@justinedelson.com>:
>
>> Hi,
>> I'm seeing an issue related to ResourceResolver.map(). In the
>> two-argument version, if the HttpServletRequest object passed has a
>> context path, that context path is *always* prepended to the mapped
>> resource path. In SLING-3338[1], I made a change to
>> SlingHttpServletResponseImpl so that the two-argument map() method was
>> called. As described in SLING-3338, this was necessary because without
>> it, the request's domain was not taken into account when determining
>> the path mapping.
>>
>> However, it turns out that this is problematic because the API
>> contract of HttpServletResponse.encodeURL(String) and
>> HttpServletResponse.encodeRedirectURL(String) requires that the
>> context path *not* be prepended. Meaning that callers of these method
>> typically manually add the context path, i.e
>>
>> String url = response.encodeURL(request.getContextPath() + "/foo");
>>
>> The simplest approach I can think of is to add code in
>> SlingHttpServletResponseImpl to remove the context path from the
>> parameter passed to encodeURL() and encodeRedirectURL(). This,
>> however, is potentially problematic as it would fail to handle
>> correctly the (edge) case where the context path and the first path
>> segment were the same.
>>
>> Does anyone have a better suggestion?
>>
>> Regards,
>> Justin
>>
>>
>> [1] https://issues.apache.org/jira/browse/SLING-3338
>>
>
>
>
> --
> Carsten Ziegeler
> cziegeler@apache.org
Re: ResourceResolver.map() always adds the context path
Posted by Carsten Ziegeler <cz...@apache.org>.
I always have the feeling that our map() method does too much magic,
especially the prepending of the context path is too much imho.
Anyways, for the issue at hand, yes, I think we should change
SlingHttpServletResponseImpl to not add the context path in front. I think
this can safely be done by comparing the beginning of the passed in path
with the one returned from the map method. So the implementation can know
whether the context path has been added again by map() or not.
Carsten
2014-04-01 20:24 GMT+02:00 Justin Edelson <ju...@justinedelson.com>:
> Hi,
> I'm seeing an issue related to ResourceResolver.map(). In the
> two-argument version, if the HttpServletRequest object passed has a
> context path, that context path is *always* prepended to the mapped
> resource path. In SLING-3338[1], I made a change to
> SlingHttpServletResponseImpl so that the two-argument map() method was
> called. As described in SLING-3338, this was necessary because without
> it, the request's domain was not taken into account when determining
> the path mapping.
>
> However, it turns out that this is problematic because the API
> contract of HttpServletResponse.encodeURL(String) and
> HttpServletResponse.encodeRedirectURL(String) requires that the
> context path *not* be prepended. Meaning that callers of these method
> typically manually add the context path, i.e
>
> String url = response.encodeURL(request.getContextPath() + "/foo");
>
> The simplest approach I can think of is to add code in
> SlingHttpServletResponseImpl to remove the context path from the
> parameter passed to encodeURL() and encodeRedirectURL(). This,
> however, is potentially problematic as it would fail to handle
> correctly the (edge) case where the context path and the first path
> segment were the same.
>
> Does anyone have a better suggestion?
>
> Regards,
> Justin
>
>
> [1] https://issues.apache.org/jira/browse/SLING-3338
>
--
Carsten Ziegeler
cziegeler@apache.org
Re: ResourceResolver.map() always adds the context path
Posted by Justin Edelson <ju...@justinedelson.com>.
Hi Alex,
TBH, when looking at SLING-3338, I didn't even consider whether or not
encodeURL and encodeRedirectURL *should* actually do path mapping. I
was only concerned with the fact that the mapping it did do (which was
there for 6+ years) was wrong in multi-domain setups.
I would be curious if anyone remembers why these methods call map() in
the first place.
Regarding c:url, my understanding is that it is supposed to result in
the the Java SessionID being appended (depending on configuration) and
so rightly uses encodeURL.
Justin
On Tue, Apr 1, 2014 at 4:06 PM, Alexander Klimetschek
<ak...@adobe.com> wrote:
> If I read the javadoc for HttpServletResponse.encodeURL() [1] correctly, all it does is to add the Java servlet Session ID. It isn't even supposed to encode anything on the string level - "encode" seems to be used as a term for "include session id" here.
>
> OTOH, this discussion [2] - or better the first comment - mention that it can be used by web frameworks to define whatever a custom mapping/encoding for a URL is.
>
> But I think in any case it is weird to add the context path to it, as that is a standard java servlet concept, and if that would be needed here, the javadocs should explicitly mention it.
>
> Looking at SLING-3338, the question is what <c:url> is supposed to do and why the org.apache.taglibs.standard.tag.rt.core.UrlTag implementation just relies on encodeUrl.
>
> [1] http://docs.oracle.com/javaee/5/api/javax/servlet/http/HttpServletResponse.html#encodeURL(java.lang.String)
> [2] http://closingbraces.net/2007/02/11/encodeurlandencoderedirecturl/
>
> Cheers,
> Alex
>
> On 01.04.2014, at 11:24, Justin Edelson <ju...@justinedelson.com> wrote:
>
>> Hi,
>> I'm seeing an issue related to ResourceResolver.map(). In the
>> two-argument version, if the HttpServletRequest object passed has a
>> context path, that context path is *always* prepended to the mapped
>> resource path. In SLING-3338[1], I made a change to
>> SlingHttpServletResponseImpl so that the two-argument map() method was
>> called. As described in SLING-3338, this was necessary because without
>> it, the request's domain was not taken into account when determining
>> the path mapping.
>>
>> However, it turns out that this is problematic because the API
>> contract of HttpServletResponse.encodeURL(String) and
>> HttpServletResponse.encodeRedirectURL(String) requires that the
>> context path *not* be prepended. Meaning that callers of these method
>> typically manually add the context path, i.e
>>
>> String url = response.encodeURL(request.getContextPath() + "/foo");
>>
>> The simplest approach I can think of is to add code in
>> SlingHttpServletResponseImpl to remove the context path from the
>> parameter passed to encodeURL() and encodeRedirectURL(). This,
>> however, is potentially problematic as it would fail to handle
>> correctly the (edge) case where the context path and the first path
>> segment were the same.
>>
>> Does anyone have a better suggestion?
>>
>> Regards,
>> Justin
>>
>>
>> [1] https://issues.apache.org/jira/browse/SLING-3338
>
Re: ResourceResolver.map() always adds the context path
Posted by Alexander Klimetschek <ak...@adobe.com>.
If I read the javadoc for HttpServletResponse.encodeURL() [1] correctly, all it does is to add the Java servlet Session ID. It isn't even supposed to encode anything on the string level - "encode" seems to be used as a term for "include session id" here.
OTOH, this discussion [2] - or better the first comment - mention that it can be used by web frameworks to define whatever a custom mapping/encoding for a URL is.
But I think in any case it is weird to add the context path to it, as that is a standard java servlet concept, and if that would be needed here, the javadocs should explicitly mention it.
Looking at SLING-3338, the question is what <c:url> is supposed to do and why the org.apache.taglibs.standard.tag.rt.core.UrlTag implementation just relies on encodeUrl.
[1] http://docs.oracle.com/javaee/5/api/javax/servlet/http/HttpServletResponse.html#encodeURL(java.lang.String)
[2] http://closingbraces.net/2007/02/11/encodeurlandencoderedirecturl/
Cheers,
Alex
On 01.04.2014, at 11:24, Justin Edelson <ju...@justinedelson.com> wrote:
> Hi,
> I'm seeing an issue related to ResourceResolver.map(). In the
> two-argument version, if the HttpServletRequest object passed has a
> context path, that context path is *always* prepended to the mapped
> resource path. In SLING-3338[1], I made a change to
> SlingHttpServletResponseImpl so that the two-argument map() method was
> called. As described in SLING-3338, this was necessary because without
> it, the request's domain was not taken into account when determining
> the path mapping.
>
> However, it turns out that this is problematic because the API
> contract of HttpServletResponse.encodeURL(String) and
> HttpServletResponse.encodeRedirectURL(String) requires that the
> context path *not* be prepended. Meaning that callers of these method
> typically manually add the context path, i.e
>
> String url = response.encodeURL(request.getContextPath() + "/foo");
>
> The simplest approach I can think of is to add code in
> SlingHttpServletResponseImpl to remove the context path from the
> parameter passed to encodeURL() and encodeRedirectURL(). This,
> however, is potentially problematic as it would fail to handle
> correctly the (edge) case where the context path and the first path
> segment were the same.
>
> Does anyone have a better suggestion?
>
> Regards,
> Justin
>
>
> [1] https://issues.apache.org/jira/browse/SLING-3338