diff --git a/news/+vhm-paths.bugfix b/news/+vhm-paths.bugfix new file mode 100644 index 0000000..33930bb --- /dev/null +++ b/news/+vhm-paths.bugfix @@ -0,0 +1,4 @@ +Fix path criteria behind a Virtual Host Monster: paths relative to the virtual hosting root (including ``_vh_`` segments) are now translated to physical paths. +Before, the navigation root or portal path was prepended, which gave wrong results when the virtual root was not the portal (e.g. a subsite or language folder) or the context was inside a navigation root. +See plone/plone.restapi#2023. +@petschki diff --git a/src/plone/app/querystring/queryparser.py b/src/plone/app/querystring/queryparser.py index 79086cd..d8e0c3d 100644 --- a/src/plone/app/querystring/queryparser.py +++ b/src/plone/app/querystring/queryparser.py @@ -12,6 +12,7 @@ from zope.component import getUtilitiesFor from zope.component import getUtility from zope.dottedname.resolve import resolve +from zope.globalrequest import getRequest Row = namedtuple("Row", ["index", "operator", "values"]) PATH_INDICES = {"path"} @@ -370,8 +371,11 @@ def _pathByRoot(root, context, row): if "/" not in values: # It must be a UID values = getPathByUID(context, values) - # take care of absolute paths without root - if not values.startswith(root + "/") and values != root: + physical_path = _virtualPathToPhysicalPath(context, values) + if physical_path is not None: + values = physical_path + elif not values.startswith(root + "/") and values != root: + # take care of absolute paths without root values = root + values query = {} if depth is not None: @@ -460,6 +464,35 @@ def _referenceIs(context, row): # Helper functions +def _virtualPathToPhysicalPath(context, path): + """Translate a path relative to the virtual hosting root into a physical path. + + Behind a Virtual Host Monster clients only know the URL path of an object, + which is relative to the virtual root (and may contain ``_vh_`` segments). + The path index needs the physical path. + + Returns None if no virtual hosting is active, if the path is already a + physical path of the portal or if it does not fit into the virtual + hosting context. + """ + if not path.startswith("/"): + return None + request = getRequest() + if request is None or not request.get("VirtualRootPhysicalPath"): + return None + portal_path = getToolByName(context, "portal_url").getPortalPath() + if path == portal_path or path.startswith(portal_path + "/"): + return None + try: + physical_path = "/".join(request.physicalPathFromURL(path)) + except ValueError: + return None + if path.endswith("/") and not physical_path.endswith("/"): + # physicalPathFromURL drops it, keep the path as given + physical_path += "/" + return physical_path + + def getPathByUID(context, uid): """Returns the path of an object specified by UID""" catalog = getToolByName(context, "portal_catalog") diff --git a/src/plone/app/querystring/tests/testQueryParser.py b/src/plone/app/querystring/tests/testQueryParser.py index 2973fb4..72d6906 100644 --- a/src/plone/app/querystring/tests/testQueryParser.py +++ b/src/plone/app/querystring/tests/testQueryParser.py @@ -1,4 +1,5 @@ from DateTime import DateTime +from io import BytesIO from plone.app.querystring import queryparser from plone.app.querystring.queryparser import Row from plone.app.querystring.testing import ( @@ -15,7 +16,11 @@ from Products.CMFCore.interfaces import IURLTool from zope.component import getGlobalSiteManager from zope.component import getSiteManager +from zope.globalrequest import clearRequest +from zope.globalrequest import setRequest from zope.interface import implementer +from ZPublisher.HTTPRequest import HTTPRequest +from ZPublisher.HTTPResponse import HTTPResponse import unittest @@ -660,3 +665,104 @@ def test_objStartsWithSiteId(self): parsed = queryparser._absolutePath(MockSite(), data) expected = {"path": {"query": [f"/{MOCK_SITE_ID}/{MOCK_SITE_ID}-news/"]}} self.assertEqual(parsed, expected) + + +def make_virtual_host_request(virtual_root_path, vh_segments=()): + """Request as prepared by the Virtual Host Monster. + + ``virtual_root_path`` is the physical path of the VirtualHostRoot, + ``vh_segments`` are the ``_vh_`` path segments (inside-out hosting). + """ + environ = { + "SERVER_NAME": "example.org", + "SERVER_PORT": "80", + "REQUEST_METHOD": "GET", + } + request = HTTPRequest(BytesIO(), environ, HTTPResponse()) + request.other["VirtualRootPhysicalPath"] = tuple(virtual_root_path.split("/")) + request._script[:] = list(vh_segments) + return request + + +class TestVirtualHostingPaths(TestQueryParserBase): + def setUpRequest(self, virtual_root_path, vh_segments=()): + setRequest(make_virtual_host_request(virtual_root_path, vh_segments)) + self.addCleanup(clearRequest) + + def navigation_context(self): + # /site/foo is a navigation root, the context is /site/foo/bar + context = MockObject(uid="00000000000000001", path="/%s/foo/bar" % MOCK_SITE_ID) + context.__parent__ = MockNavRoot( + uid="00000000000000002", path="/%s/foo" % MOCK_SITE_ID + ) + context.__parent__.__parent__ = MockSite() + return context + + def test_virtual_root_is_portal(self): + self.setUpRequest("/%s" % MOCK_SITE_ID) + data = Row(index="path", operator="_absolutePath", values="/news/") + parsed = queryparser._absolutePath(MockSite(), data) + expected = {"path": {"query": ["/%s/news/" % MOCK_SITE_ID]}} + self.assertEqual(parsed, expected) + + def test_virtual_root_is_subfolder(self): + self.setUpRequest("/%s/foo" % MOCK_SITE_ID) + data = Row(index="path", operator="_absolutePath", values="/bar::1") + parsed = queryparser._absolutePath(MockSite(), data) + expected = {"path": {"query": ["/%s/foo/bar" % MOCK_SITE_ID], "depth": 1}} + self.assertEqual(parsed, expected) + + def test_virtual_root_is_navigation_root(self): + self.setUpRequest("/%s/foo" % MOCK_SITE_ID) + data = Row(index="path", operator="_navigationPath", values="/bar/") + parsed = queryparser._navigationPath(self.navigation_context(), data) + expected = {"path": {"query": ["/%s/foo/bar/" % MOCK_SITE_ID]}} + self.assertEqual(parsed, expected) + + def test_navigation_root_below_virtual_root(self): + # The client sends the path relative to the virtual root, which + # already contains the navigation root. It must not be added twice. + self.setUpRequest("/%s" % MOCK_SITE_ID) + data = Row(index="path", operator="_navigationPath", values="/foo/bar::1") + parsed = queryparser._navigationPath(self.navigation_context(), data) + expected = {"path": {"query": ["/%s/foo/bar" % MOCK_SITE_ID], "depth": 1}} + self.assertEqual(parsed, expected) + + def test_inside_out_hosting(self): + self.setUpRequest("/%s" % MOCK_SITE_ID, vh_segments=["cms"]) + data = Row(index="path", operator="_absolutePath", values="/cms/news") + parsed = queryparser._absolutePath(MockSite(), data) + expected = {"path": {"query": ["/%s/news" % MOCK_SITE_ID]}} + self.assertEqual(parsed, expected) + + def test_inside_out_hosting_path_outside_virtual_host(self): + # A path not matching the _vh_ segments falls back to the portal path. + self.setUpRequest("/%s" % MOCK_SITE_ID, vh_segments=["cms"]) + data = Row(index="path", operator="_absolutePath", values="/other/news") + parsed = queryparser._absolutePath(MockSite(), data) + expected = {"path": {"query": ["/%s/other/news" % MOCK_SITE_ID]}} + self.assertEqual(parsed, expected) + + def test_physical_path(self): + # Physical paths are kept, the virtual root must not be added. + self.setUpRequest("/%s" % MOCK_SITE_ID) + data = Row( + index="path", operator="_absolutePath", values="/%s/news" % MOCK_SITE_ID + ) + parsed = queryparser._absolutePath(MockSite(), data) + expected = {"path": {"query": ["/%s/news" % MOCK_SITE_ID]}} + self.assertEqual(parsed, expected) + + def test_uid(self): + self.setUpRequest("/%s/foo" % MOCK_SITE_ID) + data = Row(index="path", operator="_absolutePath", values="00000000000000001") + parsed = queryparser._absolutePath(MockSite(), data) + expected = {"path": {"query": ["/%s/foo" % MOCK_SITE_ID]}} + self.assertEqual(parsed, expected) + + def test_relative_path(self): + self.setUpRequest("/%s/foo" % MOCK_SITE_ID) + data = Row(index="path", operator="_relativePath", values="..::1") + parsed = queryparser._relativePath(self.navigation_context(), data) + expected = {"path": {"query": ["/%s/foo" % MOCK_SITE_ID], "depth": 1}} + self.assertEqual(parsed, expected)