From 85ba013e6d405cbb4de5ab1b649673fdb9d954b8 Mon Sep 17 00:00:00 2001 From: Nate Coraor Date: Thu, 8 Dec 2011 13:22:39 -0500 Subject: [PATCH] Actual User: Fix for newline conversion on upload, tighten file permissions for files in upload. Please make sure you clean your temp directory regularly. --- lib/galaxy/datatypes/sniff.py | 2 +- lib/galaxy/security/__init__.py | 2 +- lib/galaxy/tools/actions/__init__.py | 2 +- lib/galaxy/tools/actions/upload_common.py | 32 ++++++++++------ tools/data_source/upload.py | 46 ++++++++++------------- 5 files changed, 43 insertions(+), 41 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 14f0f32d428..379c3713add 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -95,7 +95,7 @@ def convert_newlines( fname, in_place=True ): fp.write( "%s\n" % line.rstrip( "\r\n" ) ) fp.close() if in_place: - shutil.copyfile( temp_name, fname ) + shutil.move( temp_name, fname ) # Return number of lines in file. return ( i + 1, None ) else: diff --git a/lib/galaxy/security/__init__.py b/lib/galaxy/security/__init__.py index b2289b1940a..716964290b5 100644 --- a/lib/galaxy/security/__init__.py +++ b/lib/galaxy/security/__init__.py @@ -2,7 +2,7 @@ Galaxy Security """ -import logging, socket, operator +import logging, socket, operator, pwd from datetime import datetime, timedelta from galaxy.util.bunch import Bunch from galaxy.util import listify diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index aa76a47ad97..9f8084137c1 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -282,7 +282,7 @@ class DefaultToolAction( object ): # Create an empty file immediately open( data.file_name, "w" ).close() # Fix permissions - util.umask_fix_perms( data.file_name, trans.app.config.umask, 0666) + util.umask_fix_perms( data.file_name, trans.app.config.umask, 0666 ) # This may not be neccesary with the new parent/child associations data.designation = name # Copy metadata from one of the inputs if requested. diff --git a/lib/galaxy/tools/actions/upload_common.py b/lib/galaxy/tools/actions/upload_common.py index 671ff72624e..b2fa51153ab 100644 --- a/lib/galaxy/tools/actions/upload_common.py +++ b/lib/galaxy/tools/actions/upload_common.py @@ -1,4 +1,4 @@ -import os, tempfile, StringIO +import os, tempfile, StringIO, pwd, subprocess from cgi import FieldStorage from galaxy import datatypes, util from galaxy.util.odict import odict @@ -252,6 +252,18 @@ def create_paramfile( trans, uploaded_datasets ): """ Create the upload tool's JSON "param" file. """ + def _chown( path ): + try: + pwent = pwd.getpwnam( trans.user.email.split('@')[0] ) + cmd = [ '/usr/bin/sudo', '-E', trans.app.config.external_chown_script, path, pwent[0], str( pwent[3] ) ] + log.debug( 'Changing ownership of %s with: %s' % ( path, ' '.join( cmd ) ) ) + p = subprocess.Popen( cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE ) + stdout, stderr = p.communicate() + assert p.returncode == 0, stderr + except Exception, e: + log.warning( 'Changing ownership of uploaded file %s failed: %s' % ( path, str( e ) ) ) + + # TODO: json_file should go in the working directory json_file = tempfile.mkstemp() json_file_path = json_file[1] json_file = os.fdopen( json_file[0], 'w' ) @@ -279,10 +291,8 @@ def create_paramfile( trans, uploaded_datasets ): is_binary = None try: link_data_only = uploaded_dataset.link_data_only - chmod_flag = 1 except: link_data_only = 'copy_files' - chmod_flag = 0 json = dict( file_type = uploaded_dataset.file_type, ext = uploaded_dataset.ext, name = uploaded_dataset.name, @@ -292,13 +302,17 @@ def create_paramfile( trans, uploaded_datasets ): is_binary = is_binary, link_data_only = link_data_only, space_to_tab = uploaded_dataset.space_to_tab, + in_place = trans.app.config.external_chown_script is None, path = uploaded_dataset.path ) - if chmod_flag == 0 and trans.app.config.drmaa_external_runjob_script: - os.chmod(uploaded_dataset.path, 0777) + # TODO: This will have to change when we start bundling inputs. + # Also, in_place above causes the file to be left behind since the + # user cannot remove it unless the parent directory is writable. + if link_data_only == 'copy_files' and trans.app.config.external_chown_script: + _chown( uploaded_dataset.path ) json_file.write( to_json_string( json ) + '\n' ) json_file.close() - if trans.app.config.drmaa_external_runjob_script: - os.chmod(json_file_path, 0777) + if trans.app.config.external_chown_script: + _chown( json_file_path ) return json_file_path def create_job( trans, params, tool, json_file_path, data_list, folder=None ): """ @@ -331,16 +345,12 @@ def create_job( trans, params, tool, json_file_path, data_list, folder=None ): # Create an empty file immediately if not dataset.dataset.external_filename: open( dataset.file_name, "w" ).close() - if trans.app.config.drmaa_external_runjob_script: - os.chmod(dataset.file_name, 0777) else: for i, dataset in enumerate( data_list ): job.add_output_dataset( 'output%i' % i, dataset ) # Create an empty file immediately if not dataset.dataset.external_filename: open( dataset.file_name, "w" ).close() - if trans.app.config.drmaa_external_runjob_script: - os.chmod(dataset.file_name, 0777) job.state = job.states.NEW trans.sa_session.add( job ) diff --git a/tools/data_source/upload.py b/tools/data_source/upload.py index d2e818d5920..465c77a2fb8 100644 --- a/tools/data_source/upload.py +++ b/tools/data_source/upload.py @@ -80,6 +80,7 @@ def add_file( dataset, registry, json_file, output_path ): converted_path = None stdout = None link_data_only = dataset.get( 'link_data_only', 'copy_files' ) + in_place = dataset.get( 'in_place', True ) try: ext = dataset.file_type @@ -161,7 +162,7 @@ def add_file( dataset, registry, json_file, output_path ): os.close( fd ) gzipped_file.close() # Replace the gzipped file with the decompressed file if it's safe to do so - if dataset.type in ( 'server_dir', 'path_paste' ): + if dataset.type in ( 'server_dir', 'path_paste' ) or not in_place: dataset.path = uncompressed else: shutil.move( uncompressed, dataset.path ) @@ -194,7 +195,7 @@ def add_file( dataset, registry, json_file, output_path ): os.close( fd ) bzipped_file.close() # Replace the bzipped file with the decompressed file if it's safe to do so - if dataset.type in ( 'server_dir', 'path_paste' ): + if dataset.type in ( 'server_dir', 'path_paste' ) or not in_place: dataset.path = uncompressed else: shutil.move( uncompressed, dataset.path ) @@ -251,7 +252,7 @@ def add_file( dataset, registry, json_file, output_path ): z.close() # Replace the zipped file with the decompressed file if it's safe to do so if uncompressed is not None: - if dataset.type in ( 'server_dir', 'path_paste' ): + if dataset.type in ( 'server_dir', 'path_paste' ) or not in_place: dataset.path = uncompressed else: shutil.move( uncompressed, dataset.path ) @@ -280,7 +281,6 @@ def add_file( dataset, registry, json_file, output_path ): return if data_type != 'binary': if link_data_only == 'copy_files': - in_place = True if dataset.type in ( 'server_dir', 'path_paste' ) and data_type not in [ 'gzip', 'bz2', 'zip' ]: in_place = False if dataset.space_to_tab: @@ -305,27 +305,18 @@ def add_file( dataset, registry, json_file, output_path ): 'Copy files into Galaxy instead of Link to files without copying into Galaxy so grooming can be performed.' file_err( err_msg, dataset, json_file ) return - if link_data_only == 'copy_files' and dataset.type in ( 'server_dir', 'path_paste' ) and data_type not in [ 'gzip', 'bz2', 'zip' ]: - # Move the dataset to its "real" path - if converted_path is not None: - shutil.copy( converted_path, output_path ) - try: - os.remove( converted_path ) - except: - pass - else: - # This should not happen, but it's here just in case - shutil.move( dataset.path, output_path ) - try: - os.chmod(output_path,0644) - except: - pass - elif link_data_only == 'copy_files': + if link_data_only == 'copy_files' and converted_path is not None: + # Move the converted dataset to its "real" path + shutil.move( converted_path, output_path ) + elif link_data_only == 'copy_files' and in_place: + # Dataset was not converted but should still be removed from original location shutil.move( dataset.path, output_path ) - try: - os.chmod(output_path,0644) - except: - pass + elif link_data_only == 'copy_files': + shutil.copy( dataset.path, output_path ) + + if link_data_only == 'copy_files' and datatype.dataset_content_needs_grooming( output_path ): + # Groom the dataset content if necessary + datatype.groom_dataset_content( output_path ) # Write the job info stdout = stdout or 'uploaded %s file' % data_type @@ -336,9 +327,7 @@ def add_file( dataset, registry, json_file, output_path ): name = dataset.name, line_count = line_count ) json_file.write( to_json_string( info ) + "\n" ) - if link_data_only == 'copy_files' and datatype.dataset_content_needs_grooming( output_path ): - # Groom the dataset content if necessary - datatype.groom_dataset_content( output_path ) + def add_composite_file( dataset, registry, json_file, output_path, files_path ): if dataset.composite_files: os.mkdir( files_path ) @@ -396,7 +385,10 @@ def __main__(): add_composite_file( dataset, registry, json_file, output_path, files_path ) else: add_file( dataset, registry, json_file, output_path ) + # clean up paramfile + # TODO: this will not work when running as the actual user unless the + # parent directory is writable by the user. try: os.remove( sys.argv[3] ) except: