Conditionally quoting download filenames based on detected browser.

NEWS

@@ -1,3 +1,8 @@
+1.2.17: March 18, 2008
+ * Internet Explorer expects quoted download filenames, while Firefox
+   doesn't. So I took that into account by quoting conditionally based on the
+   detected browser.
+
 1.2.16: March 18, 2008
  * Fixed a bug that prevented the upload of filenames with special characters
    in them.

controller/Files.py

@@ -2,11 +2,12 @@ import os
 import re
 import cgi
 import time
+import urllib
 import tempfile
 import cherrypy
 from threading import Lock, Event
 from Expose import expose
-from Validate import validate, Valid_int, Validation_error
+from Validate import validate, Valid_int, Valid_bool, Validation_error
 from Database import Valid_id, end_transaction
 from Users import grab_user_id
 from Expire import strongly_expire
@@ -236,14 +237,19 @@ class Files( object ):
   @grab_user_id
   @validate(
     file_id = Valid_id(),
+    quote_filename = Valid_bool( none_okay = True ),
     user_id = Valid_id( none_okay = True ),
   )
-  def download( self, file_id, user_id = None ):
+  def download( self, file_id, quote_filename = False, user_id = None ):
     """
     Return the contents of file that a user has previously uploaded.
 
     @type file_id: unicode
     @param file_id: id of the file to download
+    @type quote_filename: bool
+    @param quote_filename: True to URL quote the filename of the downloaded file, False to leave it
+                           as UTF-8. IE expects quoting while Firefox doesn't (optional, defaults
+                           to False)
     @type user_id: unicode or NoneType
     @param user_id: id of current logged-in user (if any)
     @rtype: unicode
@@ -265,9 +271,12 @@ class Files( object ):
     db_file = self.__database.load( File, file_id )
 
     cherrypy.response.headerMap[ u"Content-Type" ] = db_file.content_type
-    disposition = u'attachment; filename="%s"' % db_file.filename.replace( '"', r"\"" )
-    disposition = disposition.encode( "utf8" )
-    cherrypy.response.headerMap[ u"Content-Disposition" ] = disposition
+
+    filename = db_file.filename.replace( '"', r"\"" ).encode( "utf8" )
+    if quote_filename:
+      filename = urllib.quote( filename, safe = "" )
+
+    cherrypy.response.headerMap[ u"Content-Disposition" ] = 'attachment; filename="%s"' % filename
     cherrypy.response.headerMap[ u"Content-Length" ] = db_file.size_bytes
 
     def stream():

controller/Validate.py

@@ -125,8 +125,12 @@ class Valid_bool( object ):
   """
   Validator for a boolean value.
   """
+  def __init__( self, none_okay = False ):
+    self.__none_okay = none_okay
+
   def __call__( self, value ):
     value = value.strip()
+    if self.__none_okay and value in ( None, "None", "" ): return None
 
     if value in ( u"True", u"true" ): return True
     if value in ( u"False", u"false" ): return False

controller/test/Test_files.py

@@ -2,6 +2,7 @@
 
 import time
 import types
+import urllib
 import cherrypy
 from threading import Thread
 from StringIO import StringIO
@@ -119,7 +120,7 @@ class Test_files( Test_controller ):
     if self.upload_thread:
       self.upload_thread.join()
 
-  def test_download( self, filename = None ):
+  def test_download( self, filename = None, quote_filename = None ):
     self.login()
 
     self.http_upload(
@@ -134,15 +135,30 @@ class Test_files( Test_controller ):
       session_id = self.session_id,
     )
 
-    result = self.http_get(
-      "/files/download?file_id=%s" % self.file_id,
-      session_id = self.session_id,
-    )
+    if quote_filename is None:
+      result = self.http_get(
+        "/files/download?file_id=%s" % self.file_id,
+        session_id = self.session_id,
+      )
+    elif quote_filename is True:
+      result = self.http_get(
+        "/files/download?file_id=%s&quote_filename=true" % self.file_id,
+        session_id = self.session_id,
+      )
+    else:
+      result = self.http_get(
+        "/files/download?file_id=%s&quote_filename=false" % self.file_id,
+        session_id = self.session_id,
+      )
 
     headers = result[ u"headers" ]
     assert headers
     assert headers[ u"Content-Type" ] == self.content_type
-    assert headers[ u"Content-Disposition" ] == ( u'attachment; filename="%s"' % ( filename or self.filename ) ).encode( "utf8" )
+
+    filename = ( filename or self.filename ).encode( "utf8" )
+    if quote_filename is True:
+      filename = urllib.quote( filename )
+    assert headers[ u"Content-Disposition" ] == 'attachment; filename="%s"' % filename
 
     gen = result[ u"body" ]
     assert isinstance( gen, types.GeneratorType )
@@ -161,6 +177,12 @@ class Test_files( Test_controller ):
   def test_download_with_unicode_filename( self ):
     self.test_download( self.unicode_filename )
 
+  def test_download_with_unicode_quoted_filename( self ):
+    self.test_download( self.unicode_filename, quote_filename = True )
+
+  def test_download_with_unicode_unquoted_filename( self ):
+    self.test_download( self.unicode_filename, quote_filename = False )
+
   def test_download_without_login( self ):
     self.login()
 

static/js/Wiki.js

@@ -2454,9 +2454,14 @@ Upload_pulldown.prototype.upload_started = function ( file_id ) {
 }
 
 Upload_pulldown.prototype.upload_complete = function () {
+  if ( /MSIE/.test( navigator.userAgent ) )
+    var quote_filename = true;
+  else
+    var quote_filename = false;
+
   // now that the upload is done, the file link should point to the uploaded file
   this.uploading = false;
-  this.link.href = "/files/download?file_id=" + this.file_id
+  this.link.href = "/files/download?file_id=" + this.file_id + "&quote_filename=" + quote_filename;
 
   new File_link_pulldown( this.wiki, this.notebook_id, this.invoker, this.editor, this.link );
   this.shutdown();