Hi Felix,
In theory, I'm in agreement with this change. However, I think we probably
need a switch to retain the old behavior if necessary. I would also caution
that I believe our test coverage around vanity paths is rather limited, so
I wouldn't take passing all the tests as necessarily being a green light.

That said, I also think we need to revisit the mapping functionality,
especially as it relates to multi-domain handling. I've not had enough time
to really think this through, but I'm confident we can do better than what
we have today.

So, if this fix scratches some near-term itch, let's do it, but I do think
something bigger is on the horizon, even if we can't see it yet.

Justin


On Mon, Dec 10, 2012 at 12:20 PM, Felix Meschberger <[email protected]>wrote:

> Hi all,
>
> We have a nice feature to easily configure URL to content mappings along
> with virtual host URL mappings in the content in the /etc/map structure.
> See also Mappings for Resource Resolution [1].
>
> Into this generic mechanism we also mix support for vanity URLs: On any
> content in the repository you can set a sling:vanityPath property with a
> short name. This automatically creates a mapping from an URL with that name
> to the respective content. Consider for example you have a content path
> /content/cars/audi/a7 with a sling:vanityPath set to "a7". The user may now
> access this page using an URL such as http://host/a7.html instead of
> having to type the full path.
>
> This all works nicely and great. But ...
>
> The problem is that the virtual host URL mapping and vanity URL mapping
> interfere with each other in a rather unpleasant way: Internally we
> conceptually represent the /etc/map content structure and the virtual paths
> as a simple table which is (basically) scanned to find a matching mapping
> from URL to content path. To sort this table, we simply sort the table by
> the lengths of the URL patterns.
>
> For example consider the following mappings:
>
>    http/example.com.8080/  > /content/example/
>    [^/]+/[^/]+/sv      > /content/short-vanity
>
> For the given URL http://example.com:8080/sv this would resolve to the
> resource path /content/example/sv instead of /content/short-vanity.
>
> I think the fix is rather simple:
>
> Index:
> bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
> ===================================================================
> ---
> bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
>       (revision 1419542)
> +++
> bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
>       (working copy)
> @@ -854,12 +854,6 @@
>              if (this.nextSpecial == null) {
>                  this.next = this.nextGlobal;
>                  this.nextGlobal = null;
> -            } else if (this.nextGlobal == null) {
> -                this.next = this.nextSpecial;
> -                this.nextSpecial = null;
> -            } else if (this.nextGlobal.getPattern().length() >=
> this.nextSpecial.getPattern().length()) {
> -                this.next = this.nextGlobal;
> -                this.nextGlobal = null;
>              } else {
>                  this.next = this.nextSpecial;
>                  this.nextSpecial = null;
>
> Basically this means, whenever a request matches a potential vanity path,
> this (nextSpecial) is considered before any /etc/map mapping (nextGlobal).
>
> I have applied this patch and Sling still builds and passes all unit tests.
>
> WDYT ?
>
> Regards
> Felix
>
>
> [1] http://sling.apache.org/site/mappings-for-resource-resolution.html

Reply via email to