From 04f86f05a675681cdc19a7aaf4635b0afb62f247 Mon Sep 17 00:00:00 2001
From: Dan Helfman <dan@luminotes.com>
Date: Mon, 4 Feb 2008 20:06:02 +0000
Subject: [PATCH] Rewrote controller.Files.upload_file() not to use a
 CherryPy-2.1-style file upload filter. Now releasing session lock before
 streaming the file to prevent session deadlock in the event of a cancelled
 upload.

---
 controller/Expose.py |  3 ++-
 controller/Files.py  | 39 ++++++++++++++++-----------------------
 static/js/Wiki.js    |  2 +-
 view/Upload_page.py  |  2 +-
 4 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/controller/Expose.py b/controller/Expose.py
index 5d02a75..c25db6c 100644
--- a/controller/Expose.py
+++ b/controller/Expose.py
@@ -1,4 +1,5 @@
 import cherrypy
+import types
 
 # module-level variable that, when set to a view, overrides the view for all exposed methods. used
 # by unit tests
@@ -64,7 +65,7 @@ def expose( view = None, rss = None ):
           result = dict( error = u"An error occurred when processing your request. Please try again or contact support." )
 
       # if the result is a generator, it's streaming data, so just let CherryPy handle it
-      if hasattr( result, "gi_running" ):
+      if isinstance( result, types.GeneratorType ):
         return result
 
       redirect = result.get( u"redirect", None )
diff --git a/controller/Files.py b/controller/Files.py
index 440843a..275574e 100644
--- a/controller/Files.py
+++ b/controller/Files.py
@@ -1,6 +1,5 @@
 import cgi
 import cherrypy
-from cherrypy.filters import basefilter
 from Expose import expose
 from Validate import validate
 from Database import Valid_id
@@ -37,21 +36,7 @@ class Upload_error( Exception ):
     )
 
 
-class File_upload_filter( basefilter.BaseFilter ):
-  def before_request_body( self ):
-    if cherrypy.request.path != "/files/upload_file":
-      return
-
-    if cherrypy.request.method != "POST":
-      raise Upload_error()
-
-    # tell CherryPy not to parse the POST data itself for this URL
-    cherrypy.request.processRequestBody = False
-
-
 class Files( object ):
-  _cpFilterList = [ File_upload_filter() ]
-
   """
   Controller for dealing with uploaded files, corresponding to the "/files" URL.
   """
@@ -94,20 +79,31 @@ class Files( object ):
   @strongly_expire
   @grab_user_id
   @validate(
+    upload = (),
+    notebook_id = Valid_id(),
+    note_id = Valid_id(),
     user_id = Valid_id( none_okay = True ),
   )
-  def upload_file( self, user_id ):
+  def upload_file( self, upload, notebook_id, note_id, user_id ):
     """
     Upload a file from the client for attachment to a particular note.
 
+    @type upload: cgi.FieldStorage
+    @param upload: file handle to uploaded file
     @type notebook_id: unicode
     @param notebook_id: id of the notebook that the upload is to
     @type note_id: unicode
     @param note_id: id of the note that the upload is to
     @raise Access_error: the current user doesn't have access to the given notebook or note
+    @raise Upload_error: an error occurred when processing the uploaded file
+    @type user_id: unicode or NoneType
+    @param user_id: id of current logged-in user (if any)
     @rtype: unicode
     @return: rendered HTML page
     """
+    if not self.__users.check_access( user_id, notebook_id ):
+      raise Access_error()
+
     cherrypy.server.max_request_body_size = 0 # remove file size limit of 100 MB
     cherrypy.response.timeout = 3600    # increase upload timeout to one hour (default is 5 min)
     cherrypy.server.socket_timeout = 60 # increase socket timeout to one minute (default is 10 sec)
@@ -125,15 +121,8 @@ class Files( object ):
     if file_size <= 0:
       raise Upload_error()
 
-    parsed_form = cgi.FieldStorage( fp = cherrypy.request.rfile, headers = headers, environ = { "REQUEST_METHOD": "POST" }, keep_blank_values = 1)
-    upload = parsed_form[ u"file" ]
-    notebook_id = parsed_form.getvalue( u"notebook_id" )
-    note_id = parsed_form.getvalue( u"note_id" )
     filename = upload.filename.strip()
 
-    if not self.__users.check_access( user_id, notebook_id ):
-      raise Access_error()
-
     def process_upload():
       """
       Process the file upload while streaming a progress meter as it uploads.
@@ -222,4 +211,8 @@ class Files( object ):
       upload.file.close()
       cherrypy.request.rfile.close()
 
+    # release the session lock before beginning the upload, because if the upload is cancelled
+    # before it's done, the lock won't be released
+    cherrypy.session.release_lock()
+
     return process_upload()
diff --git a/static/js/Wiki.js b/static/js/Wiki.js
index a491741..b374003 100644
--- a/static/js/Wiki.js
+++ b/static/js/Wiki.js
@@ -2258,7 +2258,7 @@ Upload_pulldown.prototype.init_frame = function () {
   withDocument( doc, function () {
     connect( "upload_button", "onclick", function ( event ) {
       withDocument( doc, function () {
-        self.upload_started( getElement( "file" ).value );
+        self.upload_started( getElement( "upload" ).value );
       } );
     } );
   } );
diff --git a/view/Upload_page.py b/view/Upload_page.py
index 0d6cdad..8f9f393 100644
--- a/view/Upload_page.py
+++ b/view/Upload_page.py
@@ -12,7 +12,7 @@ class Upload_page( Html ):
       Body(
         Form(
           Span( u"attach file: ", class_ = u"field_label" ),
-          Input( type = u"file", id = u"file", name = u"file", class_ = "text_field", size = u"30" ),
+          Input( type = u"file", id = u"upload", name = u"upload", class_ = "text_field", size = u"30" ),
           Input( type = u"submit", id = u"upload_button", class_ = u"button", value = u"upload" ),
           Input( type = u"hidden", id = u"notebook_id", name = u"notebook_id", value = notebook_id ),
           Input( type = u"hidden", id = u"note_id", name = u"note_id", value = note_id ),