Checkout Zope and use its master versions.

[mauritsvanrees]

There are several major updates there, which should be fine: they drop Python 2 support.

I do see that several pins have been dropped, so we need to add more ourselves. This is the output when allowing unpinned versions:

[versions]
mock = 5.0.1
zope.sendmail = 5.3

# Required by:
# plone.recipe.zope2instance==6.12.0
ZEO = 5.4.0

# Required by:
# repoze.xmliter==0.6.1
future = 0.18.3

# Required by:
# ZEO==5.4.0
zdaemon = 4.4

# Required by:
# five.customerize==2.1.0
# plone.contentrules==2.1.2
zope.componentvocabulary = 2.3.0

# Required by:
# plone.app.caching==3.0.4.dev0
# plone.memoize==3.0.0
zope.ramcache = 2.4

I added them.

[mauritsvanrees]

plone.dexterity tests fail on Zope master, because Zope fixed a typo 'succeded'. I am fixing that in https://github.com/plone/plone.dexterity/pull/173. Let's retry the current PR when that one is merged.

[mauritsvanrees]

I rebased, squashed, and force pushed. Should work now.

@jenkins-plone-org please run jobs

[jensens]

I would say, merge yourself if ready.

[mauritsvanrees]

In all jobs 19 failing tests. Really? :-( example 3.11 job At least some have the same root cause, so maybe it is just one or two fixes somewhere. Possibly we need a few other new packages, I don't know. I am not likely to continue this today, so if someone else wants to try: go ahead.

[jensens]

In all jobs 19 failing tests. Really? :-(

see my condition - I would not call it ready ;)

[jensens]

looks like the signature changed and the test dummy no longer works

https://github.com/zopefoundation/Zope/blob/master/src/ZPublisher/HTTPRequest.py#L1486 https://github.com/plone/Products.PlonePAS/blob/master/src/Products/PlonePAS/tests/dummy.py#L30

commit introducing this is https://github.com/zopefoundation/Zope/commit/5b324f6c461f5ea1cc069739b6c32a1a5ff59df9

[ale-rt]

We could change the signature in Zope master to def __init__(self, aFieldStorage, charset="latin-1"): to make it backward compatible.

[mauritsvanrees]

@jenkins-plone-org please run jobs

[mauritsvanrees]

Three robot tests fail. They are the same across all four Python versions.

TLDR: I found the problem and it needs a one-line fix in Zope.

I looked at this one and I have the same test failure locally and can reproduce it in an actual site:

• Use the buildout.coredev branch from this PR, use Python 3.11 to bootstrap and buildout it.
• Create a Plone Site.
• Go to the folder contents of the site root.
• Click the Rearrange button and reverse the sort order: nothing happens, and I see no error.
• Alternatively, drag and drop an item to change the order. Now you get an error:

Traceback (innermost last):
  Module ZPublisher.WSGIPublisher, line 181, in transaction_pubevents
  Module ZPublisher.WSGIPublisher, line 390, in publish_module
  Module ZPublisher.WSGIPublisher, line 285, in publish
  Module ZPublisher.mapply, line 85, in mapply
  Module ZPublisher.WSGIPublisher, line 68, in call_object
  Module plone.app.content.browser.contents.rearrange, line 43, in __call__
KeyError: 'delta'

That is on this line:

delta = self.request.form["delta"]

I traced this back to Zope: if I go to src/Zope, checkout tag 5.8, and retry, all is well. Looking more specifically, it is caused by the changes from https://github.com/zopefoundation/Zope/pull/1094, which replaces cgi.FieldStorage by multipart. With https://github.com/zopefoundation/Zope/commit/5b324f6c461f5ea1cc069739b6c32a1a5ff59df9 it fails, with one earlier, it works.

@d-maurer, that is your PR. Do you have an idea what causes this?

On a (POST) request where reordering works, I have this:

(Pdb++) self.request
<WSGIRequest, URL=http://localhost:8080/Plone5/fc-itemOrder>
(Pdb++) self.request.form
{'delta': '-1', 'id': 'events', '_authenticator': '19599becb83ff63f067ba031a4622eab0ff90ccb', 'subsetIds': '["news","events","Members"]'}
(Pdb++) pp self.request.items()
...
 ('delta', '-1'),

On a request where reordering fails, I get:

(Pdb++) self.request
<WSGIRequest, URL=http://localhost:8080/Plone5/fc-itemOrder>
(Pdb++) self.request.form
{}
(Pdb++) pp self.request.items()
...
 ('BODY', b'delta=-1&id=events&_authenticator=4a94398e168eb87302d55598839313413329f964&subsetIds=%5B%22news%22%2C%22events%22%2C%22Members%22%5D'),

So when it fails, the request has a BODY and no form.

Ah, found it! It is this line from your PR:

elif content_type == "application/x-www-form-urlencoded": 

In my case I have this:

(Pdb++) content_type
'application/x-www-form-urlencoded; charset=UTF-8'

So the condition does not match. It should be:

elif content_type.startswith("application/x-www-form-urlencoded"):

Then reordering works, and the test passes.

@d-maurer does that make sense? I can make a small PR, but I am getting sleepy, so that will have to wait a bit. Maybe also get my other PR in the same area approved and merged first.

[d-maurer]

Maurits van Rees wrote at 2023-3-8 15:48 -0800:

... ... Ah, found it! It is this line from your PR:

elif content_type == "application/x-www-form-urlencoded":

In my case I have this:

(Pdb++) content_type
'application/x-www-form-urlencoded; charset=UTF-8'

So the condition does not match. It should be:

elif content_type.startswith("application/x-www-form-urlencoded"):

Then reordering works, and the test passes.

@d-maurer does that make sense?

In principle, yes. BUT application/x-www-form-urlencoded is defined by "https://www.w3.org/TR/2014/REC-html5-20141028/iana.html": 12.4 application/x-www-form-urlencoded

This registration has been filed successfully with IANA.

Type name: application Subtype name: x-www-form-urlencoded Required parameters: No parameters Optional parameters: No parameters

This means: application/x-www-form-urlencoded is expected to have no parameters - and especially no charset parameter.

For that reason, I used "==" instead of startswith.

I can make a small PR, but I am getting sleepy, so that will have to wait a bit.

Not sure whether we should make Zope support the use of a parameter not supported by the relevant standard and which it would not interpret.

I suggest, you make the tests standard conform, instead.

Maybe also get my other PR in the same area approved and merged first. dG>

Sorry: I had requested changes to this PR. I do not know why it was not listed (maybe, I forgot the "submit"). I have redone the change request (this time successful).

[mauritsvanrees]

@d-maurer wrote:

application/x-www-form-urlencoded is defined by "https://www.w3.org/TR/2014/REC-html5-20141028/iana.html": This means: application/x-www-form-urlencoded is expected to have no parameters - and especially no charset parameter. For that reason, I used "==" instead of startswith.

Interesting. Most others in that document allow a charset parameter, but indeed this one not.

Not sure whether we should make Zope support the use of a parameter not supported by the relevant standard and which it would not interpret. I suggest, you make the tests standard conform, instead.

This is not only in tests. I see this in an actual browser: it makes reordering in folder contents fail.

Okay, the folder contents code in mockup explicitly sets the Content-Type header this way here and here. So we could fix it there.

I see a few more though, at least these with a simple grep, there may be more:

plone/formwidget/recurrence/tests/test_z3cwidget.py:55:                "ajaxContentType": "application/x-www-form-urlencoded; charset=UTF-8",
plone/formwidget/recurrence/z3cform/widget.py:60:            ajaxContentType="application/x-www-form-urlencoded; charset=UTF-8",
zope/publisher/tests/test_browserrequest.py:422:            'CONTENT_TYPE': 'application/x-www-form-urlencoded; charset=UTF-8',
zope/publisher/tests/test_browserrequest.py:446:                'application/x-www-form-urlencoded; charset=ISO-8859-1'),
zope/publisher/tests/test_browserrequest.py:458:                'application/x-www-form-urlencoded; charset=ISO-8859-13'),

