Improve file path validation

api/src/org/labkey/api/assay/AbstractAssayProvider.java

@@ -115,6 +115,7 @@
 import org.labkey.api.view.NavTree;
 import org.labkey.api.view.NotFoundException;
 import org.labkey.api.view.ViewContext;
+import org.labkey.vfs.FileLike;
 import org.labkey.vfs.FileSystemLike;
 import org.springframework.validation.BindException;
 import org.springframework.web.servlet.ModelAndView;
@@ -612,7 +613,7 @@ public List<Pair<Domain, Map<DomainProperty, Object>>> createDefaultDomains(Cont
     }
 
     @Override
-    public List<AssayDataCollector> getDataCollectors(@Nullable Map<String, File> uploadedFiles, AssayRunUploadForm context)
+    public List<AssayDataCollector> getDataCollectors(@Nullable Map<String, org.labkey.vfs.FileLike> uploadedFiles, AssayRunUploadForm context)
     {
         return getDataCollectors(uploadedFiles, context, true);
     }
@@ -623,7 +624,7 @@ public String getResourceName()
         return getName();
     }
 
-    public List<AssayDataCollector> getDataCollectors(@Nullable Map<String, File> uploadedFiles, AssayRunUploadForm<?> context, boolean allowFileReuseOnReRun)
+    public List<AssayDataCollector> getDataCollectors(@Nullable Map<String, org.labkey.vfs.FileLike> uploadedFiles, AssayRunUploadForm<?> context, boolean allowFileReuseOnReRun)
     {
         List<AssayDataCollector> result = new ArrayList<>();
         if (!PipelineDataCollector.getFileQueue(context).isEmpty())
@@ -636,7 +637,7 @@ public List<AssayDataCollector> getDataCollectors(@Nullable Map<String, File> up
             {
                 // In the re-run scenario, figure out what files to offer up for reuse
 
-                Map<String, File> reusableFiles = new HashMap<>();
+                Map<String, org.labkey.vfs.FileLike> reusableFiles = new HashMap<>();
                 // Include any files that were uploaded as part of this request
                 if (uploadedFiles != null && !uploadedFiles.isEmpty())
                 {
@@ -674,14 +675,15 @@ else if (inputDatas.size() > 1)
                 // Filter out any files that aren't under the current pipeline root, since we won't be able to resolve
                 // them successfully due to security restrictions for what's an allowable input to the new run. See issue 18387.
                 PipeRoot pipeRoot = PipelineService.get().findPipelineRoot(context.getContainer());
-                for (Iterator<Map.Entry<String, File>> iter = reusableFiles.entrySet().iterator(); iter.hasNext(); )
+                for (Iterator<Map.Entry<String, FileLike>> iter = reusableFiles.entrySet().iterator(); iter.hasNext(); )
                 {
-                    Map.Entry<String, File> entry = iter.next();
+                    Map.Entry<String, FileLike> entry = iter.next();
                     // If it's not under the current pipeline root
-                    if (pipeRoot == null || !pipeRoot.isUnderRoot(entry.getValue()))
+                    if (pipeRoot == null || !pipeRoot.isUnderRoot(entry.getValue().toNioPathForRead()))
                     {
-                        // Remove it from the collection
+                        // Remove it from both collections
                         iter.remove();
+                        reusableFiles.remove(entry.getKey());
                     }
                 }
 
@@ -691,8 +693,7 @@ else if (inputDatas.size() > 1)
                     // to reuse or re-upload
                     if (!reusableFiles.isEmpty())
                     {
-                        var reusableFileLike = FileSystemLike.wrapFiles(reusableFiles);
-                        result.add(new PreviouslyUploadedDataCollector<>(reusableFileLike));
+                        result.add(new PreviouslyUploadedDataCollector<>(reusableFiles));
                     }
                     result.add(new FileUploadDataCollector<>(getMaxFileInputs()));
                 }
@@ -708,21 +709,25 @@ else if (inputDatas.size() > 1)
                 // Normal (non-rerun) scenario
                 if (uploadedFiles != null)
                 {
-                    var uploadedFileLikes = FileSystemLike.wrapFiles(uploadedFiles);
-                    result.add(new PreviouslyUploadedDataCollector<>(uploadedFileLikes));
+                    result.add(new PreviouslyUploadedDataCollector<>(uploadedFiles));
                 }
                 result.add(new FileUploadDataCollector<>(getMaxFileInputs()));
             }
         }
         return result;
     }
 
