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

@@ -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:

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

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. 

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 )

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 ):
                 '<b>Copy files into Galaxy</b> instead of <b>Link to files without copying into Galaxy</b> 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: