After discussion on the PR, we concluded that the reasons that the request.body check was removed weren't valid, so this check is reinstated. I believe I've addressed all Tom Christie's concerns by now. If anyone else would like to take a look, now is the time. Thanks!
https://github.com/django/django/pull/6447 On Tuesday, April 26, 2016 at 6:47:09 PM UTC-4, Tim Graham wrote: > > It seems there was request.body checking in previous iterations of the PR > but it was dropped for reasons that aren't entirely clear to me: > https://github.com/django/django/pull/3852#discussion_r35350372 > > On Wednesday, April 20, 2016 at 9:30:48 PM UTC-4, Cristiano Coelho wrote: >> >> Hi, >> >> In particular I'm interested in this new setting: >> DATA_UPLOAD_MAX_MEMORY_SIZE >> [1] >> that only seems to be checked against mutlparts [2] and url encoded[3] >> request bodies. >> >> It could be good that this setting is also checked against other types >> where request.body is read directly, as you can still get the >> content-length from the body right? Please correct me if I'm wrong, but >> when in already django code all body data is always loaded in memory except >> for files and multi-part uploads which are streamed. >> So JSON, XML or even plain text post requests could benefit from the >> DATA_UPLOAD_MAX_MEMORY_SIZE setting and it could be very convenient for >> example if an attacker sends a huge json, the python (at least 2.7) >> json.loads call usually crashes with an out of memory error when the string >> is too big while still creating a huge RAM spike. >> >> >> [1] >> https://github.com/django/django/pull/6447/files#diff-ba8335f5987fcd81d41c28cd1879a9bfR291 >> [2] >> https://github.com/django/django/pull/6447/files#diff-ba8335f5987fcd81d41c28cd1879a9bfR291 >> [3] >> https://github.com/django/django/pull/6447/files#diff-0eb6c5000a61126731553169fddb306eR294 >> >> >> El martes, 19 de abril de 2016, 13:06:27 (UTC-3), Tom Christie escribió: >>> >>> > If you are using django-rest-framework, how would the fields counter >>> work?. It would be a shame if only multi part and urlencoded uploads would >>> have the benefit of these checks, while still allowing json, xml and others >>> still be "exploited". >>> Note I didn't really read the code changes completely so I'm talking >>> with almost no knowledge on the proposed change. >>> >>> They wouldn't be respected by anything other than multi-part or >>> urlencoded requests. >>> Tim's correct in noting that accessing `request.body` or >>> `request.stream` won't apply these checks (which is for example, what REST >>> framework does). >>> >>> Even so I think this is probably a reasonable approach. We could add >>> support for respecting these settings in REST framework too, once they >>> exist.(Although I think we'd have need to have a stricter consideration of >>> backwards compat wrt. folks POSTing large amounts of JSON data) >>> >>> -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/c8f7d421-6c33-4803-a9d8-159dd9ce4d92%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
