From 623a3b81ca71d585f2e7c2a7f0372b6382be3a89 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Wed, 27 Mar 2019 14:04:33 -0400 Subject: [PATCH 1/8] check for validity of user-given changesets --- .../tool_shed/controllers/repository.py | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/webapps/tool_shed/controllers/repository.py b/lib/galaxy/webapps/tool_shed/controllers/repository.py index 9a9ca34a3e4..7398ff30f9b 100644 --- a/lib/galaxy/webapps/tool_shed/controllers/repository.py +++ b/lib/galaxy/webapps/tool_shed/controllers/repository.py @@ -1721,6 +1721,7 @@ class RepositoryController(BaseUIController, ratings_util.ItemRatings): user_id = kwd.get('user_id', None) repository_id = kwd.get('repository_id', None) changeset_revision = kwd.get('changeset_revision', None) + self.validate_changeset_revision(trans, changeset_revision, repository_id) return trans.fill_template('/webapps/tool_shed/index.mako', repository_metadata=repository_metadata, can_administer_repositories=can_administer_repositories, @@ -2167,6 +2168,7 @@ class RepositoryController(BaseUIController, ratings_util.ItemRatings): status = kwd.get('status', 'done') repository = repository_util.get_repository_in_tool_shed(trans.app, repository_id) changeset_revision = kwd.get('changeset_revision', repository.tip(trans.app)) + self.validate_changeset_revision(trans, changeset_revision, repository_id) repository_metadata = metadata_util.get_repository_metadata_by_changeset_revision(trans.app, repository_id, changeset_revision) if repository_metadata: repository_metadata_id = trans.security.encode_id(repository_metadata.id), @@ -2801,13 +2803,7 @@ class RepositoryController(BaseUIController, ratings_util.ItemRatings): repo = hg_util.get_repo_for_repository(trans.app, repository=repository) avg_rating, num_ratings = self.get_ave_item_rating_data(trans.sa_session, repository, webapp_model=trans.model) changeset_revision = kwd.get('changeset_revision', repository.tip(trans.app)) - if not hg_util.get_changectx_for_changeset(repo, changeset_revision): - message = 'Invalid changeset revision' - return trans.response.send_redirect(web.url_for(controller='repository', - action='index', - repository_id=id, - message=message, - status='error')) + self.validate_changeset_revision(trans, changeset_revision, id) repository.share_url = repository_util.generate_sharable_link_for_repository_in_tool_shed(repository, changeset_revision=changeset_revision) repository.clone_url = common_util.generate_clone_url_for_repository_in_tool_shed(trans.user, repository) display_reviews = kwd.get('display_reviews', False) @@ -2902,6 +2898,7 @@ class RepositoryController(BaseUIController, ratings_util.ItemRatings): tool_lineage = [] tool = None guid = None + self.validate_changeset_revision(trans, changeset_revision, repository_id) revision_label = hg_util.get_revision_label(trans.app, repository, changeset_revision, include_date=False) repository_metadata = metadata_util.get_repository_metadata_by_changeset_revision(trans.app, repository_id, changeset_revision) if repository_metadata: @@ -2989,3 +2986,16 @@ class RepositoryController(BaseUIController, ratings_util.ItemRatings): metadata=metadata, message=message, status=status) + + def validate_changeset_revision(self, trans, changeset_revision, repository_id): + """In case changeset revision is invalid send them to the repository page""" + if changeset_revision: + repository = repository_util.get_repository_in_tool_shed(trans.app, repository_id) + repo = hg_util.get_repo_for_repository(trans.app, repository=repository) + if not hg_util.get_changectx_for_changeset(repo, changeset_revision): + message = 'Invalid changeset revision' + return trans.response.send_redirect(web.url_for(controller='repository', + action='index', + repository_id=repository_id, + message=message, + status='error')) From 76af203de8452407ef1f2d52c88e7dd672210ce4 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 29 Mar 2019 03:02:51 +0000 Subject: [PATCH 2/8] Encode content before writing to temp file --- test/shed_functional/base/twilltestcase.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/shed_functional/base/twilltestcase.py b/test/shed_functional/base/twilltestcase.py index 7080af5df4a..bf66965d225 100644 --- a/test/shed_functional/base/twilltestcase.py +++ b/test/shed_functional/base/twilltestcase.py @@ -29,7 +29,7 @@ import galaxy.model.tool_shed_install as galaxy_model import galaxy.util import galaxy.webapps.tool_shed.util.hgweb_config from base.testcase import FunctionalTestCase # noqa: I100,I201,I202 -from galaxy.util import unicodify # noqa: I201 +from galaxy.util import smart_str, unicodify # noqa: I201 from galaxy.web import security # noqa: I201 from tool_shed.util import hg_util, xml_util from tool_shed.util.encoding_util import tool_shed_encode @@ -465,11 +465,9 @@ class ShedTwillTestCase(FunctionalTestCase): return self.wait_for(lambda: self.get_running_datasets(), **kwds) def write_temp_file(self, content, suffix='.html'): - fd, fname = tempfile.mkstemp(suffix=suffix, prefix='twilltestcase-') - f = os.fdopen(fd, "w") - f.write(content) - f.close() - return fname + with tempfile.NamedTemporaryFile(suffix=suffix, prefix='twilltestcase-', delete=False) as fh: + fh.write(smart_str(content)) + return fh.name def add_repository_review_component(self, **kwd): params = { From 6230d93f6acef2d2728c22bbb8302d380ce7b2d9 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Fri, 29 Mar 2019 13:18:33 +0000 Subject: [PATCH 3/8] Use bleach to clean msg which will not strip safe HTML tags and attributes, which we use. Also, restrict the possible values of `status` instead of trying to escape it. --- templates/message.mako | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/templates/message.mako b/templates/message.mako index a47b02c4741..02efb55ed3b 100644 --- a/templates/message.mako +++ b/templates/message.mako @@ -1,4 +1,6 @@ <%! + import bleach + def inherit(context): if context.get('use_panels'): if context.get('webapp'): @@ -56,7 +58,14 @@ ## Render a message <%def name="render_msg( msg, status='done' )"> -
${_(msg)}
-
+ <% + if status == "done": + status = "success" + elif status == "error": + status = "danger" + if status not in ("danger", "info", "success", "warning"): + status = "info" + %> +
${_(bleach.clean(msg))}
From 58612dce4712d6f3f638d0abb337c2232d1f75fd Mon Sep 17 00:00:00 2001 From: pvanheus Date: Fri, 29 Mar 2019 23:51:11 +0200 Subject: [PATCH 4/8] backport fix from #7519 This will allow anyone with an up-to-date 19.01 server to run tests with multiple levels of subdirectories in the `test-data` directory. --- lib/galaxy/tools/verify/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/galaxy/tools/verify/__init__.py b/lib/galaxy/tools/verify/__init__.py index 0b2ee3ccecd..2b862071748 100644 --- a/lib/galaxy/tools/verify/__init__.py +++ b/lib/galaxy/tools/verify/__init__.py @@ -90,6 +90,9 @@ def verify( # if the server's env has GALAXY_TEST_SAVE, save the output file to that dir if keep_outputs_dir: ofn = os.path.join(keep_outputs_dir, filename) + out_dir = os.path.dirname(ofn) + if not os.path.exists(out_dir): + os.makedirs(out_dir) log.debug('keep_outputs_dir: %s, ofn: %s', keep_outputs_dir, ofn) try: shutil.copy(temp_name, ofn) From 95fc75078f83c2164b43c727e7f8a82eeee9531b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 31 Mar 2019 12:38:14 +0200 Subject: [PATCH 5/8] Delay tool exec for discovered & mapped-over input We should only finalize collections up until the map over depth, as indicated by the TODO that is now obsolete. Fixes https://github.com/galaxyproject/galaxy/issues/5867 and probably a bunch of other issues where workflows don't run to completion. --- lib/galaxy/model/__init__.py | 7 +++---- lib/galaxy/tools/execute.py | 4 +++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 294afde52f9..3a4e269dedb 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -3445,14 +3445,13 @@ class DatasetCollection(Dictifiable, UsesAnnotations, RepresentById): self.populated_state = DatasetCollection.populated_states.FAILED self.populated_state_message = message - def finalize(self): + def finalize(self, collection_type_description): # All jobs have written out their elements - everything should be populated # but might not be - check that second case! (TODO) self.mark_as_populated() - if self.has_subcollections: - # THIS IS WRONG - SHOULD ONLY BE TO THE DEPTH OF THE MAP OVER. + if self.has_subcollections and collection_type_description.has_subcollections(): for element in self.elements: - element.child_collection.finalize() + element.child_collection.finalize(collection_type_description.child_collection_type_description()) @property def dataset_instances(self): diff --git a/lib/galaxy/tools/execute.py b/lib/galaxy/tools/execute.py index d2e722cdae0..18f932aa02c 100644 --- a/lib/galaxy/tools/execute.py +++ b/lib/galaxy/tools/execute.py @@ -314,7 +314,9 @@ class ExecutionTracker(object): implicit_collection_jobs = implicit_collection.implicit_collection_jobs implicit_collection_jobs.populated_state = "ok" trans.sa_session.add(implicit_collection_jobs) - implicit_collection.collection.finalize() + implicit_collection.collection.finalize( + collection_type_description=self.collection_info.structure.collection_type_description + ) trans.sa_session.add(implicit_collection.collection) trans.sa_session.flush() From e8ea08dd61851e158eb41e3ea5edd262b3d2d61b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 31 Mar 2019 13:54:50 +0200 Subject: [PATCH 6/8] Add testcase for mapped over discovered dataset consumption --- test/api/test_workflows.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/api/test_workflows.py b/test/api/test_workflows.py index dbbbdd7c96f..dd8ee60307b 100644 --- a/test/api/test_workflows.py +++ b/test/api/test_workflows.py @@ -1130,6 +1130,41 @@ steps: assert len(identifiers) == 6 assert "oe1-ie1" in identifiers + @skip_without_tool("collection_paired_test") + def test_workflow_flatten_with_mapped_over_execution(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(r""" +class: GalaxyWorkflow +inputs: + input_fastqs: collection +steps: + split_up: + tool_id: collection_split_on_column + in: + input1: input_fastqs + flatten: + tool_id: '__FLATTEN__' + in: + input: split_up/split_output + join_identifier: '-' +test_data: + input_fastqs: + type: list + elements: + - identifier: samp1 + content: "0\n1" +""", history_id=history_id) + history = self._get('histories/%s/contents' % history_id).json() + flattened_collection = history[-1] + assert flattened_collection['history_content_type'] == 'dataset_collection' + assert flattened_collection['collection_type'] == 'list' + assert flattened_collection['element_count'] == 2 + nested_collection = self.dataset_populator.get_history_collection_details(history_id, hid=3) + assert nested_collection['collection_type'] == 'list:list' + assert nested_collection['element_count'] == 1 + assert nested_collection['elements'][0]['object']['populated'] + assert nested_collection['elements'][0]['object']['element_count'] == 2 + @skip_without_tool("__APPLY_RULES__") def test_workflow_run_apply_rules(self): with self.dataset_populator.test_history() as history_id: From 8db8ce5de447b9f6b3c2c2cce14618209a55dd22 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 31 Mar 2019 17:15:42 +0200 Subject: [PATCH 7/8] Fix float to int casting Fixes https://github.com/galaxyproject/galaxy/issues/998 --- lib/galaxy/tools/wrappers.py | 2 +- test/functional/tools/cheetah_casting.xml | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index 6f10b668031..79f3883c70c 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -108,7 +108,7 @@ class InputValueWrapper(ToolParameterValueWrapper): return getattr(self.value, key) def __int__(self): - return int(str(self)) + return int(float(self)) def __float__(self): return float(str(self)) diff --git a/test/functional/tools/cheetah_casting.xml b/test/functional/tools/cheetah_casting.xml index d12bb6569e8..ab9c7606158 100644 --- a/test/functional/tools/cheetah_casting.xml +++ b/test/functional/tools/cheetah_casting.xml @@ -1,9 +1,11 @@ #set $int_val_inc = int($inttest) + 1 + #set $int_flot_val_inc = int($floattest) + 1 #set $float_val_inc = float($floattest) + 1 - echo $int_val_inc >> $out_file1; - echo $float_val_inc >> $out_file1; + echo $int_val_inc >> '$out_file1'; + echo $int_flot_val_inc >> '$out_file1'; + echo $float_val_inc >> '$out_file1'; @@ -19,6 +21,7 @@ + From a07c99d5901d7bad1939dfc9ab8cf0b4ab123409 Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Fri, 29 Mar 2019 16:40:35 -0400 Subject: [PATCH 8/8] prevent XSS on old style-mako and expand to the messageLarge method --- templates/message.mako | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/templates/message.mako b/templates/message.mako index 02efb55ed3b..8c980e21f05 100644 --- a/templates/message.mako +++ b/templates/message.mako @@ -53,19 +53,23 @@ ## Render large message. <%def name="render_large_message( message, status )"> -
${_(message)}
+ <% + if status not in ("done", "info", "error", "warning"): + status = "infomessagelarge" + else: + status = status + "messagelarge" + %> +
${_(bleach.clean(message))}
## Render a message <%def name="render_msg( msg, status='done' )"> <% - if status == "done": - status = "success" - elif status == "error": - status = "danger" - if status not in ("danger", "info", "success", "warning"): - status = "info" + if status not in ("done", "info", "error", "warning"): + status = "infomessage" + else: + status = status + "message" %> -
${_(bleach.clean(msg))}
+
${_(bleach.clean(msg))}