Commits

Sergey Schetinin  committed 28de330

* instead of detecting if env['wsgi.input'].seek works, assume it does not unless a env.get('webob.is_body_seekable') is True
* make sure req.body_file is always seekable
* raw, possibly unseekable wsgi.input is accessible as req.body_file_raw
* now after calling req.make_body_seekable() the body_file is seeked to 0 and the req.content_length is set

  • Participants
  • Parent commits 2c78e2b

Comments (0)

Files changed (3)

File tests/test_request.py

 
 def test_copy():
     req = Request.blank('/', method='POST', body='some text', request_body_tempfile_limit=1)
-    old_body_file = req.body_file
+    old_body_file = req.body_file_raw
     req.copy_body()
-    assert req.body_file is not old_body_file
+    assert req.body_file_raw is not old_body_file
     req = Request.blank('/', method='POST', body_file=UnseekableInput('0123456789'), content_length=10)
-    assert not hasattr(req.body_file, 'seek')
-    old_body_file = req.body_file
+    assert not hasattr(req.body_file_raw, 'seek')
+    old_body_file = req.body_file_raw
     req.make_body_seekable()
-    assert req.body_file is not old_body_file
+    assert req.body_file_raw is not old_body_file
     assert req.body == '0123456789'
     old_body_file = req.body_file
     req.make_body_seekable()
+    assert req.body_file_raw is old_body_file
     assert req.body_file is old_body_file
 
 class UnseekableInputWithSeek(UnseekableInput):
 def test_broken_seek():
     # copy() should work even when the input has a broken seek method
     req = Request.blank('/', method='POST', body_file=UnseekableInputWithSeek('0123456789'), content_length=10)
-    assert hasattr(req.body_file, 'seek')
-    assert_raises(IOError, req.body_file.seek, [0])
+    assert hasattr(req.body_file_raw, 'seek')
+    assert_raises(IOError, req.body_file_raw.seek, 0)
     old_body_file = req.body_file
     req2 = req.copy()
-    assert req2.body_file is not old_body_file
+    assert req2.body_file_raw is req2.body_file is not old_body_file
     assert req2.body == '0123456789'
 
 def test_set_body():
     req = BaseRequest.blank('/', body='foo')
+    assert req.is_body_seekable
     eq_(req.body, 'foo')
     eq_(req.content_length, 3)
     del req.body
     eq_(req.content_length, 0)
 
 
+
 def test_broken_clen_header():
     # if the UA sends "content_length: ..' header (the name is wrong)
     # it should not break the req.headers.items()

File tests/test_request.txt

     >>> start, end, length
     (0, 100, 1000)
 
