From eaf33b5d4090c62f14fad7fffd32f76d91cc23d9 Mon Sep 17 00:00:00 2001 From: Carl Eberhard Date: Fri, 10 Jan 2014 12:55:43 -0500 Subject: [PATCH] History & HDA API: fix unicode support in update --- lib/galaxy/web/base/controller.py | 23 ++++ lib/galaxy/webapps/galaxy/api/histories.py | 30 +---- .../webapps/galaxy/api/history_contents.py | 39 ++----- test/casperjs/api-hda-tests.js | 110 ++++++------------ test/casperjs/api-history-tests.js | 81 ++++--------- 5 files changed, 97 insertions(+), 186 deletions(-) diff --git a/lib/galaxy/web/base/controller.py b/lib/galaxy/web/base/controller.py index 0ad6395c688..3f4e7b45550 100644 --- a/lib/galaxy/web/base/controller.py +++ b/lib/galaxy/web/base/controller.py @@ -179,6 +179,29 @@ class BaseController( object ): rval[k] = self.encode_all_ids(trans, v, recursive) return rval + # incoming param validation + # should probably be in sep. serializer class/object _used_ by controller + def validate_and_sanitize_basestring( self, key, val ): + if not isinstance( val, basestring ): + raise ValueError( '%s must be a string or unicode: %s' %( key, str( type( val ) ) ) ) + return unicode( sanitize_html( val, 'utf-8', 'text/html' ), 'utf-8' ) + + def validate_and_sanitize_basestring_list( self, key, val ): + if not isinstance( val, list ): + raise ValueError( '%s must be a list: %s' %( key, str( type( val ) ) ) ) + return [ unicode( sanitize_html( t, 'utf-8', 'text/html' ), 'utf-8' ) for t in val ] + + def validate_boolean( self, key, val ): + if not isinstance( val, bool ): + raise ValueError( '%s must be a boolean: %s' %( key, str( type( val ) ) ) ) + return val + + #TODO: + #def validate_integer( self, key, val, min, max ): + #def validate_float( self, key, val, min, max ): + #def validate_number( self, key, val, min, max ): + #def validate_genome_build( self, key, val ): + Root = BaseController diff --git a/lib/galaxy/webapps/galaxy/api/histories.py b/lib/galaxy/webapps/galaxy/api/histories.py index a6000894323..6f58b702f62 100644 --- a/lib/galaxy/webapps/galaxy/api/histories.py +++ b/lib/galaxy/webapps/galaxy/api/histories.py @@ -342,34 +342,14 @@ class HistoriesController( BaseAPIController, UsesHistoryMixin, UsesTagsMixin ): 'id', 'model_class', 'nice_size', 'contents_url', 'purged', 'tags', 'state', 'state_details', 'state_ids' ) - validated_payload = {} for key, val in payload.items(): - # TODO: lots of boilerplate here, but overhead on abstraction is equally onerous - if key == 'name': - if not ( isinstance( val, str ) or isinstance( val, unicode ) ): - raise ValueError( 'name must be a string or unicode: %s' %( str( type( val ) ) ) ) - validated_payload[ 'name' ] = sanitize_html( val, 'utf-8' ) - #TODO:?? if sanitized != val: log.warn( 'script kiddie' ) - elif key == 'deleted': - if not isinstance( val, bool ): - raise ValueError( 'deleted must be a boolean: %s' %( str( type( val ) ) ) ) - validated_payload[ 'deleted' ] = val - elif key == 'published': - if not isinstance( val, bool ): - raise ValueError( 'published must be a boolean: %s' %( str( type( val ) ) ) ) - validated_payload[ 'published' ] = val - elif key == 'genome_build' and val is not None: - if not ( isinstance( val, str ) or isinstance( val, unicode ) ): - raise ValueError( 'genome_build must be a string: %s' %( str( type( val ) ) ) ) - validated_payload[ 'genome_build' ] = sanitize_html( val, 'utf-8' ) - elif key == 'annotation': - if not ( isinstance( val, str ) or isinstance( val, unicode ) ): - raise ValueError( 'annotation must be a string or unicode: %s' %( str( type( val ) ) ) ) - validated_payload[ 'annotation' ] = sanitize_html( val, 'utf-8' ) + if key in ( 'name', 'genome_build', 'annotation' ): + validated_payload[ key ] = self.validate_and_sanitize_basestring( key, val ) + if key in ( 'deleted', 'published' ): + validated_payload[ key ] = self.validate_boolean( key, val ) elif key == 'tags': - if isinstance( val, list ): - validated_payload[ 'tags' ] = [ sanitize_html( t, 'utf-8' ) for t in val ] + validated_payload[ key ] = self.validate_and_sanitize_basestring_list( key, val ) elif key not in valid_but_uneditable_keys: pass #log.warn( 'unknown key: %s', str( key ) ) diff --git a/lib/galaxy/webapps/galaxy/api/history_contents.py b/lib/galaxy/webapps/galaxy/api/history_contents.py index e77f05d9841..afeb6e67d08 100644 --- a/lib/galaxy/webapps/galaxy/api/history_contents.py +++ b/lib/galaxy/webapps/galaxy/api/history_contents.py @@ -385,38 +385,17 @@ class HistoryContentsController( BaseAPIController, UsesHistoryDatasetAssociatio ) validated_payload = {} for key, val in payload.items(): - # TODO: lots of boilerplate here, but overhead on abstraction is equally onerous - # typecheck, parse, remap key - if key == 'name': - if not ( isinstance( val, str ) or isinstance( val, unicode ) ): - raise ValueError( 'name must be a string or unicode: %s' %( str( type( val ) ) ) ) - validated_payload[ 'name' ] = util.sanitize_html.sanitize_html( val, 'utf-8' ) - #TODO:?? if sanitized != val: log.warn( 'script kiddie' ) - elif key == 'deleted': - if not isinstance( val, bool ): - raise ValueError( 'deleted must be a boolean: %s' %( str( type( val ) ) ) ) - validated_payload[ 'deleted' ] = val - elif key == 'visible': - if not isinstance( val, bool ): - raise ValueError( 'visible must be a boolean: %s' %( str( type( val ) ) ) ) - validated_payload[ 'visible' ] = val - elif key == 'genome_build': - if not ( isinstance( val, str ) or isinstance( val, unicode ) ): - raise ValueError( 'genome_build must be a string: %s' %( str( type( val ) ) ) ) - validated_payload[ 'dbkey' ] = util.sanitize_html.sanitize_html( val, 'utf-8' ) - elif key == 'annotation' and val is not None: - if not ( isinstance( val, str ) or isinstance( val, unicode ) ): - raise ValueError( 'annotation must be a string or unicode: %s' %( str( type( val ) ) ) ) - validated_payload[ 'annotation' ] = util.sanitize_html.sanitize_html( val, 'utf-8' ) - elif key == 'misc_info': - if not ( isinstance( val, str ) or isinstance( val, unicode ) ): - raise ValueError( 'misc_info must be a string or unicode: %s' %( str( type( val ) ) ) ) - validated_payload[ 'info' ] = util.sanitize_html.sanitize_html( val, 'utf-8' ) + if key in ( 'name', 'genome_build', 'misc_info', 'annotation' ): + val = self.validate_and_sanitize_basestring( key, val ) + #TODO: need better remap system or eliminate the need altogether + key = 'dbkey' if key == 'genome_build' else key + key = 'info' if key == 'misc_info' else key + validated_payload[ key ] = val + if key in ( 'deleted', 'visible' ): + validated_payload[ key ] = self.validate_boolean( key, val ) elif key == 'tags': - if isinstance( val, list ): - validated_payload[ 'tags' ] = [ sanitize_html( t, 'utf-8' ) for t in val ] + validated_payload[ key ] = self.validate_and_sanitize_basestring_list( key, val ) elif key not in valid_but_uneditable_keys: pass #log.warn( 'unknown key: %s', str( key ) ) return validated_payload - diff --git a/test/casperjs/api-hda-tests.js b/test/casperjs/api-hda-tests.js index a61cd9e3f81..5a24a73042f 100644 --- a/test/casperjs/api-hda-tests.js +++ b/test/casperjs/api-hda-tests.js @@ -174,24 +174,14 @@ spaceghost.historypanel.waitForHdas().then( function(){ hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); this.test.assert( hdaShow.name === 'New name', "Update sanitized name: " + hdaShow.name ); - //NOTE!: this fails on sqlite3 (with default setup) - try { - this.test.comment( 'update should allow unicode in names' ); - var unicodeName = 'Ржевский сапоги'; - returned = this.api.hdas.update( lastHistory.id, firstHda.id, { - name : unicodeName - }); - //this.debug( 'returned:\n' + this.jsonStr( returned ) ); - hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); - this.test.assert( hdaShow.name === unicodeName, "Update accepted unicode name: " + hdaShow.name ); - } catch( err ){ - //this.debug( this.jsonStr( err ) ); - if( ( err instanceof this.api.APIError ) - && ( err.status === 500 ) - && ( err.message.indexOf( '(ProgrammingError) You must not use 8-bit bytestrings' ) !== -1 ) ){ - this.skipTest( 'Unicode update failed. Are you using sqlite3 as the db?' ); - } - } + this.test.comment( 'update should allow unicode in names' ); + var unicodeName = 'Ржевский сапоги'; + returned = this.api.hdas.update( lastHistory.id, firstHda.id, { + name : unicodeName + }); + //this.debug( 'returned:\n' + this.jsonStr( returned ) ); + hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); + this.test.assert( hdaShow.name === unicodeName, "Update accepted unicode name: " + hdaShow.name ); this.test.comment( 'update should allow escaped quotations in names' ); var quotedName = '"Bler"'; @@ -239,11 +229,11 @@ spaceghost.historypanel.waitForHdas().then( function(){ }); //this.debug( 'returned:\n' + this.jsonStr( returned ) ); hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); + //this.debug( 'hdaShow:\n' + this.jsonStr( hdaShow ) ); this.test.assert( hdaShow.genome_build === 'hg18', "genome_build successfully set via update: " + hdaShow.genome_build ); this.test.assert( hdaShow.metadata_dbkey === 'hg18', "metadata_dbkey successfully set via the same update: " + hdaShow.metadata_dbkey ); - this.test.comment( 'update should sanitize any genome_build' ); returned = this.api.hdas.update( lastHistory.id, firstHda.id, { genome_build : 'hg18' @@ -257,23 +247,14 @@ spaceghost.historypanel.waitForHdas().then( function(){ this.test.comment( 'update should allow unicode in genome builds' ); var unicodeBuild = 'Ржевский18'; - //NOTE!: this fails on sqlite3 (with default setup) - try { - returned = this.api.hdas.update( lastHistory.id, firstHda.id, { - name : unicodeBuild - }); - //this.debug( 'returned:\n' + this.jsonStr( returned ) ); - hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); - this.test.assert( hdaShow.genome_build === unicodeBuild, - "Update accepted unicode genome_build: " + hdaShow.name ); - } catch( err ){ - //this.debug( this.jsonStr( err ) ); - if( ( err instanceof this.api.APIError ) - && ( err.status === 500 ) - && ( err.message.indexOf( '(ProgrammingError) You must not use 8-bit bytestrings' ) !== -1 ) ){ - this.skipTest( 'Unicode update failed. Are you using sqlite3 as the db?' ); - } - } + returned = this.api.hdas.update( lastHistory.id, firstHda.id, { + genome_build : unicodeBuild + }); + this.debug( 'returned:\n' + this.jsonStr( returned ) ); + hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); + this.debug( 'hdaShow:\n' + this.jsonStr( hdaShow ) ); + this.test.assert( hdaShow.genome_build === unicodeBuild, + "Update accepted unicode genome_build: " + hdaShow.genome_build ); // ........................................................................................... misc_info/info this.test.comment( 'update should allow changing the misc_info' ); @@ -298,25 +279,14 @@ spaceghost.historypanel.waitForHdas().then( function(){ this.test.comment( 'update should allow unicode in misc_info' ); var unicodeInfo = '여보!'; - //NOTE!: this fails on sqlite3 (with default setup) - try { - returned = this.api.hdas.update( lastHistory.id, firstHda.id, { - misc_info : unicodeInfo - }); - //this.debug( 'returned:\n' + this.jsonStr( returned ) ); - hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); - this.test.assert( hdaShow.misc_info === unicodeInfo, - "Update accepted unicode misc_info: " + hdaShow.misc_info ); - } catch( err ){ - //this.debug( this.jsonStr( err ) ); - if( ( err instanceof this.api.APIError ) - && ( err.status === 500 ) - && ( err.message.indexOf( '(ProgrammingError) You must not use 8-bit bytestrings' ) !== -1 ) ){ - this.skipTest( 'Unicode update failed. Are you using sqlite3 as the db?' ); - } - } + returned = this.api.hdas.update( lastHistory.id, firstHda.id, { + misc_info : unicodeInfo + }); + //this.debug( 'returned:\n' + this.jsonStr( returned ) ); + hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); + this.test.assert( hdaShow.misc_info === unicodeInfo, + "Update accepted unicode misc_info: " + hdaShow.misc_info ); -/* // ........................................................................................... annotation // currently fails because no annotation is returned in details this.test.comment( 'update should allow changing the annotation' ); @@ -338,25 +308,15 @@ spaceghost.historypanel.waitForHdas().then( function(){ this.test.assert( hdaShow.annotation === 'New annotation', "Update sanitized annotation: " + hdaShow.annotation ); - //NOTE!: this fails on sqlite3 (with default setup) - try { - this.test.comment( 'update should allow unicode in annotations' ); - var unicodeAnnotation = 'お願いは、それが落下させない'; - returned = this.api.hdas.update( lastHistory.id, firstHda.id, { - annotation : unicodeAnnotation - }); - //this.debug( 'returned:\n' + this.jsonStr( returned ) ); - hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); - this.test.assert( hdaShow.annotation === unicodeAnnotation, - "Update accepted unicode annotation: " + hdaShow.annotation ); - } catch( err ){ - //this.debug( this.jsonStr( err ) ); - if( ( err instanceof this.api.APIError ) - && ( err.status === 500 ) - && ( err.message.indexOf( '(ProgrammingError) You must not use 8-bit bytestrings' ) !== -1 ) ){ - this.skipTest( 'Unicode update failed. Are you using sqlite3 as the db?' ); - } - } + this.test.comment( 'update should allow unicode in annotations' ); + var unicodeAnnotation = 'お願いは、それが落下させない'; + returned = this.api.hdas.update( lastHistory.id, firstHda.id, { + annotation : unicodeAnnotation + }); + //this.debug( 'returned:\n' + this.jsonStr( returned ) ); + hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); + this.test.assert( hdaShow.annotation === unicodeAnnotation, + "Update accepted unicode annotation: " + hdaShow.annotation ); this.test.comment( 'update should allow escaped quotations in annotations' ); var quotedAnnotation = '"Bler"'; @@ -367,8 +327,6 @@ spaceghost.historypanel.waitForHdas().then( function(){ hdaShow = this.api.hdas.show( lastHistory.id, firstHda.id ); this.test.assert( hdaShow.annotation === quotedAnnotation, "Update accepted escaped quotations in annotation: " + hdaShow.annotation ); -*/ - // ------------------------------------------------------------------------------------------- ERRORS this.test.comment( 'create should error with "Please define the source" when the param "from_ld_id" is not used' ); @@ -391,6 +349,8 @@ spaceghost.historypanel.waitForHdas().then( function(){ //var returned = this.api.hdas.update( lastHistory.id, { deleted: true, blerp: 'blerp' }); //this.debug( 'returned:' + this.jsonStr( returned ) ); //this.debug( 'page:' + this.jsonStr( this.page ) ); +/* +*/ }); // =================================================================== diff --git a/test/casperjs/api-history-tests.js b/test/casperjs/api-history-tests.js index 76ec3071416..1339e0c63b0 100644 --- a/test/casperjs/api-history-tests.js +++ b/test/casperjs/api-history-tests.js @@ -218,23 +218,14 @@ spaceghost.thenOpen( spaceghost.baseUrl ).then( function(){ this.test.assert( historyShow.name === 'New name', "Update sanitized name: " + historyShow.name ); //NOTE!: this fails on sqlite3 (with default setup) - try { - this.test.comment( 'update should allow unicode in names' ); - var unicodeName = '桜ゲノム'; - returned = this.api.histories.update( newFirstHistory.id, { - name : unicodeName - }); - //this.debug( 'returned:\n' + this.jsonStr( returned ) ); - historyShow = this.api.histories.show( newFirstHistory.id ); - this.test.assert( historyShow.name === unicodeName, "Update accepted unicode name: " + historyShow.name ); - } catch( err ){ - //this.debug( this.jsonStr( err ) ); - if( ( err instanceof this.api.APIError ) - && ( err.status === 500 ) - && ( err.message.indexOf( '(ProgrammingError) You must not use 8-bit bytestrings' ) !== -1 ) ){ - this.skipTest( 'Unicode update failed. Are you using sqlite3 as the db?' ); - } - } + this.test.comment( 'update should allow unicode in names' ); + var unicodeName = '桜ゲノム'; + returned = this.api.histories.update( newFirstHistory.id, { + name : unicodeName + }); + //this.debug( 'returned:\n' + this.jsonStr( returned ) ); + historyShow = this.api.histories.show( newFirstHistory.id ); + this.test.assert( historyShow.name === unicodeName, "Update accepted unicode name: " + historyShow.name ); this.test.comment( 'update should allow escaped quotations in names' ); var quotedName = '"Bler"'; @@ -294,27 +285,16 @@ spaceghost.thenOpen( spaceghost.baseUrl ).then( function(){ this.test.assert( historyShow.genome_build === 'hg18', "Update sanitized genome_build: " + historyShow.genome_build ); + // removing for now until I can determine the relationship between unicode and genome_builds this.test.comment( 'update should allow unicode in genome builds' ); var unicodeBuild = '桜12'; - //NOTE!: this fails on sqlite3 (with default setup) - try { - returned = this.api.histories.update( newFirstHistory.id, { - name : unicodeBuild - }); - //this.debug( 'returned:\n' + this.jsonStr( returned ) ); - historyShow = this.api.histories.show( newFirstHistory.id ); - this.test.assert( historyShow.genome_build === unicodeBuild, - "Update accepted unicode genome_build: " + historyShow.genome_build ); - } catch( err ){ - //this.debug( this.jsonStr( err ) ); - if( ( err instanceof this.api.APIError ) - && ( err.status === 500 ) - && ( err.message.indexOf( '(ProgrammingError) You must not use 8-bit bytestrings' ) !== -1 ) ){ - this.skipTest( 'Unicode update failed. Are you using sqlite3 as the db?' ); - } else { - throw err; - } - } + returned = this.api.histories.update( newFirstHistory.id, { + genome_build : unicodeBuild + }); + //this.debug( 'returned:\n' + this.jsonStr( returned ) ); + historyShow = this.api.histories.show( newFirstHistory.id ); + this.test.assert( historyShow.genome_build === unicodeBuild, + "Update accepted unicode genome_build: " + historyShow.genome_build ); // ........................................................................................... annotation @@ -337,25 +317,15 @@ spaceghost.thenOpen( spaceghost.baseUrl ).then( function(){ this.test.assert( historyShow.annotation === 'New annotation', "Update sanitized annotation: " + historyShow.annotation ); - //NOTE!: this fails on sqlite3 (with default setup) - try { - this.test.comment( 'update should allow unicode in annotations' ); - var unicodeAnnotation = 'お願いは、それが落下させない'; - returned = this.api.histories.update( newFirstHistory.id, { - annotation : unicodeAnnotation - }); - //this.debug( 'returned:\n' + this.jsonStr( returned ) ); - historyShow = this.api.histories.show( newFirstHistory.id ); - this.test.assert( historyShow.annotation === unicodeAnnotation, - "Update accepted unicode annotation: " + historyShow.annotation ); - } catch( err ){ - //this.debug( this.jsonStr( err ) ); - if( ( err instanceof this.api.APIError ) - && ( err.status === 500 ) - && ( err.message.indexOf( '(ProgrammingError) You must not use 8-bit bytestrings' ) !== -1 ) ){ - this.skipTest( 'Unicode update failed. Are you using sqlite3 as the db?' ); - } - } + this.test.comment( 'update should allow unicode in annotations' ); + var unicodeAnnotation = 'お願いは、それが落下させない'; + returned = this.api.histories.update( newFirstHistory.id, { + annotation : unicodeAnnotation + }); + //this.debug( 'returned:\n' + this.jsonStr( returned ) ); + historyShow = this.api.histories.show( newFirstHistory.id ); + this.test.assert( historyShow.annotation === unicodeAnnotation, + "Update accepted unicode annotation: " + historyShow.annotation ); this.test.comment( 'update should allow escaped quotations in annotations' ); var quotedAnnotation = '"Bler"'; @@ -371,7 +341,6 @@ spaceghost.thenOpen( spaceghost.baseUrl ).then( function(){ // ------------------------------------------------------------------------------------------- ERRORS //TODO: make sure expected errors are being passed back (but no permissions checks here - different suite) // bad ids: index, show, update, delete, undelete - /* */ //this.debug( this.jsonStr( historyShow ) );