extractCredentials: do not fail when the request is too large to read.

[mauritsvanrees]

This is a problem since at least Zope 5.8.4, introduced in Plone 6.0.7. See https://github.com/plone/Products.CMFPlone/issues/3848 and https://github.com/zopefoundation/Zope/pull/1180.

This PR makes sure that a too big request body does not throw an error when extracting credentials. This makes large image upload work again in ClassicUI with plone.restapi installed.

Uploading a file or image larger than 1MB using plone.restapi still likely fails, unless you have increased the form-memory-limit in zope.conf. Fixing this may need a change along these lines, but that would be a more far reaching change that needs careful consideration.

[mister-roboto]

@mauritsvanrees thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

[mauritsvanrees]

@jenkins-plone-org please run jobs

[mauritsvanrees]

As alternative, or as extra, we could do this before calling creds = deserializer.json_body(request):

if request.get_header("Content-Type") != "application/json":
    return

This was suggested by @davisagli in https://github.com/plone/Products.CMFPlone/issues/3848#issuecomment-1769272646

In my case in ClassicUI during an image upload, the content type is something like multipart/form-data; boundary=.... When just browsing the site, the content type is None. But then, when I visit http://localhost:8080/Plone/++api++/ in the browser (so not called by javascript), the Content-Type request header is also None. So that check might not be enough.

[netlify[bot]]

✅ Deploy Preview for plone-restapi canceled.

Name	Link
🔨 Latest commit	3a3770bd9ac0045ca07e3afbe484b8c586af8b8a
🔍 Latest deploy log	https://app.netlify.com/sites/plone-restapi/deploys/6540e2249f1fa2000840ff29

[davisagli]

@mauritsvanrees In both of those cases, the request does not have a JSON body, and we don't want to waste effort in trying to parse it as JSON. The only problem would be if there is a client which is sending a JSON body but not sending the Content-Type: application/json header. That is a buggy client not following the documented API.

It should be a conditional around the code that tries to get login & password from the JSON body though, not a return out of the function. We still need to run the code after that looks for the token in the Authorization header, regardless of request content type.

[davisagli]

Hmm, the whole chunk of code looking for login and password in the request body seems misplaced. Shouldn't that be done only in the login service?

[mauritsvanrees]

Hmm, the whole chunk of code looking for login and password in the request body seems misplaced. Shouldn't that be done only in the login service?

I think you are right. Theoretically this chunk gets a dict with login and password, but would then be passed on to authenticateCredentials a few lines down, and this only checks for a token. So extracting login and password indeed seems useless.

[mauritsvanrees]

Do note this remark in extractCredentials:

Prefer any credentials in a JSON POST request under the assumption that any such requested sent when a JWT token is already in the Authorization header is intended to change or update the logged in user.

But as said, that should not have any effect.

I will keep this PR, as it should be okay, but then create another one where I remove these lines.

[mauritsvanrees]

Closed in favour of PR #1728.