Ticket #859 (enhancement)

If static.serve_file() handles a request which has a Ranges header, it will read the entire range of the file into memory. For very large files, this is undesirable behavior.

The following patch uses file_generator() to read the file in reasonable-sized chunks. The patch is against svn trunk.

Also, this patch revealed a debatable design flaw in the current file_generator() function: file_generator() takes an open file object as an argument, but closes the file object before it returns. It would be better if file_generator() left the responsibility for closing the file object to the entity who opened it.

In the interest of not breaking existing code I am unaware of, in this patch I preserved the file-closing behavior of file_generator() if EOF is reached. But I propose removing the input.close() statement from file_generator(). I request comments on this proposal.

diff -r a131852b482a cherrypy/lib/__init__.py
--- a/cherrypy/lib/__init__.py  Thu Sep 18 11:41:21 2008 -0500
+++ b/cherrypy/lib/__init__.py  Thu Sep 18 20:18:50 2008 -0500
@@ -134,13 +134,32 @@ def unrepr(s):
     return _Builder().build(obj)


-def file_generator(input, chunkSize=65536):
-    """Yield the given input (a file object) in chunks (default 64k). (Core)"""
+def file_generator(input, length=None, chunkSize=65536):
+    """Yield the given input (a file object) in chunks (default 64k). (Core)
+    If length is specified, it must be a positive integer, and
+    the generator will stop after emitting that many bytes.
+    """
+    if length is not None:
+        if not isinstance(length, (int, long)):
+            raise TypeError("length must be an integer or long")
+        if length <= 0:
+            raise ValueError("length must be a positive integer or long")
+
+    count = 0
     chunk = input.read(chunkSize)
     while chunk:
-        yield chunk
+        if length is None:
+            yield chunk
+        else:
+            if count + len(chunk) <= length:
+                yield chunk
+                count += len(chunk)
+            else:
+                if length != count:
+                    yield chunk[:length-count]
+                break
         chunk = input.read(chunkSize)
-    input.close()

+    if not chunk:
+        input.close()

-
diff -r a131852b482a cherrypy/lib/static.py
--- a/cherrypy/lib/static.py    Thu Sep 18 11:41:21 2008 -0500
+++ b/cherrypy/lib/static.py    Thu Sep 18 18:59:04 2008 -0500
@@ -10,7 +10,7 @@ import urllib
 import urllib

 import cherrypy
-from cherrypy.lib import cptools, http
+from cherrypy.lib import cptools, http, file_generator


 def serve_file(path, content_type=None, disposition=None, name=None):
@@ -83,13 +83,15 @@ def serve_file(path, content_type=None,
             if len(r) == 1:
                 # Return a single-part response.
                 start, stop = r[0]
+                if stop > c_len:
+                    stop = c_len
                 r_len = stop - start
                 response.status = "206 Partial Content"
                 response.headers['Content-Range'] = ("bytes %s-%s/%s" %
                                                        (start, stop - 1, c_len))
                 response.headers['Content-Length'] = r_len
                 bodyfile.seek(start)
-                response.body = bodyfile.read(r_len)
+                response.body = file_generator(bodyfile, length=r_len)
             else:
                 # Return a multipart/byteranges response.
                 response.status = "206 Partial Content"
@@ -111,7 +113,8 @@ def serve_file(path, content_type=None,
                         yield ("\r\nContent-range: bytes %s-%s/%s\r\n\r\n"
                                % (start, stop - 1, c_len))
                         bodyfile.seek(start)
-                        yield bodyfile.read(stop - start)
+                        for chunk in file_generator(bodyfile, length=stop-start):
+                            yield chunk
                         yield "\r\n"
                     # Final boundary
                     yield "--" + boundary + "--"