This is a little bit tricky - I think Felix suggestion makes sense - the current implementation is problematic as vanity paths might be shadowed. So I'm +1 on this change. On the other hand, this introduces a change in behaviour - there might be some rare cases where before a mapping of /etc/map was used, hiding a vanity path and now this is reverted - but the old behaviour is wanted :) But I doubt that there are many cases and simply deleting the vanity path definition solves the problem.
I also agree with Justin that we could start on improvements in this area, things like run mode dependend mappings come to my mind and I guess there are more things. Carsten 2012/12/11 Justin Edelson <[email protected]>: > 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 -- Carsten Ziegeler [email protected]
