changeset 3568:6fe1ea4d9d1a

use the open temporary file for file uploads (fixes big memory consumption for big file uploads) - Twisted and mod_python is completely untested
author Thomas Waldmann <tw AT waldmann-edv DOT de>
date Thu, 08 May 2008 10:10:12 +0200
parents ddf31f2ae8e3
children 7593b6b4590c
files MoinMoin/action/AttachFile.py MoinMoin/action/Load.py MoinMoin/request/__init__.py MoinMoin/request/request_modpython.py MoinMoin/server/server_twisted.py
diffstat 5 files changed, 41 insertions(+), 26 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/action/AttachFile.py	Tue May 06 22:52:39 2008 +0200
+++ b/MoinMoin/action/AttachFile.py	Thu May 08 10:10:12 2008 +0200
@@ -168,7 +168,19 @@
     return "\n<p>\n%s\n</p>\n" % attach_info
 
 
+def _write_stream(content, stream, bufsize=8192):
+    if isinstance(content, str):
+        stream.write(content)
+    elif isinstance(content, file):
+        import shutil
+        shutil.copyfileobj(content, stream, bufsize)
+
 def add_attachment(request, pagename, target, filecontent, overwrite=0):
+    """ save <filecontent> to an attachment <target> of page <pagename>
+
+        filecontent can be either a str (in memory file content),
+        or an open file object (file content in e.g. a tempfile).
+    """
     _ = request.getText
 
     # replace illegal chars
@@ -180,7 +192,8 @@
     fpath = os.path.join(attach_dir, target).encode(config.charset)
     exists = os.path.exists(fpath)
     if exists and not overwrite:
-        msg = _("Attachment '%(target)s' already exists.") % {'target': target, }
+        filesize = 0
+        msg = _("Attachment '%(target)s' already exists.") % {'target': target, } # XXX unused?
     else:
         if exists:
             try:
@@ -189,16 +202,17 @@
                 pass
         stream = open(fpath, 'wb')
         try:
-            stream.write(filecontent)
+            _write_stream(filecontent, stream)
         finally:
             stream.close()
 
         _addLogEntry(request, 'ATTNEW', pagename, target)
 
-        event = FileAttachedEvent(request, pagename, target, len(filecontent))
+        filesize = os.path.getsize(fpath)
+        event = FileAttachedEvent(request, pagename, target, filesize)
         send_event(event)
 
-        return target
+    return target, filesize
 
 
 #############################################################################
@@ -583,9 +597,8 @@
 
     # add the attachment
     try:
-        add_attachment(request, pagename, target, filecontent, overwrite=overwrite)
+        target, bytes = add_attachment(request, pagename, target, filecontent, overwrite=overwrite)
 
-        bytes = len(filecontent)
         msg = _("Attachment '%(target)s' (remote name '%(filename)s')"
                 " with %(bytes)d bytes saved.") % {
                 'target': target, 'filename': filename, 'bytes': bytes}
@@ -627,7 +640,7 @@
     else:
         stream = open(savepath, 'wb')
         try:
-            stream.write(filecontent)
+            _write_stream(filecontent, stream)
         finally:
             stream.close()
 
--- a/MoinMoin/action/Load.py	Tue May 06 22:52:39 2008 +0200
+++ b/MoinMoin/action/Load.py	Thu May 08 10:10:12 2008 +0200
@@ -40,7 +40,6 @@
             filename = form['file__filename__']
 
         filecontent = form['file'][0]
-        bytes = len(filecontent)
 
         overwrite = False
         if 'overwrite' in form:
@@ -76,14 +75,16 @@
                     msg = _("You are not allowed to delete attachments on this page.")
                     return status, msg
 
-            AttachFile.add_attachment(self.request, self.pagename, target, filecontent)
-            bytes = len(filecontent)
+            target, bytes = AttachFile.add_attachment(self.request, self.pagename, target, filecontent)
             msg = _("Attachment '%(target)s' (remote name '%(filename)s') with %(bytes)d bytes saved.") % {
                    'target': target, 'filename': filename, 'bytes': bytes}
             status = True
 
         else:
-            filecontent = unicode(filecontent, config.charset)
+            if isinstance(filecontent, file):
+                filecontent = filecontent.read() # XXX reads complete file into memory!
+            if isinstance(filecontent, str):
+                filecontent = unicode(filecontent, config.charset)
             self.pagename = target
             page = Page(self.request, self.pagename)
             pagedir = page.getPagePath("", check_create=0)
--- a/MoinMoin/request/__init__.py	Tue May 06 22:52:39 2008 +0200
+++ b/MoinMoin/request/__init__.py	Thu May 08 10:10:12 2008 +0200
@@ -1073,10 +1073,12 @@
                 values = [values]
             fixedResult = []
             for item in values:
-                fixedResult.append(item.value)
                 if isinstance(item, cgi.FieldStorage) and item.filename:
+                    fixedResult.append(item.file) # open data tempfile
                     # Save upload file name in a separate key
                     args[key + '__filename__'] = item.filename
+                else:
+                    fixedResult.append(item.value)
             args[key] = fixedResult
 
         return self.decodeArgs(args)
--- a/MoinMoin/request/request_modpython.py	Tue May 06 22:52:39 2008 +0200
+++ b/MoinMoin/request/request_modpython.py	Thu May 08 10:10:12 2008 +0200
@@ -93,16 +93,16 @@
             if not isinstance(values, list):
                 values = [values]
             fixedResult = []
-
             for item in values:
-                # Remember filenames with a name hack
-                if hasattr(item, 'filename') and item.filename:
+                if isinstance(item, util.StringField):
+                    fixedResult.append(item.value)
+                elif isinstance(item, util.Field) and item.filename:
+                    fixedResult.append(item.file)
+                    # Remember filenames with a name hack
                     args[key + '__filename__'] = item.filename
-                # mod_python 2.7 might return strings instead of Field
-                # objects.
-                if hasattr(item, 'value'):
-                    item = item.value
-                fixedResult.append(item)
+                elif isinstance(item, str):
+                    # mod_python 2.7 might return strings instead of Field objects.
+                    fixedResult.append(item)
             args[key] = fixedResult
 
         return self.decodeArgs(args)
--- a/MoinMoin/server/server_twisted.py	Tue May 06 22:52:39 2008 +0200
+++ b/MoinMoin/server/server_twisted.py	Thu May 08 10:10:12 2008 +0200
@@ -160,13 +160,12 @@
                     values = [values]
                 fixedResult = []
                 for i in values:
-                    if isinstance(i, cgi.MiniFieldStorage):
+                    if isinstance(i, cgi.FieldStorage) and i.filename:
+                        fixedResult.append(i.file)
+                        # multiple uploads to same form field are stupid!
+                        args[key + '__filename__'] = i.filename
+                    else:
                         fixedResult.append(i.value)
-                    elif isinstance(i, cgi.FieldStorage):
-                        fixedResult.append(i.value)
-                        # multiple uploads to same form field are stupid!
-                        if i.filename:
-                            args[key + '__filename__'] = i.filename
                 args[key] = fixedResult
 
         self.process()