Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PloneSite acquired for itself in aq_chain #169

Open
jaroel opened this issue Oct 20, 2023 · 2 comments
Open

PloneSite acquired for itself in aq_chain #169

jaroel opened this issue Oct 20, 2023 · 2 comments

Comments

@jaroel
Copy link
Member

jaroel commented Oct 20, 2023

We had https://github.com/zopefoundation/Products.CMFCore/blob/master/src/Products/CMFCore/explicitacquisition.py enabled, which made requests for (at least) site root fail.

The CMFCore behaviour is correct, just don't know why we have PloneSite twice in the aq_chain.

/Users/roel/.buildout/eggs/cp311/Products.CMFCore-3.2-py3.11.egg/Products/CMFCore/explicitacquisition.py(30)content_allowed()
-> return context.aq_chain == context.aq_inner.aq_chain
(Pdb) context
<PloneSite at /plone used for /plone>
(Pdb) context.aq_chain
[<PloneSite at /plone used for /plone>, <PloneSite at /plone>, , <ZPublisher.BaseRequest.RequestContainer object at 0x120a1f710>]
(Pdb) context.aq_inner.aq_chain
[<PloneSite at /plone>, , <ZPublisher.BaseRequest.RequestContainer object at 0x120a1f710>]
(Pdb)

Reproduce: explicitly enable the check or disable skipping it and then run the test test_site_root_get_request.

@wesleybl
Copy link
Member

wesleybl commented Oct 20, 2023

@jaroel I took a look at this problem and it's not the fault of plone.rest but rather the way ZPublihser handles traversers of type ++api++.

In the URL:

http://localhost:8080/plone/++api++

When ZPublihser resolves the name ++api++ it considers that the parent container of ++api++ is the Plone Site. It then calls plone.rest's traverse method, which returns the object referent to ++api++ as being the Plone site as well:

def traverse(self, name_ignored, subpath_ignored):
name = "/++api++"
url = self.request.ACTUAL_URL
if url.count(name) > 1:
# Redirect to proper url.
while f"{name}{name}" in url:
url = url.replace(f"{name}{name}", name)
if url.count(name) > 1:
# Something like: .../++api++/something/++api++
# Return nothing, so a NotFound is raised.
return
# Raise a redirect exception to stop execution of the current request.
raise Redirect(url)
mark_as_api_request(self.request, "application/json")
return self.context

When ZPublihser resolves names that start with "+" it sets the of of the object returned by the traverse method with the parent container of ++api++. Which in this case are the same object. So that's why the Plone site is acquired by itself.

I don't think this behavior needs to be changed. Perhaps what needs to be changed is the way Product.CMFCore discovers whether an object is acquired or not.

@jaroel
Copy link
Member Author

jaroel commented Oct 23, 2023

While the aq_chain check might not catch all edge cases yet, the goal of the ++api++ namespace is to mark the request as an API request, like the doc string says:

class MarkAsRESTTraverser:
"""
Traversal adapter for the ``++api++`` namespace.
It marks the request as API request.
"""
. It shouldn't affect the aq_chain imho, and the problem should be addressed here.

I think your comment is helpful, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants