Skip to content
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

Remove uneccesary request_body_tempfile_limit override from webob #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kofalt
Copy link

@kofalt kofalt commented Jul 8, 2015

The request_body_tempfile_limit key is only used in a single place in webapp2:

# grep -r request_body_tempfile_limit
webob/request.py:    request_body_tempfile_limit = 10*1024
webob/request.py:        ``self.request_body_tempfile_limit``
webob/request.py:        tempfile_limit = self.request_body_tempfile_limit
webapp2.py:    request_body_tempfile_limit = 0

Specifically, in webob's _copy_body_tempfile, which cancels the use of a tempfile if the content is too small:

if not tempfile_limit or todo <= tempfile_limit:
    return False

Setting this key to zero has the unintended effect of never using a tempfile, which will cause webob's copy_body function to always read a body straight into memory.

did_copy = self._copy_body_tempfile()
if not did_copy:
    # it wasn't necessary, so just read it into memory
    self.body = self.body_file.read(self.content_length)

This is clearly wrong, as the body is of arbitrary size and will crash if the request is larger than your available memory. Presumably, the intended effect was to have a body of any size be written to a tempfile, but webob already has a sensible default:

## The limit after which request bodies should be stored on disk
## if they are read in (under this, and the request body is stored
## in memory):
request_body_tempfile_limit = 10*1024

When chasing this back, it looks like this override was simply a mistake during import:

# hg blame webapp2.py  | grep -C 5 request_body_tempfile_limit
...
255:     # Attributes from webapp.
255:     request_body_tempfile_limit = 0
...

# hg log | grep -C 5 '255:'
...
changeset:   255:a24a0f6231af
user:        Rodrigo Moraes <[email protected]>
date:        Thu Jun 16 16:00:29 2011 -0300
summary:     Copied Request from webapp, so that the whole API is available without SDK.
...

In summary, it is my belief that overriding this value causes the opposite of its intended effect and further is not necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant