> So, OP should not be trying to read more than CONTENT_LENGTH. >From the underlying stream, sure. The question is if it's okay to do on the HttpRequest object. It's an issue because now that HttpRequest exposes a file-like interface to the stream some users are likely to do things like this:
request_data = json.load(request) And expect that everything should work fine. > Django should provide a limited stream to prevent user reading more > than CONTENT_LENGTH either by returning an empty end sentinel, > or perhaps if wanted to be pedantic, raise an error. And it does currently provide a limited stream, in some cases, but for (very reasonable) performance reasons, only when it appears neccessary. > Now they just seem to be duplicates > and it's a good idea to ditch LimitBytes (since LimitedStream implements > a readline() too) and move it into some common place. http.utils seems a > good fit for it. Moving LimitedStream to somewhere like http.utils sounds like a good plan to me. > Thoughts? I think HttpRequest definatly needs to ensure that it's safe to treat it as any other file-like object, now that it exposes the .read()/.readlines() methods, and at the moment it sounds like the behaviour of .read(BUFFER_SIZE) past the end of the input is not well defined. 1. It might be reasonable to defer the creation of HttpRequest._stream, but to _always_ wrap it in LimitedStream if it is created. (IE HttpRequest._stream is a property that's only initialised when it's accessed) That'd presumably be a performance gain for any code path that _doesn't_ access .POST/.raw_post_data/.read, and a performance hit for anything that _does_. In 99% of cases you'll be doing a .read() operation without specifying any length at all so the performance hit will be the initial creation of the LimitedStream object, but you won't actually have subsequent function calls that incur the extra layer of wrapping. 2. Ensure that the .read()/.readline() interfaces will always expose a LimitedStream, but avoid creating a limited stream for .POST and .raw_post_data by reading directly from the underlying stream and enforcing the CONTENT_LENGTH behavior explicitly in those cases. 3. Alter the LimitedStream wrapping behaviour to be more cautious. The current behaviour is to wrap it in LimitedStream if it's known to be necessary. It could instead wrap it in LimitedStream _unless_ it's known to be _uneccessary_. Do any of those sound like reasonable options? I guess the other possibility is: 4. Everything is fine as it is, and in real world cases WSGI servers are passing a limited stream on already. It basically sounds like that's the case, but it's not obvious how much we can rely on that. It might be a case that I need to file a seperate bug for this: HttpRequest.read(BUFFER_SIZE) behaviour is unclear. and place a note on the other two that their resolution is dependant on this, does that make sense to y'all? Really appreciate your input on this, Tom -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
