Temporarily disable form memory limit checking for files and images. #1729
Conversation
✅ Deploy Preview for plone-restapi canceled.
|
This comment was marked as resolved.
This comment was marked as resolved.
This fixes a regression due to a low Zope form memory limit of 1MB used since Plone 6.0.7. You can use ``dos_protection`` settings in ``etc/zope.conf`` to change the limit. See plone/Products.CMFPlone#3848 and zopefoundation/Zope#1142.
mauritsvanrees
force-pushed
the
maurits-patch-zope-form-memory-limit
branch
from
721833d
to
796732d
Compare
|
@jenkins-plone-org please run jobs |
…es and images. This seems a better way, then increasing the limit to 16MB. See zopefoundation/Zope#1180 (comment)
|
@jenkins-plone-org please run jobs |
mauritsvanrees
changed the title
There was a problem hiding this comment.
@mauritsvanrees Thanks for continuing to follow up on this.
I'm okay with this as is, but am also making a comment with a suggestion that may avoid the need for a monkeypatch.
| _limit = getattr(ZopeFieldStorage, _attr, None) | ||
| if _limit: | ||
| setattr(ZopeFieldStorage, _attr, None) | ||
| logger.info( |
There was a problem hiding this comment.
| logger.info( | |
| logger.debug( |
I don't feel strongly about this, but it feels like it's probably noise for most people.
There was a problem hiding this comment.
I don't feel strongly about this either, but I think it would be good to have this noticeable, because it may remind us developers that we still need to fix this instead of having this temporary patch.
| @@ -4,6 +4,9 @@ | |||
|
|
|||
|
|
|||
| def json_body(request): | |||
| # TODO We should not read the complete request BODY in memory. | |||
| # Once we have fixed this, we can remove the temporary patches.py. | |||
| # See there for background information. | |||
| try: | |||
| data = json.loads(request.get("BODY") or "{}") | |||
There was a problem hiding this comment.
Based on https://github.com/zopefoundation/Zope/blob/master/src/ZPublisher/HTTPRequest.py#L1061, I think we could do (untested):
body = request.get("BODYFILE")
data = {} if body is None else json.load(body)
This will not actually avoid reading the file into memory (https://pythonspeed.com/articles/json-memory-streaming/ makes it clear that json.load still does that) but would bypass the descriptor that enforces the VALUE_LIMIT.
There was a problem hiding this comment.
I thought BODYFILE would only be defined when an actual file is being uploaded, and not for example when you POST a login and password. But that is not true.
It does not sound like a permanent solution, as this would still offer a way to potentially DOS the server. But maybe very large uploads would still get stopped by one of the other limits. And initial testing seems to work out.
Let me open a different PR, so we still have the current one in case there are problems.
…ing for files and images. plone#1729
This fixes a regression due to a low Zope form memory limit of 1MB used since Plone 6.0.7. You can use
dos_protectionsettings inetc/zope.confto change the limit. See plone/Products.CMFPlone#3848 and zopefoundation/Zope#1142.