From a251928761be6df054f6601ba24e1ccb3f36e47f Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Thu, 19 Oct 2023 14:27:20 +0200 Subject: [PATCH 01/11] Fix link_redirect_view --- plone/app/contenttypes/browser/link_redirect_view.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plone/app/contenttypes/browser/link_redirect_view.py b/plone/app/contenttypes/browser/link_redirect_view.py index eee881ca..e048730a 100644 --- a/plone/app/contenttypes/browser/link_redirect_view.py +++ b/plone/app/contenttypes/browser/link_redirect_view.py @@ -6,6 +6,7 @@ from Products.Five.browser import BrowserView from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile from urllib.parse import urlparse +from zope.component import getMultiAdapter from zope.component import getUtility @@ -111,10 +112,16 @@ def absolute_target_url(self): if "resolveuid" in url: uid = url.split("/")[-1] obj = uuidToObject(uid) + + portal_state = getMultiAdapter( + (self.context, self.request), name="plone_portal_state" + ) + portal_url = portal_state.portal_url() + if obj: url = "/".join(obj.getPhysicalPath()[2:]) if not url.startswith("/"): - url = "/" + url + url = f"{portal_url}/{url}" if not url.startswith(("http://", "https://")): url = self.request["SERVER_URL"] + url From 08635f71504f48dbd73cde597aaef09c70ce2df8 Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Thu, 19 Oct 2023 14:27:24 +0200 Subject: [PATCH 02/11] Add News --- news/671.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/671.bugfix diff --git a/news/671.bugfix b/news/671.bugfix new file mode 100644 index 00000000..00b5b905 --- /dev/null +++ b/news/671.bugfix @@ -0,0 +1 @@ +Fix link_redirect_view, respect vhm vs none-vhm url schemes @1letter \ No newline at end of file From f64ecd9b28a7a9e75305fb293e6c9d07fac0a7a5 Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Thu, 19 Oct 2023 14:53:37 +0200 Subject: [PATCH 03/11] Fix Test --- plone/app/contenttypes/tests/test_link.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/plone/app/contenttypes/tests/test_link.py b/plone/app/contenttypes/tests/test_link.py index dd306c27..3d3ac8bd 100644 --- a/plone/app/contenttypes/tests/test_link.py +++ b/plone/app/contenttypes/tests/test_link.py @@ -375,4 +375,10 @@ def test_resolve_uid_to_absolute_target(self): doc1 = self.portal["doc1"] uid = IUUID(doc1) self.link.remoteUrl = f"${{portal_url}}/resolveuid/{uid}" - self.assertEqual(view.absolute_target_url(), "http://nohost/doc1") + + portal_state = getMultiAdapter( + (self.link, self.request), name="plone_portal_state" + ) + portal_url = portal_state.portal_url() + + self.assertEqual(view.absolute_target_url(), f"{portal_url}/doc1") From 4fae87ef0a80e2484570c79720f3c82d2e63e7f4 Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Thu, 19 Oct 2023 15:30:47 +0200 Subject: [PATCH 04/11] Fix path construction --- plone/app/contenttypes/browser/link_redirect_view.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plone/app/contenttypes/browser/link_redirect_view.py b/plone/app/contenttypes/browser/link_redirect_view.py index e048730a..7c2b9272 100644 --- a/plone/app/contenttypes/browser/link_redirect_view.py +++ b/plone/app/contenttypes/browser/link_redirect_view.py @@ -1,3 +1,4 @@ +from plone import api from plone.app.contenttypes.utils import replace_link_variables_by_paths from plone.app.uuid.utils import uuidToObject from plone.base.interfaces import ITypesSchema @@ -116,10 +117,16 @@ def absolute_target_url(self): portal_state = getMultiAdapter( (self.context, self.request), name="plone_portal_state" ) + + portal = portal_state.portal() portal_url = portal_state.portal_url() + path_pieces = tuple( + set(portal.getPhysicalPath()) ^ set(obj.getPhysicalPath()) + ) + if obj: - url = "/".join(obj.getPhysicalPath()[2:]) + url = "/".join(path_pieces) if not url.startswith("/"): url = f"{portal_url}/{url}" if not url.startswith(("http://", "https://")): From 9a2e6530fb97bfba7515eda86855fa5d99906c28 Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Thu, 19 Oct 2023 15:37:22 +0200 Subject: [PATCH 05/11] Simplify url constrction in link_redirect_view --- .../contenttypes/browser/link_redirect_view.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/plone/app/contenttypes/browser/link_redirect_view.py b/plone/app/contenttypes/browser/link_redirect_view.py index 7c2b9272..9fbfedd3 100644 --- a/plone/app/contenttypes/browser/link_redirect_view.py +++ b/plone/app/contenttypes/browser/link_redirect_view.py @@ -113,22 +113,7 @@ def absolute_target_url(self): if "resolveuid" in url: uid = url.split("/")[-1] obj = uuidToObject(uid) - - portal_state = getMultiAdapter( - (self.context, self.request), name="plone_portal_state" - ) - - portal = portal_state.portal() - portal_url = portal_state.portal_url() - - path_pieces = tuple( - set(portal.getPhysicalPath()) ^ set(obj.getPhysicalPath()) - ) - - if obj: - url = "/".join(path_pieces) - if not url.startswith("/"): - url = f"{portal_url}/{url}" + url = obj.absolute_url() if not url.startswith(("http://", "https://")): url = self.request["SERVER_URL"] + url From 7693b1a8c5bb9ebe3617eb4b2611bdc87992df48 Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Thu, 19 Oct 2023 15:42:35 +0200 Subject: [PATCH 06/11] remove unused imports --- plone/app/contenttypes/browser/link_redirect_view.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/plone/app/contenttypes/browser/link_redirect_view.py b/plone/app/contenttypes/browser/link_redirect_view.py index 9fbfedd3..5c57fa62 100644 --- a/plone/app/contenttypes/browser/link_redirect_view.py +++ b/plone/app/contenttypes/browser/link_redirect_view.py @@ -1,4 +1,3 @@ -from plone import api from plone.app.contenttypes.utils import replace_link_variables_by_paths from plone.app.uuid.utils import uuidToObject from plone.base.interfaces import ITypesSchema @@ -7,7 +6,6 @@ from Products.Five.browser import BrowserView from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile from urllib.parse import urlparse -from zope.component import getMultiAdapter from zope.component import getUtility From 6f805c8ee0d23d8609b43ca0c6f61a34208bfbae Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Fri, 20 Oct 2023 09:24:32 +0200 Subject: [PATCH 07/11] Make resolution of uid more robust --- .../browser/link_redirect_view.py | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/plone/app/contenttypes/browser/link_redirect_view.py b/plone/app/contenttypes/browser/link_redirect_view.py index 5c57fa62..d6747feb 100644 --- a/plone/app/contenttypes/browser/link_redirect_view.py +++ b/plone/app/contenttypes/browser/link_redirect_view.py @@ -25,6 +25,21 @@ ] +def get_uid_from_path(url=None): + if not url: + return None + + paths = url.split("/") + uid = None + if "resolveuid" in paths: + ri = paths.index("resolveuid") + if ri + 1 != len(paths): + uid = paths[ri + 1] + if uid == "": + uid = None + return uid + + class LinkRedirectView(BrowserView): index = ViewPageTemplateFile("templates/link.pt") @@ -109,9 +124,13 @@ def absolute_target_url(self): url = "/".join([context_state.canonical_object_url(), url]) else: if "resolveuid" in url: - uid = url.split("/")[-1] + uid = get_uid_from_path(url) obj = uuidToObject(uid) + if obj is None: + # uid can't resolve, return the url + return url url = obj.absolute_url() + if not url.startswith(("http://", "https://")): url = self.request["SERVER_URL"] + url From e4c5d657c84b7eff0fb0d8a372f8e857abcf3634 Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Fri, 20 Oct 2023 09:25:06 +0200 Subject: [PATCH 08/11] Update Tests - tests for get_uid_from_path --- plone/app/contenttypes/tests/test_link.py | 66 +++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/plone/app/contenttypes/tests/test_link.py b/plone/app/contenttypes/tests/test_link.py index 3d3ac8bd..2287972a 100644 --- a/plone/app/contenttypes/tests/test_link.py +++ b/plone/app/contenttypes/tests/test_link.py @@ -1,4 +1,5 @@ from datetime import datetime +from plone.app.contenttypes.browser.link_redirect_view import get_uid_from_path from plone.app.contenttypes.interfaces import ILink from plone.app.contenttypes.testing import ( # noqa PLONE_APP_CONTENTTYPES_FUNCTIONAL_TESTING, @@ -230,6 +231,67 @@ def test_ftp_type(self): self.assertTrue(view()) self._assert_redirect(self.link.remoteUrl) + def test_get_uid_from_path(self): + url = None + uid = get_uid_from_path(url) + self.assertIsNone(uid) + + url = "http://nohost" + uid = get_uid_from_path(url) + self.assertIsNone(uid) + + url = "http://nohost/test1" + uid = get_uid_from_path(url) + self.assertIsNone(uid) + + url = "http://nohost/test1/resolveuid" + uid = get_uid_from_path(url) + self.assertIsNone(uid) + + url = "http://nohost/test1/resolveuid/" + uid = get_uid_from_path(url) + self.assertIsNone(uid) + + url = "http://nohost/test1/resolveuid123/" + uid = get_uid_from_path(url) + self.assertIsNone(uid) + + url = "http://nohost/test1/resolveuid/123" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + + url = "http://nohost/test1/resolveuid/123/" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + + url = "http://nohost/test1/resolveuid/123/test" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + + url = "resolveuid/123" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + + url = "resolveuid/123/" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + + url = "resolveuid/123/abc" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + + url = "/resolveuid/123" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + + url = "/resolveuid/123/" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + + url = "/resolveuid/123/abc" + uid = get_uid_from_path(url) + self.assertEqual(uid, "123") + def _publish(self, obj): portal_workflow = getToolByName(self.portal, "portal_workflow") portal_workflow.doActionFor(obj, "publish") @@ -382,3 +444,7 @@ def test_resolve_uid_to_absolute_target(self): portal_url = portal_state.portal_url() self.assertEqual(view.absolute_target_url(), f"{portal_url}/doc1") + + # check not resolvable uid + self.link.remoteUrl = "/resolveuid/abc123" + self.assertEqual(view.absolute_target_url(), "/resolveuid/abc123") From 68f1f42f3ca893dc3f1c7ff288a1c74fd58c3156 Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Fri, 20 Oct 2023 10:36:19 +0200 Subject: [PATCH 09/11] Calculate a fragment of path - Make resolution of uid more robust - calculate a fragment of anchor if available --- .../browser/link_redirect_view.py | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/plone/app/contenttypes/browser/link_redirect_view.py b/plone/app/contenttypes/browser/link_redirect_view.py index d6747feb..645ee5b4 100644 --- a/plone/app/contenttypes/browser/link_redirect_view.py +++ b/plone/app/contenttypes/browser/link_redirect_view.py @@ -25,19 +25,42 @@ ] -def get_uid_from_path(url=None): +def normalize_uid_from_path(url=None): + """ + Args: + url (string): a path or orl + + Returns: + tuple: tuple of (uid, fragment) a fragment is an anchor id e.g. #head1 + """ + uid = None + fragment = None + if not url: - return None + return uid, fragment + # resolve uid paths = url.split("/") - uid = None - if "resolveuid" in paths: - ri = paths.index("resolveuid") + paths_lower = [_item.lower() for _item in paths] + + if "resolveuid" in paths_lower: + ri = paths_lower.index("resolveuid") if ri + 1 != len(paths): uid = paths[ri + 1] if uid == "": uid = None - return uid + + if not uid: + return uid, fragment + + # resolve fragment + parts = urlparse(uid) + + uid = parts.path + + fragment = f"#{parts.fragment}" if parts.fragment else None + + return uid, fragment class LinkRedirectView(BrowserView): @@ -124,12 +147,15 @@ def absolute_target_url(self): url = "/".join([context_state.canonical_object_url(), url]) else: if "resolveuid" in url: - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) obj = uuidToObject(uid) if obj is None: # uid can't resolve, return the url return url + url = obj.absolute_url() + if fragment is not None: + url = f"{url}{fragment}" if not url.startswith(("http://", "https://")): url = self.request["SERVER_URL"] + url From 798963fb294828c7731cf99b7749cb02e1223cb3 Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Fri, 20 Oct 2023 10:36:24 +0200 Subject: [PATCH 10/11] Update tests --- plone/app/contenttypes/tests/test_link.py | 93 ++++++++++++++++++----- 1 file changed, 75 insertions(+), 18 deletions(-) diff --git a/plone/app/contenttypes/tests/test_link.py b/plone/app/contenttypes/tests/test_link.py index 2287972a..702e4d99 100644 --- a/plone/app/contenttypes/tests/test_link.py +++ b/plone/app/contenttypes/tests/test_link.py @@ -1,5 +1,5 @@ from datetime import datetime -from plone.app.contenttypes.browser.link_redirect_view import get_uid_from_path +from plone.app.contenttypes.browser.link_redirect_view import normalize_uid_from_path from plone.app.contenttypes.interfaces import ILink from plone.app.contenttypes.testing import ( # noqa PLONE_APP_CONTENTTYPES_FUNCTIONAL_TESTING, @@ -231,66 +231,116 @@ def test_ftp_type(self): self.assertTrue(view()) self._assert_redirect(self.link.remoteUrl) - def test_get_uid_from_path(self): + def test_normalize_uid_from_path(self): url = None - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertIsNone(uid) + self.assertIsNone(fragment) url = "http://nohost" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertIsNone(uid) + self.assertIsNone(fragment) url = "http://nohost/test1" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertIsNone(uid) + self.assertIsNone(fragment) url = "http://nohost/test1/resolveuid" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertIsNone(uid) + self.assertIsNone(fragment) url = "http://nohost/test1/resolveuid/" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertIsNone(uid) + self.assertIsNone(fragment) url = "http://nohost/test1/resolveuid123/" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertIsNone(uid) + self.assertIsNone(fragment) url = "http://nohost/test1/resolveuid/123" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertIsNone(fragment) + + url = "http://nohost/test1/resolveuid/123#my-id" + uid, fragment = normalize_uid_from_path(url) + self.assertEqual(uid, "123") + self.assertEqual(fragment, "#my-id") url = "http://nohost/test1/resolveuid/123/" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertIsNone(fragment) + + url = "http://nohost/test1/resolveuid/123#my-id/" + uid, fragment = normalize_uid_from_path(url) + self.assertEqual(uid, "123") + self.assertEqual(fragment, "#my-id") url = "http://nohost/test1/resolveuid/123/test" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertIsNone(fragment) url = "resolveuid/123" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertIsNone(fragment) + + url = "resolveuid/123#my-id" + uid, fragment = normalize_uid_from_path(url) + self.assertEqual(uid, "123") + self.assertEqual(fragment, "#my-id") url = "resolveuid/123/" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) + self.assertEqual(uid, "123") + self.assertIsNone(fragment) + + url = "resolveuid/123#my-id/" + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertEqual(fragment, "#my-id") url = "resolveuid/123/abc" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertIsNone(fragment) url = "/resolveuid/123" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) + self.assertEqual(uid, "123") + self.assertIsNone(fragment) + + url = "/resolveuid/123#my-id" + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertEqual(fragment, "#my-id") url = "/resolveuid/123/" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) + self.assertEqual(uid, "123") + self.assertIsNone(fragment) + + url = "/resolveuid/123#my-id/" + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertEqual(fragment, "#my-id") url = "/resolveuid/123/abc" - uid = get_uid_from_path(url) + uid, fragment = normalize_uid_from_path(url) + self.assertEqual(uid, "123") + self.assertIsNone(fragment) + + url = "/resolveUid/123#my-id" + uid, fragment = normalize_uid_from_path(url) self.assertEqual(uid, "123") + self.assertEqual(fragment, "#my-id") def _publish(self, obj): portal_workflow = getToolByName(self.portal, "portal_workflow") @@ -436,15 +486,22 @@ def test_resolve_uid_to_absolute_target(self): ) doc1 = self.portal["doc1"] uid = IUUID(doc1) - self.link.remoteUrl = f"${{portal_url}}/resolveuid/{uid}" portal_state = getMultiAdapter( (self.link, self.request), name="plone_portal_state" ) portal_url = portal_state.portal_url() + # check an internal link + self.link.remoteUrl = f"${{portal_url}}/resolveuid/{uid}" self.assertEqual(view.absolute_target_url(), f"{portal_url}/doc1") + # check an internal link with fragment + self.link.remoteUrl = f"${{portal_url}}/resolveuid/{uid}#autotoc-item-autotoc-1" + self.assertEqual( + view.absolute_target_url(), f"{portal_url}/doc1#autotoc-item-autotoc-1" + ) + # check not resolvable uid self.link.remoteUrl = "/resolveuid/abc123" self.assertEqual(view.absolute_target_url(), "/resolveuid/abc123") From 614d5a4691f3ba3bd53506b6251973d2bd11041f Mon Sep 17 00:00:00 2001 From: 1letter <1letter@gmx.de> Date: Fri, 20 Oct 2023 15:11:22 +0200 Subject: [PATCH 11/11] Update 671.bugfix add empty line at the end of news file --- news/671.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/671.bugfix b/news/671.bugfix index 00b5b905..074ab236 100644 --- a/news/671.bugfix +++ b/news/671.bugfix @@ -1 +1 @@ -Fix link_redirect_view, respect vhm vs none-vhm url schemes @1letter \ No newline at end of file +Fix link_redirect_view, respect vhm vs none-vhm url schemes @1letter