enonic / xp

Enonic XP
https://enonic.com
GNU General Public License v3.0
201 stars 34 forks source link

Trailing slash is removed from rawPath #8858

Closed espen42 closed 3 years ago

espen42 commented 3 years ago

In at least one circumstance, neither path nor contextPath nor rawPath can be used to determine whether or not a call to a controller was done with a trailing slash or not - and from what I could see, not any other fields either.

The circumstance: vhosting active (localhost/wapp -> localhost/webapp/my.app etc, in com.enonic.xp.web.vhost.cfg), and , following a link in a webapp (link made by portal.serviceUrl) to localhost/wapp/_/service/my.app/myservice. In the webapp, the trailing slashes were visible. But in the service, when logging the entire request, there was no difference between navigating to localhost/wapp/_/service/my.app/myservice/stuff and localhost/wapp/_/service/my.app/myservice/stuff/.

Notably it only occurred when enabled = true was set in com.enonic.xp.web.vhost.cfg. If it was commented out or set to false, then trailing slashes were visible - but vhosting was still working, for some reason. Unexpected to me at least)

Not sure if this is a bug, or necessary behavior for something else to work. But this looks like a blocking problem for lib-static, both since detecting trailing slashes is a crucial part of falling back to an index.html, and since leaning on the request object for fallback creates recursive redirect loops whenever using lib-static below a service and calling it (or even a sub-uri) without a trailing slash.

In what other circumstances might this be an issue? Does this need to be mapped out completely for knowing when lib-static can be trusted to detect this (or, can be used at all - which won't look very good IMO)? If so, verifying all the different kinds of controllers (service, webapp, mapped controller, component controller, service URIs created by serviceUrl in all of those), orthogonally combined with [no vhosting] vs [vhosting with enabled = true] vs [vhosting with enabled = false], maybe even [vhosting with enabled commented out]?

alansemenov commented 3 years ago

Duplicate of https://github.com/enonic/xp/issues/8848?

espen42 commented 3 years ago

@alansemenov IMO, no, this is bigger (and worse?). Since it applies to all of the fields in request and not only path, there's no opening for a workaround in lib-static.

rymsha commented 3 years ago

related #7087

rymsha commented 3 years ago

The issue happens because when vhosts are enabled "VirtualHostFilter" rebuilds path from "source" to "target" path. But the way it is done also strips away all extra slashes and whitespaces - and as we see, they are sometimes essential. The fix is to be less aggressive in path "normalization":

before the fix: "<source>/foo/" -> "<target>/foo" after the fix: "<source>/foo/" -> "<target>/foo/"

This does not affect path just yet, so #8848 remains open.