-
-
Notifications
You must be signed in to change notification settings - Fork 75
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Branch: refs/heads/master Date: 2023-09-19T12:39:57+02:00 Author: Maurits van Rees (mauritsvanrees) <[email protected]> Commit: plone/plone.rest@db0cb9a When ++api++ is in the url multiple times, redirect to the proper url. When the url is badly formed, for example `++api++/something/++api++`, give a 404 NotFound. Fixes a denial of service. Files changed: A news/1.bugfix M src/plone/rest/tests/test_traversal.py M src/plone/rest/traverse.py Repository: plone.rest Branch: refs/heads/master Date: 2023-09-21T13:15:43+02:00 Author: Maurits van Rees (mauritsvanrees) <[email protected]> Commit: plone/plone.rest@77846a9 Merge pull request from GHSA-h6rp-mprm-xgcq When ++api++ is in the url multiple times, redirect to the proper url. [master] Files changed: A news/1.bugfix M src/plone/rest/tests/test_traversal.py M src/plone/rest/traverse.py
- Loading branch information
1 parent
7a691d4
commit e6bb173
Showing
1 changed file
with
19 additions
and
48 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,68 +1,39 @@ | ||
| Repository: plone.testing | ||
| Repository: plone.rest | ||
|
|
||
|
|
||
| Branch: refs/heads/master | ||
| Date: 2023-09-20T21:24:17+02:00 | ||
| Date: 2023-09-19T12:39:57+02:00 | ||
| Author: Maurits van Rees (mauritsvanrees) <[email protected]> | ||
| Commit: https://github.com/plone/plone.testing/commit/786074e81efb0c2c2f9f489191f6de1629ff4fd4 | ||
| Commit: https://github.com/plone/plone.rest/commit/db0cb9adb7f483bffb3e1ab06d1270c7db0b04a8 | ||
|
|
||
| Fix tests when run with ZODB 5.8.1+. | ||
| When ++api++ is in the url multiple times, redirect to the proper url. | ||
|
|
||
| Test failures: | ||
| ``` | ||
| Failure in test /Users/maurits/community/plone-coredev/6.0/src/plone.testing/src/plone/testing/zodb.rst | ||
| Failed doctest test for zodb.rst | ||
| File "/Users/maurits/community/plone-coredev/6.0/src/plone.testing/src/plone/testing/zodb.rst", line 0 | ||
|
|
||
| ---------------------------------------------------------------------- | ||
| File "/Users/maurits/community/plone-coredev/6.0/src/plone.testing/src/plone/testing/zodb.rst", line 55, in zodb.rst | ||
| Failed example: | ||
| zodb.EMPTY_ZODB['zodbConnection'] | ||
| Expected: | ||
| <Connection at ...> | ||
| Got: | ||
| <ZODB.Connection.Connection object at 0x1071e7f10> | ||
| ---------------------------------------------------------------------- | ||
| File "/Users/maurits/community/plone-coredev/6.0/src/plone.testing/src/plone/testing/zodb.rst", line 138, in zodb.rst | ||
| Failed example: | ||
| POPULATED_ZODB['zodbConnection'] | ||
| Expected: | ||
| <Connection at ...> | ||
| Got: | ||
| <ZODB.Connection.Connection object at 0x107256a10> | ||
| ---------------------------------------------------------------------- | ||
| File "/Users/maurits/community/plone-coredev/6.0/src/plone.testing/src/plone/testing/zodb.rst", line 233, in zodb.rst | ||
| Failed example: | ||
| EXPANDED_ZODB['zodbConnection'] | ||
| Expected: | ||
| <Connection at ...> | ||
| Got: | ||
| <ZODB.Connection.Connection object at 0x107268550> | ||
| ``` | ||
| When the url is badly formed, for example `++api++/something/++api++`, give a 404 NotFound. | ||
| Fixes a denial of service. | ||
|
|
||
| Files changed: | ||
| A news/581.bugfix | ||
| M pyproject.toml | ||
| M src/plone/testing/zodb.rst | ||
| A news/1.bugfix | ||
| M src/plone/rest/tests/test_traversal.py | ||
| M src/plone/rest/traverse.py | ||
|
|
||
| b'diff --git a/news/581.bugfix b/news/581.bugfix\nnew file mode 100644\nindex 0000000..69f5d06\n--- /dev/null\n+++ b/news/581.bugfix\n@@ -0,0 +1,2 @@\n+Fix tests when run with ZODB 5.8.1+.\n+[maurits]\ndiff --git a/pyproject.toml b/pyproject.toml\nindex 05b615d..12c6bc8 100644\n--- a/pyproject.toml\n+++ b/pyproject.toml\n@@ -1,6 +1,6 @@\n [tool.towncrier]\n-filename = "CHANGES.rst"\n directory = "news/"\n+filename = "CHANGES.rst"\n title_format = "{version} ({project_date})"\n underlines = ["-", ""]\n \n@@ -18,3 +18,21 @@ showcontent = true\n directory = "bugfix"\n name = "Bug fixes:"\n showcontent = true\n+\n+[[tool.towncrier.type]]\n+directory = "internal"\n+name = "Internal:"\n+showcontent = true\n+\n+[[tool.towncrier.type]]\n+directory = "documentation"\n+name = "Documentation:"\n+showcontent = true\n+\n+[[tool.towncrier.type]]\n+directory = "tests"\n+name = "Tests"\n+showcontent = true\n+\n+[tool.isort]\n+profile = "plone"\ndiff --git a/src/plone/testing/zodb.rst b/src/plone/testing/zodb.rst\nindex 6a9cf73..a18c473 100644\n--- a/src/plone/testing/zodb.rst\n+++ b/src/plone/testing/zodb.rst\n@@ -53,7 +53,7 @@ Let\'s now simulate a test.::\n The test would then execute. It may use the ZODB root.::\n \n >>> zodb.EMPTY_ZODB[\'zodbConnection\']\n- <Connection at ...>\n+ <...Connection...at ...>\n \n >>> zodb.EMPTY_ZODB[\'zodbRoot\']\n {}\n@@ -136,7 +136,7 @@ Let\'s now simulate a test.::\n The test would then execute. It may use the ZODB root.::\n \n >>> POPULATED_ZODB[\'zodbConnection\']\n- <Connection at ...>\n+ <...Connection...at ...>\n \n >>> POPULATED_ZODB[\'zodbRoot\']\n {\'someData\': \'a string\'}\n@@ -231,7 +231,7 @@ Let\'s now simulate a test.::\n The test would then execute. It may use the ZODB root.::\n \n >>> EXPANDED_ZODB[\'zodbConnection\']\n- <Connection at ...>\n+ <...Connection...at ...>\n \n >>> EXPANDED_ZODB[\'zodbRoot\'] == dict(someData=\'a string\', additionalData=\'Some new data\')\n True\n' | ||
| b'diff --git a/news/1.bugfix b/news/1.bugfix\nnew file mode 100644\nindex 0000000..019268d\n--- /dev/null\n+++ b/news/1.bugfix\n@@ -0,0 +1,5 @@\n+When ``++api++`` is in the url multiple times, redirect to the proper url.\n+When the url is badly formed, for example ``++api++/something/++api++``, give a 404 NotFound.\n+Fixes a denial of service.\n+See `security advisory <https://github.com/plone/plone.rest/security/advisories/GHSA-h6rp-mprm-xgcq>`_.\n+[maurits]\ndiff --git a/src/plone/rest/tests/test_traversal.py b/src/plone/rest/tests/test_traversal.py\nindex 963d1c4..5d5389d 100644\n--- a/src/plone/rest/tests/test_traversal.py\n+++ b/src/plone/rest/tests/test_traversal.py\n@@ -10,6 +10,8 @@\n from plone.app.testing import TEST_USER_ID\n from plone.rest.service import Service\n from plone.rest.testing import PLONE_REST_INTEGRATION_TESTING\n+from zExceptions import NotFound\n+from zExceptions import Redirect\n from zope.event import notify\n from zope.interface import alsoProvides\n from zope.publisher.interfaces.browser import IBrowserView\n@@ -106,6 +108,34 @@ def test_html_request_on_existing_view_returns_view(self):\n obj = self.traverse(path="/plone/folder1/search", accept="text/html")\n self.assertFalse(isinstance(obj, Service), "Got a service")\n \n+ def test_html_request_via_api_returns_service(self):\n+ obj = self.traverse(path="/plone/++api++", accept="text/html")\n+ self.assertTrue(isinstance(obj, Service), "Not a service")\n+\n+ def test_html_request_via_double_apis_raises_redirect(self):\n+ portal_url = self.portal.absolute_url()\n+ with self.assertRaises(Redirect) as exc:\n+ self.traverse(path="/plone/++api++/++api++", accept="text/html")\n+ self.assertEqual(\n+ exc.exception.headers["Location"],\n+ f"{portal_url}/++api++",\n+ )\n+\n+ def test_html_request_via_multiple_apis_raises_redirect(self):\n+ portal_url = self.portal.absolute_url()\n+ with self.assertRaises(Redirect) as exc:\n+ self.traverse(\n+ path="/plone/++api++/++api++/++api++/search", accept="text/html"\n+ )\n+ self.assertEqual(\n+ exc.exception.headers["Location"],\n+ f"{portal_url}/++api++/search",\n+ )\n+\n+ def test_html_request_via_multiple_bad_apis_raises_not_found(self):\n+ with self.assertRaises(NotFound):\n+ self.traverse(path="/plone/++api++/search/++api++", accept="text/html")\n+\n def test_virtual_hosting(self):\n app = self.layer["app"]\n vhm = VirtualHostMonster()\ndiff --git a/src/plone/rest/traverse.py b/src/plone/rest/traverse.py\nindex f8d4a23..0a151c8 100644\n--- a/src/plone/rest/traverse.py\n+++ b/src/plone/rest/traverse.py\n@@ -5,6 +5,7 @@\n from plone.rest.interfaces import IAPIRequest\n from plone.rest.interfaces import IService\n from plone.rest.events import mark_as_api_request\n+from zExceptions import Redirect\n from zope.component import adapter\n from zope.component import queryMultiAdapter\n from zope.interface import implementer\n@@ -64,6 +65,18 @@ def __init__(self, context, request):\n self.request = request\n \n def traverse(self, name_ignored, subpath_ignored):\n+ name = "/++api++"\n+ url = self.request.ACTUAL_URL\n+ if url.count(name) > 1:\n+ # Redirect to proper url.\n+ while f"{name}{name}" in url:\n+ url = url.replace(f"{name}{name}", name)\n+ if url.count(name) > 1:\n+ # Something like: .../++api++/something/++api++\n+ # Return nothing, so a NotFound is raised.\n+ return\n+ # Raise a redirect exception to stop execution of the current request.\n+ raise Redirect(url)\n mark_as_api_request(self.request, "application/json")\n return self.context\n \n' | ||
|
|
||
| Repository: plone.testing | ||
| Repository: plone.rest | ||
|
|
||
|
|
||
| Branch: refs/heads/master | ||
| Date: 2023-09-20T22:17:18+02:00 | ||
| Date: 2023-09-21T13:15:43+02:00 | ||
| Author: Maurits van Rees (mauritsvanrees) <[email protected]> | ||
| Commit: https://github.com/plone/plone.testing/commit/5cbdff3fa9d1a7b1102d6807db73a059203af475 | ||
| Commit: https://github.com/plone/plone.rest/commit/77846a9842889b24f35e8bedc2e9d461388d3302 | ||
|
|
||
| Merge pull request #84 from plone/maurits-fix-zodb-581-tests | ||
| Merge pull request from GHSA-h6rp-mprm-xgcq | ||
|
|
||
| Fix tests when run with ZODB 5.8.1+. | ||
| When ++api++ is in the url multiple times, redirect to the proper url. [master] | ||
|
|
||
| Files changed: | ||
| A news/581.bugfix | ||
| M pyproject.toml | ||
| M src/plone/testing/zodb.rst | ||
| A news/1.bugfix | ||
| M src/plone/rest/tests/test_traversal.py | ||
| M src/plone/rest/traverse.py | ||
|
|
||
| b'diff --git a/news/581.bugfix b/news/581.bugfix\nnew file mode 100644\nindex 0000000..69f5d06\n--- /dev/null\n+++ b/news/581.bugfix\n@@ -0,0 +1,2 @@\n+Fix tests when run with ZODB 5.8.1+.\n+[maurits]\ndiff --git a/pyproject.toml b/pyproject.toml\nindex 05b615d..12c6bc8 100644\n--- a/pyproject.toml\n+++ b/pyproject.toml\n@@ -1,6 +1,6 @@\n [tool.towncrier]\n-filename = "CHANGES.rst"\n directory = "news/"\n+filename = "CHANGES.rst"\n title_format = "{version} ({project_date})"\n underlines = ["-", ""]\n \n@@ -18,3 +18,21 @@ showcontent = true\n directory = "bugfix"\n name = "Bug fixes:"\n showcontent = true\n+\n+[[tool.towncrier.type]]\n+directory = "internal"\n+name = "Internal:"\n+showcontent = true\n+\n+[[tool.towncrier.type]]\n+directory = "documentation"\n+name = "Documentation:"\n+showcontent = true\n+\n+[[tool.towncrier.type]]\n+directory = "tests"\n+name = "Tests"\n+showcontent = true\n+\n+[tool.isort]\n+profile = "plone"\ndiff --git a/src/plone/testing/zodb.rst b/src/plone/testing/zodb.rst\nindex 6a9cf73..a18c473 100644\n--- a/src/plone/testing/zodb.rst\n+++ b/src/plone/testing/zodb.rst\n@@ -53,7 +53,7 @@ Let\'s now simulate a test.::\n The test would then execute. It may use the ZODB root.::\n \n >>> zodb.EMPTY_ZODB[\'zodbConnection\']\n- <Connection at ...>\n+ <...Connection...at ...>\n \n >>> zodb.EMPTY_ZODB[\'zodbRoot\']\n {}\n@@ -136,7 +136,7 @@ Let\'s now simulate a test.::\n The test would then execute. It may use the ZODB root.::\n \n >>> POPULATED_ZODB[\'zodbConnection\']\n- <Connection at ...>\n+ <...Connection...at ...>\n \n >>> POPULATED_ZODB[\'zodbRoot\']\n {\'someData\': \'a string\'}\n@@ -231,7 +231,7 @@ Let\'s now simulate a test.::\n The test would then execute. It may use the ZODB root.::\n \n >>> EXPANDED_ZODB[\'zodbConnection\']\n- <Connection at ...>\n+ <...Connection...at ...>\n \n >>> EXPANDED_ZODB[\'zodbRoot\'] == dict(someData=\'a string\', additionalData=\'Some new data\')\n True\n' | ||
| b'diff --git a/news/1.bugfix b/news/1.bugfix\nnew file mode 100644\nindex 0000000..019268d\n--- /dev/null\n+++ b/news/1.bugfix\n@@ -0,0 +1,5 @@\n+When ``++api++`` is in the url multiple times, redirect to the proper url.\n+When the url is badly formed, for example ``++api++/something/++api++``, give a 404 NotFound.\n+Fixes a denial of service.\n+See `security advisory <https://github.com/plone/plone.rest/security/advisories/GHSA-h6rp-mprm-xgcq>`_.\n+[maurits]\ndiff --git a/src/plone/rest/tests/test_traversal.py b/src/plone/rest/tests/test_traversal.py\nindex 963d1c4..5d5389d 100644\n--- a/src/plone/rest/tests/test_traversal.py\n+++ b/src/plone/rest/tests/test_traversal.py\n@@ -10,6 +10,8 @@\n from plone.app.testing import TEST_USER_ID\n from plone.rest.service import Service\n from plone.rest.testing import PLONE_REST_INTEGRATION_TESTING\n+from zExceptions import NotFound\n+from zExceptions import Redirect\n from zope.event import notify\n from zope.interface import alsoProvides\n from zope.publisher.interfaces.browser import IBrowserView\n@@ -106,6 +108,34 @@ def test_html_request_on_existing_view_returns_view(self):\n obj = self.traverse(path="/plone/folder1/search", accept="text/html")\n self.assertFalse(isinstance(obj, Service), "Got a service")\n \n+ def test_html_request_via_api_returns_service(self):\n+ obj = self.traverse(path="/plone/++api++", accept="text/html")\n+ self.assertTrue(isinstance(obj, Service), "Not a service")\n+\n+ def test_html_request_via_double_apis_raises_redirect(self):\n+ portal_url = self.portal.absolute_url()\n+ with self.assertRaises(Redirect) as exc:\n+ self.traverse(path="/plone/++api++/++api++", accept="text/html")\n+ self.assertEqual(\n+ exc.exception.headers["Location"],\n+ f"{portal_url}/++api++",\n+ )\n+\n+ def test_html_request_via_multiple_apis_raises_redirect(self):\n+ portal_url = self.portal.absolute_url()\n+ with self.assertRaises(Redirect) as exc:\n+ self.traverse(\n+ path="/plone/++api++/++api++/++api++/search", accept="text/html"\n+ )\n+ self.assertEqual(\n+ exc.exception.headers["Location"],\n+ f"{portal_url}/++api++/search",\n+ )\n+\n+ def test_html_request_via_multiple_bad_apis_raises_not_found(self):\n+ with self.assertRaises(NotFound):\n+ self.traverse(path="/plone/++api++/search/++api++", accept="text/html")\n+\n def test_virtual_hosting(self):\n app = self.layer["app"]\n vhm = VirtualHostMonster()\ndiff --git a/src/plone/rest/traverse.py b/src/plone/rest/traverse.py\nindex f8d4a23..0a151c8 100644\n--- a/src/plone/rest/traverse.py\n+++ b/src/plone/rest/traverse.py\n@@ -5,6 +5,7 @@\n from plone.rest.interfaces import IAPIRequest\n from plone.rest.interfaces import IService\n from plone.rest.events import mark_as_api_request\n+from zExceptions import Redirect\n from zope.component import adapter\n from zope.component import queryMultiAdapter\n from zope.interface import implementer\n@@ -64,6 +65,18 @@ def __init__(self, context, request):\n self.request = request\n \n def traverse(self, name_ignored, subpath_ignored):\n+ name = "/++api++"\n+ url = self.request.ACTUAL_URL\n+ if url.count(name) > 1:\n+ # Redirect to proper url.\n+ while f"{name}{name}" in url:\n+ url = url.replace(f"{name}{name}", name)\n+ if url.count(name) > 1:\n+ # Something like: .../++api++/something/++api++\n+ # Return nothing, so a NotFound is raised.\n+ return\n+ # Raise a redirect exception to stop execution of the current request.\n+ raise Redirect(url)\n mark_as_api_request(self.request, "application/json")\n return self.context\n \n' | ||
|
|