-
Notifications
You must be signed in to change notification settings - Fork 101
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.
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
D.O.S. Protection Limits #1177
Comments
Georg Gogo. BERNHARD wrote at 2023-10-25 03:05 -0700:
...
If "disk limit" is the request body size, why is it called "disk" limit?
The names come originally from `multipart`.
`multipart` maintains parts in memory when they are sufficiently small
(controlled by `form_memory_limit`) and stores them on disk otherwise.
The `form-disk-limit` is the maximal amount of disk space usable
for a form.
Do I understand it right that "memfile limit" is a "switch" so that smaller data are stored in memory whereas data larger than 4k are being written to disk?
Right.
Additionally I wanted to report that our employees were unable to upload image files with the very restrictive default values after an update; could we consider to raise the form memory limit to say 10MB or more? Will the "memfile" limit of 4k lead to problems with uploads?
The `form-memory-limit` limits access when a request variable of type
`file` or the request body is not accessed via the "file API" but
as `bytes`.
E.g. `request["BODYFILE"]` accesses the request body as a file
(`form-memory-limit` has no effect)
while `request["BODY"]` accesses it as `bytes` (`form-memory-limit` applies).
Thus, `form-memory-limit` is used for 2 purposes:
1. control when a part is moved to disk
2. limit the part size when accessed as `bytes`
If you are (or are ready to become) a Zope contributer,
you could make a "pull request" to provide separate
configuration options for those purposes.
I would be in favor thereof.
|
Thank you so much for shedding some light on this! We ran into this problem when we came across this traceback:
I found that in ZPublisher.HTTPRequest the value from the configuration is used Zope/src/ZPublisher/HTTPRequest.py Line 1427 in 8fdd567
Zope/src/ZPublisher/HTTPRequest.py Line 1370 in 8fdd567
Zope/src/ZPublisher/HTTPRequest.py Line 1058 in 8fdd567
Plone just wanted to read the value like this: Now I wonder how I could make this more convenient. Changing the default values would of course help, but the actual problem is the Any thoughts on this? |
Georg Gogo. BERNHARD wrote at 2023-10-30 02:46 -0700:
...
Plone just wanted to read the value like this: `data = json.loads(request.get("BODY") or "{}")` <https://github.com/plone/plone.restapi/blob/5f9214950a50d4728d7a245bec1358a68c8f2316/src/plone/restapi/deserializer/__init__.py#L8>, not knowing that there are two ways request data are being handled.
Now I wonder how I could make this more convenient. Changing the default values would of course help, but the actual problem is the `request.get` method that should return a value no matter how it was being stored.
Any thoughts on this?
`Zope` should not use `FORM_MEMORY_LIMIT` to limit how much
data can be returned from `request["BODY"]`. It should use a separate
configuration parameter for this (which could be larger than
`FORM_MEMORY_LIMIT` as it affects only a single value, not potentially
a whole set).
Would you like to contribute this change?
Of course, `plone.restapi` could use
`data = {} if "BODYFILE" not in request else json.load(request["BODYFILE"])`
instead of
`data = json.loads(request.get("BODY") or "{}")`.
This assumes that `BODYFILE` is not an empty file, if it exists.
A more conservative solutions would be
```python
body = request.get("BODYFILE")
if body is not None:
body = body.read()
data = json.loads(body or "{}")
```
I prefer the `Zope` change.
Should you fell uncomfortable with the change contribution,
I will do it.
|
@gogobd I must revert this statement: having reread the documentation in I no longer think that I will create a PR which improves/corrects the DOS protection related |
Hello, every 01!
I just came across
In
ZPublisher/HTTPRequest.py
and I have a question: how do these values relate, "memory limit" is responsible for limiting the request "stream", but I don't understand the documentation of "disk limit" and "memfile limit".If "disk limit" is the request body size, why is it called "disk" limit?
Do I understand it right that "memfile limit" is a "switch" so that smaller data are stored in memory whereas data larger than 4k are being written to disk?
Additionally I wanted to report that our employees were unable to upload image files with the very restrictive default values after an update; could we consider to raise the form memory limit to say 10MB or more? Will the "memfile" limit of 4k lead to problems with uploads?
Thank you for clarification!
The text was updated successfully, but these errors were encountered: