Skip to content

Avoid a plone.protect warning on index_html documents in portal root. #4281

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.

Sign up for GitHub

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

Merged
thet merged 1 commit into from
Feb 23, 2026
+37 −2
Merged

Avoid a plone.protect warning on index_html documents in portal root. #4281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions news/4279.bugfix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Avoid a plone.protect warning on `index_html` documents in portal root.

`index_html` are often used to create default pages in containers. On the
portal root this was prevented by a problem in a name collision checker in
plone.base, fixed in https://github.com/plone/plone.base/pull/107 and a
plone.protect warning issued by a write-on-read by setting the
`__replaceable__` attribute on the `index_html` object.

Fixes: https://github.com/plone/Products.CMFPlone/issues/4279
[thet]
3 changes: 1 addition & 2 deletions src/Products/CMFPlone/Portal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,9 @@ def index_html(self):
return result
elif method not in ("GET", "HEAD", "POST"):
raise AttributeError("index_html")
# Acquire from skin.
# Acquire content from here.
_target = self.__getattr__("index_html")
result = aq_base(_target).__of__(self)
setattr(result, "__replaceable__", REPLACEABLE)
Copy link
Member

@ale-rt ale-rt Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you remove this one you have to remove also the one in https://github.com/plone/Products.CMFPlone/pull/4281/changes#diff-108b30c9523956cc50ba0e46e6221344a0f9c01ad8c53e663a11c83ca854f248R171
for consistency (even if it is probably dead code).

Copy link
Member Author

@thet thet Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ale-rt happy to remove it! But in the case of the WebDAV __replaceable I thought it might be OK, as this is a NullResource and someone might want to replace it with real content named index_html. But honestly, I'm guessing because I don't really know what the __replaceable__ even is.

Btw. I know some people still using WebDAV and IMO it could be a killer feature of Plone if it would work fine.

Copy link
Member

@davisagli davisagli Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thet @ale-rt __replaceable__ is used in https://github.com/zopefoundation/Zope/blob/c927fc8b004c624b1c86f359be00d138e98c34f7/src/OFS/ObjectManager.py#L122 to prevent giving a new item a name that can already be found by getattr -- unless the object is marked as replaceable.

So this was kind of there to handle exactly the problem that you're trying to solve (being able to replace the index_html even though it is a method). Except the _check_for_collision in plone.base is not as smart as checkValidId in Zope, and doesn't pay attention to __replaceable__.

tl;dr: I think this change is ok as long as we merge it together with the one you made in plone.base

Copy link
Member Author

@thet thet Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ale-rt I'm undecided - is it now better to remove the __replaceable__ attribute setting for the WebDAV NullResource or not?

Copy link
Member

@ale-rt ale-rt Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that anyway having a Plone server behind Zserver is very unlikely.
If somebody has issues he will ask for a change.

return result

index_html = ComputedAttribute(index_html, 1)
Expand Down
26 changes: 26 additions & 0 deletions src/Products/CMFPlone/tests/testPortalCreation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,32 @@ def testSyndicationTabDisabled(self):
if action.getId() == "syndication" and action.visible:
self.fail("Actions tool still has visible 'syndication' action")

def testIndexHtmlNotInvokingPloneProtect(self):
"""A `index_html` in the portal root should not invoke a plone.protect
exception.
"""
from plone.app.testing import SITE_OWNER_NAME
from plone.app.testing import SITE_OWNER_PASSWORD
from plone.testing.zope import Browser

browser = Browser(self.layer["app"])
browser.open(self.portal.absolute_url() + "/login_form")

browser.getControl("Login Name").value = SITE_OWNER_NAME
browser.getControl("Password").value = SITE_OWNER_PASSWORD
browser.getControl("Log in").click()

browser.open(self.portal.absolute_url() + "/++add++Document")
browser.getControl("Title").value = "index_html"
browser.getControl("Save").click()
browser.open(self.portal.absolute_url() + "/index_html")

contents = browser.contents

# Check, if plone.protect confirm view isn't shown.
self.assertNotIn("exploit", contents)
self.assertNotIn("Confirm action", contents)

def testObjectButtonActionsInvisibleOnPortalDefaultDocument(self):
# only a manager would have proper permissions
self.setRoles(["Manager", "Member"])
Expand Down