plone.formwidget.recurrence is the only one in this list where the problem is also in live code. This should mean that recurring events in Plone may have a problem as well, but I don't see this reflected in the tests. I did not try anything though.

For zope.publisher here is a list of search results. Some of its tests that use the no allowed charset parameter, were changed in this PR from 2021, which handles similar encoding problems as you did. Maybe the examples in the tests were just badly chosen.

So: I could update mockup and plone.formwidget.recurrence to no longer send the charset parameter. But I do wonder if it makes sense to make the Zope code more lenient, to avoid problems for other code out there that makes the same mistake.

[d-maurer]

Maurits van Rees wrote at 2023-3-9 01:11 -0800:

... But I do wonder if it makes sense to make the Zope code more lenient, to avoid problems for other code out there that makes the same mistake.

If we allow the parameter, its value should get interpreted, too. However, this would require significantly more work than the replacement of == by startswith. In fact, the current multipart integration into HTTPRequest.processInputs is based on the assumption that charset information comes either from default_encoding or via explicit conversion specs in the form variable names.

[d-maurer]

Dieter Maurer wrote at 2023-3-9 10:24 +0100:

Maurits van Rees wrote at 2023-3-9 01:11 -0800:

... But I do wonder if it makes sense to make the Zope code more lenient, to avoid problems for other code out there that makes the same mistake.

