Removed the docstring from methods to avoid publishing them [6.0] #3282
Conversation
…able via a url. From [Products.PloneHotfix20210518](https://plone.org/security/hotfix/20210518/reflected-xss-in-various-spots).
This comment has been minimized.
This comment has been minimized.
|
@jenkins-plone-org please run jobs |
|
Why not make the methods private by using |
|
At least for Plone 6 and Zope 5 that would be the perfect timing for doing such a breaking change, otherwise we would have to wait until Plone 7 and Zope 6? 😅 that would be far too long IMHO |
I think the idea of the PloneTool is to provide functionality to be used in untrusted templates, but are not meant to be directly published. Well, overall today one would write this different. There are probably multiple places to improve here. |
Indeed. Untrusted code should be able to call these. But they should not be available at a url, because that would make reflected XSS easy. I think this is a valid use case, which would break when docstrings are not required anymore. That does seem the right direction though. And the use case gets less important, because less code is in skins these days. So we could choose to tighten the security in Plone 6, in preparation for stricter handling in Zope. Maybe start saying that doing things in skins is deprecated, and people should move to browser views and resources. (But Plone itself still uses a few skin folders.) That is a wider discussion though, not something for a random PR. Anyway, for now I think the changes in this PR are fine. Can someone approve? @gforcada The checks expect a Jenkins Plone 6.0 Python 3.6 job, but this is not supported, so it can be ignored. Do you think this would be fixed by calling "Re-create GitHub commit hooks" on https://jenkins.plone.org/roboto/ ? It is probably safe to try, and I can do that. |
|
@mauritsvanrees no not there, that's on the mr.roboto configuration: plone/mr.roboto#81 I will check why 3.6 is still expected for 6.0 |
|
I see that the configuration looks fine, so it might be a bug on mr.roboto? 🤔 |
|
Didn't we dropped Python 3.6 for Plone 6.0? |
Yes we did. Well, officially this is still pending an okay from the framework team. But when we sat with a few people a month or two ago to fix some Jenkins stuff, we removed 3.6 from the checks already. (Could be reactivated if the framework team decides otherwise.) So the point now is that it is strange that 3.6 is still in the checks here. I have called the "Re-create GitHub commit hooks" command from mr.roboto, to see if that fixes it. It is still running, because it needs to touch a lot of repositories. I see it is also adding hooks to some Guillotina repositories. I hope that is okay. Ah, it seems finished, no further logs are coming in. Nothing has changed in this PR, but perhaps in a fresh PR it will work correctly. Anyway, this PR is approved, I will merge. |
|
@mauritsvanrees the hooks are to make repositories (not PR) to mr.roboto, so that whenever an action happens in a repository mr.roboto knows about it, it has nothing to do with which jobs a PR is expected to run 😅 The code for that is https://github.com/plone/mr.roboto/blob/3fd95d071d62f30caf6d55028767cedbde651439/src/mr.roboto/src/mr/roboto/subscriber.py#L375 (I think 😅 I had only a brief look at the code in 2 seconds) |
From Products.PloneHotfix20210518.