-    private void addReusableData(Map<String, File> reusableFiles, ExpData inputData)
+    private void addReusableData(Map<String, org.labkey.vfs.FileLike> reusableFiles, ExpData inputData)
     {
         // Not all datas are associated with a file
         if (inputData.getFile() != null)
         {
-            reusableFiles.put(AssayDataCollector.PRIMARY_FILE + (reusableFiles.isEmpty() ? "" : Integer.toString(reusableFiles.size())), inputData.getFile());
+            String key = AssayDataCollector.PRIMARY_FILE + (reusableFiles.isEmpty() ? "" : Integer.toString(reusableFiles.size()));
+            File f = inputData.getFile();
+            if (f != null)
+            {
+                reusableFiles.put(key, org.labkey.vfs.FileSystemLike.wrapFile(f));
+            }
         }
     }
 
@@ -733,7 +738,7 @@ public AssayRunCreator getRunCreator()
     }
 
     @Override
-    public ExpProtocol createAssayDefinition(User user, Container container, String name, String description, ExpProtocol.Status status, XarContext context)
+    public ExpProtocol createAssayDefinition(User user, Container container, String name, String description, ExpProtocol.Status status, @NotNull XarContext context)
             throws ExperimentException
     {
         String protocolLsid = getAssayProtocolLsid(container, name, context);

api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java

@@ -143,19 +143,12 @@ public String toString()
     protected abstract boolean allowEmptyData();
 
     @Override
-    public void importFile(@NotNull ExpData data, File dataFile, @NotNull ViewBackgroundInfo info, @NotNull Logger log, @NotNull XarContext context) throws ExperimentException
+    public void importFile(@NotNull ExpData data, @NotNull FileLike dataFile, @NotNull ViewBackgroundInfo info, @NotNull Logger log, @NotNull XarContext context) throws ExperimentException
     {
-        importFile(data, dataFile, info, log, context, true);
+        importFile(data, dataFile, info, log, context, true, false);
     }
 
-    @Override
-    public void importFile(@NotNull ExpData data, File dataFile, @NotNull ViewBackgroundInfo info, @NotNull Logger log, @NotNull XarContext context, boolean allowLookupByAlternateKey) throws ExperimentException
-    {
-        importFile(data, dataFile, info, log, context, allowLookupByAlternateKey, false);
-    }
-
-    @Override
-    public void importFile(@NotNull ExpData data, File dataFile, @NotNull ViewBackgroundInfo info, @NotNull Logger log, @NotNull XarContext context, boolean allowLookupByAlternateKey, boolean autoFillDefaultResultColumns) throws ExperimentException
+    protected void importFile(@NotNull ExpData data, @NotNull FileLike dataFile, @NotNull ViewBackgroundInfo info, @NotNull Logger log, @NotNull XarContext context, boolean allowLookupByAlternateKey, boolean autoFillDefaultResultColumns) throws ExperimentException
     {
         ExpProtocolApplication sourceApplication = data.getSourceApplication();
         if (sourceApplication == null)
@@ -175,7 +168,7 @@ public void importFile(@NotNull ExpData data, File dataFile, @NotNull ViewBackgr
         // type conversion error).
         settings.setBestEffortConversion(true);
 
-        FileLike fo = FileSystemLike.wrapFile(dataFile);
+        FileLike fo = dataFile;
         Map<DataType, DataIteratorBuilder> rawData = getValidationDataMap(data, fo, info, log, context, settings);
         assert(rawData.size() <= 1);
         try

api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java

@@ -47,31 +47,7 @@ public abstract class AbstractTempDirDataCollector<ContextType extends AssayRunU
     protected boolean _uploadComplete = false;
 
     private static final Logger LOG = LogHelper.getLogger(AbstractTempDirDataCollector.class, "Activity related to data collection into a temporary directory");
-
-    private void removeTempDir(ContextType context) throws ExperimentException
-    {
-        // TODO: remove this instanceof check
-        if (!(context instanceof AssayRunUploadForm))
-            return;
-
-        String uploadAttemptID = ((AssayRunUploadForm<?>) context).getUploadAttemptID();
-
-        // Cleanup files other than input generated by transform scripts
-        FileLike tempDir = ensureSubdirectory(context.getContainer(), TEMP_DIR_NAME);
-        FileLike uploadAttemptDir = tempDir.resolveChild(uploadAttemptID);
-        if (NetworkDrive.exists(uploadAttemptDir))
-        {
-            try
-            {
-                FileUtils.deleteDirectory(uploadAttemptDir.toNioPathForWrite().toFile());
-            }
-            catch (IOException e)
-            {
-                // Delete quietly
-            }
-        }
-    }
-
+
     @Override
     public void initDir(ContextType context) throws ExperimentException
     {
@@ -234,12 +210,6 @@ public Map<String, FileLike> uploadComplete(ContextType context, @Nullable ExpRu
         return result;
     }
 
-    /** @return the preferred name for the run given the primary data file */
-    protected String getPreferredAssayId(File primaryFile)
-    {
-        return primaryFile.getName();
-    }
-
     protected String getPreferredAssayId(FileLike primaryFile)
     {
         return primaryFile.getName();

api/src/org/labkey/api/assay/AssayDataCollector.java

@@ -23,7 +23,6 @@
 import org.labkey.api.view.HttpView;
 import org.labkey.vfs.FileLike;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.Map;
 
@@ -57,7 +56,7 @@ public String getButtonText()
     }
 
     /** @return the UI to plug into the import wizard for the user to somehow select/upload the file */
-    HttpView getView(ContextType context) throws ExperimentException;
+    HttpView<?> getView(ContextType context) throws ExperimentException;
 
     /** @return the name for this AssayDataCollector. Needs to be unique within the set of data collectors for any given import attempt */
     String getShortName();
@@ -81,5 +80,5 @@ default void initDir(ContextType context) throws ExperimentException {}
      * @return null if the file was uploaded as part of the import
      */
     @Nullable
-    default File getOriginalFileLocation() { return null; }
+    default FileLike getOriginalFileLocation() { return null; }
 }

api/src/org/labkey/api/assay/AssayFileWriter.java

@@ -33,7 +33,6 @@
 import org.labkey.api.util.NetworkDrive;
 import org.labkey.api.view.ViewContext;
 import org.labkey.vfs.FileLike;
-import org.labkey.vfs.FileSystemLike;
 import org.springframework.web.multipart.MultipartFile;
 import org.springframework.web.multipart.MultipartHttpServletRequest;
 
@@ -189,8 +188,7 @@ protected FileLike getFileTargetDir(ContextType context) throws ExperimentExcept
         return ensureUploadDirectory(context.getContainer());
     }
 
-    /* TODO: this is a really awkward transition between File->FileLike (files come from FileQueue) */
-    public Map<String, FileLike> savePipelineFiles(ContextType context, Map<String, File> files) throws ExperimentException, IOException
+    public Map<String, FileLike> savePipelineFiles(ContextType context, Map<String, FileLike> files) throws ExperimentException, IOException
     {
         Map<String, FileLike> savedFiles = CollectionUtils.enforceValueClass(new TreeMap<>(), FileLike.class);
         if (context.getRequest() instanceof MultipartHttpServletRequest)
@@ -201,27 +199,24 @@ public Map<String, FileLike> savePipelineFiles(ContextType context, Map<String,
             for (String key : files.keySet())
             {
                 // Copy the user uploaded files not already under the pipeline root to the temp directory.
-                File file = files.get(key);
+                FileLike file = files.get(key);
                 if (!root.isUnderRoot(file))
                 {
                     FileLike savedFile = dir.resolveChild(file.getName());
                     LOG.debug("savePipelineFiles: file '" + file + "' is not under pipeline root. copying to savedFile=" + savedFile);
-                    FileUtils.copyFile(file, toFileForWrite(savedFile));
+                    FileUtils.copyFile(toFileForRead(file), toFileForWrite(savedFile));
                     savedFiles.put(key, savedFile);
                 }
                 else
                 {
-                    savedFiles.put(key, FileSystemLike.wrapFile(file));
+                    savedFiles.put(key, file);
                     LOG.debug("savePipelineFiles: file '" + file.getPath() + "' is already under pipeline root. not copying");
                 }
             }
         }
         else
         {
-            for (var entry : files.entrySet())
-            {
-                savedFiles.put(entry.getKey(), FileSystemLike.wrapFile((entry.getValue())));
-            }
+            savedFiles.putAll(files);
         }
 
         return savedFiles;
@@ -232,7 +227,7 @@ public String getFileName(MultipartFile file)
         return file.getOriginalFilename();
     }
 
-    public Map<String, FileLike> savePostedFiles(ContextType context, Set<String> parameterNames, boolean allowMultiple, boolean ensureExpData) throws ExperimentException, IOException
+    public Map<String, FileLike> savePostedFiles(ContextType context, @NotNull Set<String> parameterNames, boolean allowMultiple, boolean ensureExpData) throws ExperimentException, IOException
     {
         Map<String, FileLike> files = CollectionUtils.enforceValueClass(new TreeMap<>(), FileLike.class);
         Set<String> originalFileNames = new HashSet<>();
@@ -244,7 +239,7 @@ public Map<String, FileLike> savePostedFiles(ContextType context, Set<String> pa
             while (iter.hasNext())
             {
                 Map.Entry<String, List<MultipartFile>> entry = iter.next();
-                if (parameterNames == null || parameterNames.contains(entry.getKey()))
+                if (parameterNames.contains(entry.getKey()))
                 {
                     List<MultipartFile> multipartFiles = entry.getValue();
                     boolean isAfterFirstFile = false;

api/src/org/labkey/api/assay/AssayProvider.java

@@ -59,10 +59,8 @@
 import org.springframework.web.servlet.ModelAndView;
 import org.springframework.web.servlet.mvc.Controller;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -116,17 +114,7 @@ enum ReRunSupport
     AssayRunCreator getRunCreator();
 
     /** @return all the legal data collectors that the user can choose from for the current import attempt */
-    // TODO File->FileLike
-    List<AssayDataCollector> getDataCollectors(Map<String, File> uploadedFiles, AssayRunUploadForm context);
-
-    default List<AssayDataCollector> getDataCollectorsFileObject(Map<String, FileLike> uploadedFileObjects, AssayRunUploadForm context)
-    {
-        Map<String,File> map = new HashMap<>();
-        if (uploadedFileObjects != null)
-            for (var entry : uploadedFileObjects.entrySet())
-                map.put(entry.getKey(), entry.getValue().toNioPathForRead().toFile());
-        return getDataCollectors(map, context);
-    }
+    List<AssayDataCollector> getDataCollectors(Map<String, FileLike> uploadedFileObjects, AssayRunUploadForm context);
 
     /**
      * @return the name of the assay provider.

api/src/org/labkey/api/assay/AssayRunDatabaseContext.java

@@ -182,14 +182,14 @@ public Map<String, FileLike> getUploadedData() throws ExperimentException
 
     @Nullable
     @Override
-    public File getOriginalFileLocation()
+    public FileLike getOriginalFileLocation()
     {
         for (ExpData data : _run.getOutputDatas(_provider.getDataType()))
         {
             File f = data.getFile();
             if (f != null)
             {
-                return f.getParentFile();
+                return FileSystemLike.wrapFile(f.getParentFile());
             }
         }
         return null;

api/src/org/labkey/api/assay/AssayRunUploadContext.java

@@ -213,7 +213,7 @@ default void init() throws ExperimentException
      * @return null if the file was uploaded as part of the import
      */
     @Nullable
-    default File getOriginalFileLocation()
+    default FileLike getOriginalFileLocation()
     {
         return null;
     }

api/src/org/labkey/api/assay/DefaultAssayRunCreator.java

@@ -607,7 +607,15 @@ protected void importStandardResultData(
             {
                 ExperimentDataHandler dataHandler = insertedData.findDataHandler();
 
-                dataHandler.importFile(insertedData, insertedData.getFile(), info, logger, xarContext, context.isAllowLookupByAlternateKey(), context.shouldAutoFillDefaultResultColumns());
+                FileLike fileLike = FileSystemLike.wrapFile(insertedData.getFile());
+                if (dataHandler instanceof AbstractAssayTsvDataHandler tsvHandler)
+                {
+                    tsvHandler.importFile(insertedData, fileLike, info, logger, xarContext, context.isAllowLookupByAlternateKey(), context.shouldAutoFillDefaultResultColumns());
+                }
+                else
+                {
+                    dataHandler.importFile(insertedData, fileLike, info, logger, xarContext);
+                }
             }
         }
     }

api/src/org/labkey/api/assay/FileUploadDataCollector.java

@@ -23,7 +23,6 @@
 import org.labkey.api.view.JspView;
 import org.labkey.vfs.FileLike;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
@@ -38,7 +37,7 @@
 public class FileUploadDataCollector<ContextType extends AssayRunUploadContext<? extends AssayProvider>> extends AbstractTempDirDataCollector<ContextType>
 {
     private final int _maxFileInputs;
-    private final Map<String, File> _reusableFiles;
+    private final Map<String, FileLike> _reusableFiles;
     // Name of the form <input> for the file.
     private final String _fileInputName;
 
@@ -52,12 +51,12 @@ public FileUploadDataCollector(int maxFileInputs)
         this(maxFileInputs, Collections.emptyMap());
     }
 
-    public FileUploadDataCollector(int maxFileInputs, Map<String, File> reusableFiles)
+    public FileUploadDataCollector(int maxFileInputs, Map<String, FileLike> reusableFiles)
     {
         this(maxFileInputs, reusableFiles, PRIMARY_FILE);
     }
 
-    public FileUploadDataCollector(int maxFileInputs, Map<String, File> reusableFiles, String fileInputName)
+    public FileUploadDataCollector(int maxFileInputs, Map<String, FileLike> reusableFiles, String fileInputName)
     {
         _maxFileInputs = maxFileInputs;
         _reusableFiles = Collections.unmodifiableMap(reusableFiles);
@@ -69,12 +68,12 @@ public FileUploadDataCollector(int maxFileInputs, Map<String, File> reusableFile
     }
 
     @Override
-    public HttpView getView(ContextType context)
+    public HttpView<?> getView(ContextType context)
     {
-        return new JspView<FileUploadDataCollector>("/org/labkey/api/assay/fileUpload.jsp", this);
+        return new JspView<>("/org/labkey/api/assay/fileUpload.jsp", this);
     }
 
-    public Map<String, File> getReusableFiles()
+    public Map<String, FileLike> getReusableFiles()
     {
         return _reusableFiles;
     }