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 2012/01/13 22:36:37 UTC

what should resourceResolver.map(" ") do?

I discovered today that JcrResourceResolver.map(" ") throws a
NullPointerException from inside the mangleNamespaces() method. This
is unintuitive, so I was trying to make it more intuitive.

But then I got stuck trying to think about what it should do. I've
come up with two options, but am open to others. Note - this isn't a
formal vote (unless someone else feels it should be):

1) throw IllegalArgumentException. In this case, map("") should
probably throw this exception as well.
2) return the root node (which is what map("") does currently).

WDYT?

Re: what should resourceResolver.map(" ") do?

Posted by Felix Meschberger <fm...@adobe.com>.
Hi,

Am 16.01.2012 um 08:41 schrieb Felix Meschberger:

> Hi,
> 
> Am 13.01.2012 um 22:36 schrieb Justin Edelson:
> 
>> I discovered today that JcrResourceResolver.map(" ") throws a
>> NullPointerException from inside the mangleNamespaces() method. This
>> is unintuitive, so I was trying to make it more intuitive.
>> 
> 
> Hmm, the question is, what map("") should do in the first place ?

Oh, sorry for the confusion: This is of course not "" but " ".

I think in this case the same should happen as in the case of a non-existing path such as "/bla".

Of course NPE is not a valid result -- actually no exception from mangleNamespace is a valid result.

The problem seems to be that

   String path = new URI(" ").getPath();

sets path to null.

What should we do here ? just fall back to the original path " " and return it ?   

            URI uri = new URI(mappedPath, false);
            String path = uri.getPath();
            if (path != null) {
              path = mangleNamespaces(path);
              if (request != null && request.getContextPath() != null
                  && request.getContextPath().length() > 0) {
                  path = request.getContextPath().concat(path);
              }
              uri.setPath(path);
              mappedPath = uri.toString();
            }

Regards
Felix

> 
> If we are ok with return the root (URL), then having the mangleNamespaces throw any thing is clearly wrong.
> 
> Today, it looks like map("") returns the root URL, so the mangleNamespaces should be resilient to this.
> 
> (I cannot reproduce this NPE, though in the current trunk)
> 
>> But then I got stuck trying to think about what it should do. I've
>> come up with two options, but am open to others. Note - this isn't a
>> formal vote (unless someone else feels it should be):
>> 
>> 1) throw IllegalArgumentException. In this case, map("") should
>> probably throw this exception as well.
> 
> I think this is wrong. mangelNamespace should not throw.
> 
>> 2) return the root node (which is what map("") does currently).
> 
> if "" maps to "/", mangleNamespace should be null-op anyway, because "/" does not contain a colon.
> 
> Regards
> Felix


Re: what should resourceResolver.map(" ") do?

Posted by Felix Meschberger <fm...@adobe.com>.
Hi,

Am 13.01.2012 um 22:36 schrieb Justin Edelson:

> I discovered today that JcrResourceResolver.map(" ") throws a
> NullPointerException from inside the mangleNamespaces() method. This
> is unintuitive, so I was trying to make it more intuitive.
> 

Hmm, the question is, what map("") should do in the first place ?

If we are ok with return the root (URL), then having the mangleNamespaces throw any thing is clearly wrong.

Today, it looks like map("") returns the root URL, so the mangleNamespaces should be resilient to this.

(I cannot reproduce this NPE, though in the current trunk)

> But then I got stuck trying to think about what it should do. I've
> come up with two options, but am open to others. Note - this isn't a
> formal vote (unless someone else feels it should be):
> 
> 1) throw IllegalArgumentException. In this case, map("") should
> probably throw this exception as well.

I think this is wrong. mangelNamespace should not throw.

> 2) return the root node (which is what map("") does currently).

if "" maps to "/", mangleNamespace should be null-op anyway, because "/" does not contain a colon.

Regards
Felix

Re: what should resourceResolver.map(" ") do?

Posted by Alexander Klimetschek <ak...@adobe.com>.
On 16.01.12 08:24, "Carsten Ziegeler" <cz...@apache.org> wrote:

>Tricky question, if I request /%20 (which is the equivalent of the
>space) on some servers I always get a 404, so this is handled as a
>valid request to a resource with the name " "

Nitpicking: "/%20" refers to the resource "/ ", not " "...

So " " would be definitely a relative resource if considered valid.

I would check what JCR does.

Cheers,
Alex

-- 
Alexander Klimetschek
Developer // Adobe (Day) // Berlin - Basel





Re: what should resourceResolver.map(" ") do?

Posted by Carsten Ziegeler <cz...@apache.org>.
Tricky question, if I request /%20 (which is the equivalent of the
space) on some servers I always get a 404, so this is handled as a
valid request to a resource with the name " "
So one could argue that map should just work and leave it untouched
I'm fine with 1) as well, but I wouldn't do 2)

Carsten

2012/1/13 Justin Edelson <ju...@justinedelson.com>:
> I discovered today that JcrResourceResolver.map(" ") throws a
> NullPointerException from inside the mangleNamespaces() method. This
> is unintuitive, so I was trying to make it more intuitive.
>
> But then I got stuck trying to think about what it should do. I've
> come up with two options, but am open to others. Note - this isn't a
> formal vote (unless someone else feels it should be):
>
> 1) throw IllegalArgumentException. In this case, map("") should
> probably throw this exception as well.
> 2) return the root node (which is what map("") does currently).
>
> WDYT?



-- 
Carsten Ziegeler
cziegeler@apache.org