From 2cae1faf2dac3717055adaa0347f71e4d61c3ee7 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 18 Mar 2008 22:22:19 +0000 Subject: [PATCH] Conditionally quoting download filenames based on detected browser. --- NEWS | 5 +++++ controller/Files.py | 19 ++++++++++++++----- controller/Validate.py | 4 ++++ controller/test/Test_files.py | 34 ++++++++++++++++++++++++++++------ static/js/Wiki.js | 7 ++++++- 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/NEWS b/NEWS index febc513..c26446b 100644 --- a/NEWS +++ b/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. diff --git a/controller/Files.py b/controller/Files.py index 41bb1f1..fe95bf8 100644 --- a/controller/Files.py +++ b/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(): diff --git a/controller/Validate.py b/controller/Validate.py index 7e8681e..f6a4794 100644 --- a/controller/Validate.py +++ b/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 diff --git a/controller/test/Test_files.py b/controller/test/Test_files.py index 2110ec6..e3a9d39 100644 --- a/controller/test/Test_files.py +++ b/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"e_filename=true" % self.file_id, + session_id = self.session_id, + ) + else: + result = self.http_get( + "/files/download?file_id=%s"e_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() diff --git a/static/js/Wiki.js b/static/js/Wiki.js index 7bea9a4..18d04e0 100644 --- a/static/js/Wiki.js +++ b/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 + ""e_filename=" + quote_filename; new File_link_pulldown( this.wiki, this.notebook_id, this.invoker, this.editor, this.link ); this.shutdown();