If we allow the parameter, its value should get interpreted, too. However, this would require significantly more work than the replacement of == by startswith. In fact, the current multipart integration into HTTPRequest.processInputs is based on the assumption that charset information comes either from default_encoding or via explicit conversion specs in the form variable names.

The server usually uses default_encoding as charset for its forms. Therefore, it can expect that received form data by default uses this charset as well.

If form data arrives with an explicit charset, then the client likely wants to tell the server that it has used this charset when it constructed the form data. This would mean that the server should use this explicit charset (rather than its default_encoding) as default during the processing of this form data. This suggests how a charset parameter should be supported by Zope (if it is supported at all).

Alternative: raise an exception when the unsupported charset parameter is encountered.

[mauritsvanrees]

I made two PRs to remove the charset:

• https://github.com/plone/mockup/pull/1292
• https://github.com/plone/plone.formwidget.recurrence/pull/31

Those fix a few real problems in the folder contents and for recurring events.

But reordering in the folder contents still does not work. And I have no idea which code is responsible for adding the Content-Type header application/x-www-form-urlencoded; charset=UTF-8. I would expect either mockup or Patternslib, but I don't see anything, other than what I am already fixing in the above PRs.

@thet @petschki Something in the folder contents code results in a Content-Type header application/x-www-form-urlencoded with a charset. This worked until now, although it is against the rules (as I learned today). With Zope master it fails. You can see this when you try to reorder items: the main Reorder button with a form does nothing. Dragging and dropping is clearer because it gives an error. Can you find out which code is responsible for this? I can't find it. I have a mockup PR linked above that solves similar problems in folder contents.

@d-maurer What about accepting the charset in the header when it is the same as the default_encoding? We would still ignore it, but at least we would use the wanted code path. When the charset does not match, we could log a warning and then ignore it.

[mauritsvanrees]

@jenkins-plone-org please run jobs

[d-maurer]

Maurits van Rees wrote at 2023-3-9 13:25 -0800:

... @d-maurer What about accepting the charset in the header when it is the same as the default_encoding? We would still ignore it, but at least we would use the wanted code path. When the charset does not match, we could log a warning and then ignore it.

I would not be happy.

Should you really be unable to get rid of the charset parameter for the application/x-www-form-urlencoded media type, I think that Zope should support the parameter fully. This will only be slightly more complex than what you have proposed above: currently, the initial request data processing returns a FieldStorage; to fully support the charset parameter, it would instead return a tuple FieldStorage, charset. If charset is not None, it is used (instead of default_encoding) as the default encoding for the current form data interpretation. Everything else can mostly remain as it is now (we might need to introduce a new local variable to replace default_encoding locally).

[petschki]

@thet @petschki Something in the folder contents code results in a Content-Type header application/x-www-form-urlencoded with a charset. This worked until now, although it is against the rules (as I learned today). With Zope master it fails. You can see this when you try to reorder items: the main Reorder button with a form does nothing. Dragging and dropping is clearer because it gives an error. Can you find out which code is responsible for this? I can't find it. I have a mockup PR linked above that solves similar problems in folder contents.

See my comment here https://github.com/plone/mockup/pull/1292#issuecomment-1462925362

[mauritsvanrees]

So all/most jQuery Ajax calls are the problem! Thanks for figuring that out @petschki.

At least with all the various PRs included, the jobs pass, so we are in the right direction.

[mauritsvanrees]

With Dieter's branch from https://github.com/zopefoundation/Zope/pull/1104/ both the folder contents and recurring events work again. Thanks! The only other PR we need is a test fix in https://github.com/plone/plone.formwidget.namedfile/pull/60 That is tested in this coredev PR.

Let's see if Jenkins is happy too:

@jenkins-plone-org please run jobs

[mauritsvanrees]

All is well now. I merged this manually, plus the related plone.formwidget.namedfile/recurrence PRs.