-A quick test of caching the request body (now that we check for seek()
-this doesn't exercise it well):
+A quick test of caching the request body:
 
     >>> from cStringIO import StringIO
     >>> length = Request.request_body_tempfile_limit+10
     >>> req = Request.blank('/')
     >>> req.content_length = length
     >>> req.body_file = data
+    >>> req.body_file_raw
+    <...IO... object at ...>
     >>> len(req.body)
     10250
     >>> req.body_file
-    <...IO... object at ...>
-    >>> req.body_file.tell()
+    <open file ..., mode 'w+b' at ...>
+    >>> int(req.body_file.tell())
+    0
+    >>> req.POST
+    UnicodeMultiDict([])
+    >>> int(req.body_file.tell())
     0
 
 Some query tests:
     >>> req.POST['c'] = 'd'
     >>> req.str_POST
     MultiDict([('a', 'b'), ('upload', FieldStorage('upload', 'test.html')), ('c', 'd')])
-    >>> req.body_file
+    >>> req.body_file_raw
     <FakeCGIBody at ... viewing MultiDict([('a'...d')])>
+    >>> sorted(req.str_POST.keys())
+    ['a', 'c', 'upload']
     >>> print req.body.replace('\r', '') # doctestx: +REPORT_UDIFF
     --foobar
     Content-Disposition: form-data; name="a"

File webob/request.py

 
     def _body_file__get(self):
         """
-        Access the body of the request (wsgi.input) as a file-like
+        Access the body of the request (wsgi.input) as a seekable file-like
         object.
 
-        If you set this value, CONTENT_LENGTH will also be updated
-        (either set to -1, 0 if you delete the attribute, or if you
-        set the attribute to a string then the length of the string).
+        If you set this value, CONTENT_LENGTH will also be updated.
         """
-        return self.environ['wsgi.input']
+        self.make_body_seekable()
+        return self.body_file_raw
     def _body_file__set(self, value):
         if isinstance(value, str):
-            self.environ['CONTENT_LENGTH'] = str(len(value))
-            value = StringIO(value)
+            # FIXME: This should issue a warning
+            self.body = value
         else:
-            self.environ.pop('CONTENT_LENGTH', None)
-        self.environ['wsgi.input'] = value
+            self.content_length = None
+            self.body_file_raw = value
+            self.is_body_seekable = False
     def _body_file__del(self):
-        self.environ['wsgi.input'] = StringIO('')
-        self.environ['CONTENT_LENGTH'] = '0'
+        self.body = ''
     body_file = property(_body_file__get, _body_file__set, _body_file__del, doc=_body_file__get.__doc__)
+    body_file_raw = environ_getter('wsgi.input')
 
     scheme = environ_getter('wsgi.url_scheme')
     method = environ_getter('REQUEST_METHOD')
         Return the content of the request body.
         """
         self.make_body_seekable()
-        clen = self.content_length
-        if clen is None:
-            clen = -1
-        f = self.environ['wsgi.input']
-        r = f.read(clen)
-        f.seek(0)
-        return r
-
+        return self.body_file.read(self.content_length)
     def _body__set(self, value):
         if value is None:
-            del self.body
-            return
+            value = ''
         if not isinstance(value, str):
             raise TypeError("You can only set Request.body to a str (not %r)" % type(value))
-        self.environ['wsgi.input'] = StringIO(value)
-        self.environ['CONTENT_LENGTH'] = str(len(value))
-
-
+        self.content_length = len(value)
+        self.body_file_raw = StringIO(value)
+        self.is_body_seekable = True
     def _body__del(self):
-        del self.body_file
-
+        self.body = ''
     body = property(_body__get, _body__set, _body__del, doc=_body__get.__doc__)
 
 
         # default of 0 is better:
         fs_environ.setdefault('CONTENT_LENGTH', '0')
         fs_environ['QUERY_STRING'] = ''
+        self.body_file.seek(0)
         fs = cgi.FieldStorage(fp=self.body_file,
                               environ=fs_environ,
                               keep_blank_values=True)
         env['REQUEST_METHOD'] = 'GET'
         return self.__class__(env)
 
-    @property
-    def is_body_seekable(self):
-        try:
-            self.body_file.seek(0, 1) # relative seek
-            return True
-        except (AttributeError, IOError):
-            return False
+    is_body_seekable = environ_getter('webob.is_body_seekable', False)
 
     def make_body_seekable(self):
         """
-        This forces ``environ['wsgi.input']`` to be seekable.  That
-        is, if it doesn't have a `seek` method already, the content is
-        copied into a StringIO or temporary file.
+        This forces ``environ['wsgi.input']`` to be seekable.
+        That means that, the content is copied into a StringIO or temporary file
+        and flagged as seekable, so that it will not be unnecessarily copied again.
+        After calling this method the .body_file is always seeked to the start of file
+        and .content_length is not None.
 
         The choice to copy to StringIO is made from
         ``self.request_body_tempfile_limit``
         """
-        if not self.is_body_seekable:
+        if self.is_body_seekable:
+            self.body_file_raw.seek(0)
+        else:
             self.copy_body()
 
+
     def copy_body(self):
         """
         Copies the body, in cases where it might be shared with
         if length is not None:
             did_copy = self._copy_body_tempfile()
             if not did_copy:
-                self.body = self.body_file.read(length)
+                self.body = self.body_file_raw.read(length)
         else:
-            self.body = self.body_file.read(-1)
+            self.body = self.body_file_raw.read(-1)
             self._copy_body_tempfile()
 
     def _copy_body_tempfile(self):
         if not tempfile_limit or length <= tempfile_limit:
             return False
         fileobj = self.make_tempfile()
-        input = self.body_file
+        input = self.body_file_raw
         while length:
             data = input.read(min(length, 65536))
             fileobj.write(data)
             length -= len(data)
         fileobj.seek(0)
-        self.environ['wsgi.input'] = fileobj
+        self.body_file_raw = fileobj
+        self.is_body_seekable = True
         return True
 
     def make_tempfile(self):
         """
-            Create a tempfile to store big request body
+            Create a tempfile to store big request body.
+            This API is not stable yet. A 'size' argument might be added.
         """
         return tempfile.TemporaryFile()
 
         'wsgi.multithread': False,
         'wsgi.multiprocess': False,
         'wsgi.run_once': False,
+        #'webob.is_body_seekable': True,
     }
     return env
 
     if not isinstance(data, str):
         data = urllib.urlencode(data)
     env['wsgi.input'] = StringIO(data)
+    env['webob.is_body_seekable'] = True
     env['CONTENT_LENGTH'] = str(len(data))
     env['CONTENT_TYPE'] = 'application/x-www-form-urlencoded'