Issue 54218: Escape special characters in form field names

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

@@ -24,6 +24,7 @@
 import org.labkey.api.audit.provider.FileSystemAuditProvider;
 import org.labkey.api.collections.CollectionUtils;
 import org.labkey.api.data.Container;
+import org.labkey.api.data.TableViewForm;
 import org.labkey.api.exp.ExperimentException;
 import org.labkey.api.exp.api.ExpProtocol;
 import org.labkey.api.pipeline.PipeRoot;
@@ -40,9 +41,11 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Deque;
+import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -229,72 +232,84 @@ public String getFileName(MultipartFile file)
 
     public Map<String, FileLike> savePostedFiles(ContextType context, @NotNull Set<String> parameterNames, boolean allowMultiple, boolean ensureExpData) throws ExperimentException, IOException
     {
+        if (!(context.getRequest() instanceof MultipartHttpServletRequest multipartRequest))
+            return Collections.emptyMap();
+
         Map<String, FileLike> files = CollectionUtils.enforceValueClass(new TreeMap<>(), FileLike.class);
         Set<String> originalFileNames = new HashSet<>();
-        if (context.getRequest() instanceof MultipartHttpServletRequest multipartRequest)
+        Map<String, String> encodedParameterNames = new HashMap<>();
+        for (String parameterName : parameterNames)
+            encodedParameterNames.put(TableViewForm.getMultiPartFormFieldNameForColumn(parameterName), parameterName);
+
+        Deque<FileLike> overflowFiles = new ArrayDeque<>();  // using a deque for easy removal of single elements
+        Set<String> unusedParameterNames = new HashSet<>(parameterNames);
+        List<FileSystemAuditProvider.FileSystemAuditEvent> auditEvents = new ArrayList<>();
+
+        for (Map.Entry<String, List<MultipartFile>> entry : multipartRequest.getMultiFileMap().entrySet())
         {
-            Iterator<Map.Entry<String, List<MultipartFile>>> iter = multipartRequest.getMultiFileMap().entrySet().iterator();
-            Deque<FileLike> overflowFiles = new ArrayDeque<>();  // using a deque for easy removal of single elements
-            Set<String> unusedParameterNames = new HashSet<>(parameterNames);
-            while (iter.hasNext())
+            if (!(encodedParameterNames.containsKey(entry.getKey()) || parameterNames.contains(entry.getKey())))
+                continue;
+
+            boolean isAfterFirstFile = false;
+            for (MultipartFile multipartFile : entry.getValue())
             {
-                Map.Entry<String, List<MultipartFile>> entry = iter.next();
-                if (parameterNames.contains(entry.getKey()))
+                String fileName = getFileName(multipartFile);
+                if (!fileName.isEmpty() && !originalFileNames.add(fileName))
+                    throw new ExperimentException("The file '" + fileName + " ' was uploaded twice - all files must be unique");
+
+                if (multipartFile.isEmpty())
+                    continue;
+
+                FileLike dir = getFileTargetDir(context);
+                FileLike file = FileUtil.findUniqueFileName(fileName, dir);
+                multipartFile.transferTo(toFileForWrite(file));
+                if (!dir.toURI().getPath().contains(TEMP_DIR_NAME))
                 {
-                    List<MultipartFile> multipartFiles = entry.getValue();
-                    boolean isAfterFirstFile = false;
-                    for (MultipartFile multipartFile : multipartFiles)
-                    {
-                        String fileName = getFileName(multipartFile);
-                        if (!fileName.isEmpty() && !originalFileNames.add(fileName))
-                        {
-                            throw new ExperimentException("The file '" + fileName + " ' was uploaded twice - all files must be unique");
-                        }
-                        if (!multipartFile.isEmpty())
-                        {
-                            FileLike dir = getFileTargetDir(context);
-                            FileLike file = FileUtil.findUniqueFileName(fileName, dir);
-                            multipartFile.transferTo(toFileForWrite(file));
-                            if (!dir.toURI().getPath().contains(TEMP_DIR_NAME))
-                            {
-                                FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(context.getContainer(), allowMultiple ? "File field provided for assay import" : "Primary file provided for assay import");
-                                event.setProvidedFileName(fileName);
-                                event.setFile(file.getName());
-                                event.setDirectory(dir.toURI().getPath());
-                                AuditLogService.get().addEvent(context.getUser(), event);
-                            }
-                            if (!isAfterFirstFile)  // first file gets stored with multipartFile's name
-                            {
-                                files.put(multipartFile.getName(), file);
-                                isAfterFirstFile = true;
-                                unusedParameterNames.remove(multipartFile.getName());
-                            }
-                            else  // other files get stored in leftover keys later to store only one file per key (bit of a hack)
-                            {
-                                overflowFiles.add(file);
-                            }
-
-                            if (ensureExpData)
-                                AbstractQueryUpdateService.ensureExpData(context.getUser(), context.getContainer(), toFileForWrite(file));
-                        }
-                    }
+                    FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(context.getContainer(), allowMultiple ? "File field provided for assay import" : "Primary file provided for assay import");
+                    event.setProvidedFileName(fileName);
+                    event.setFile(file.getName());
+                    event.setDirectory(dir.toURI().getPath());
+                    auditEvents.add(event);
                 }
-            }
-            // now process overflow files, if any
-            for (String unusedParameterName : unusedParameterNames)
-            {
-                if (overflowFiles.isEmpty())
-                    break;  // we're done
-                else
+                if (!isAfterFirstFile)  // first file gets stored with multipartFile's name
                 {
-                    files.put(unusedParameterName, overflowFiles.remove());
+                    String name = multipartFile.getName();
+                    String param = encodedParameterNames.get(name);
+                    if (param == null && parameterNames.contains(name))
+                        param = name;
+                    if (param == null)
+                        throw new ExperimentException("No parameter name found for multipart file '" + name + "'");
+
+                    files.put(param, file);
+                    isAfterFirstFile = true;
+                    unusedParameterNames.remove(param);
                 }
+                else  // other files get stored in leftover keys later to store only one file per key (bit of a hack)
+                {
+                    overflowFiles.add(file);
+                }
+
+                if (ensureExpData)
+                    AbstractQueryUpdateService.ensureExpData(context.getUser(), context.getContainer(), toFileForWrite(file));
             }
+        }
+
+        if (!auditEvents.isEmpty())
+            AuditLogService.get().addEvents(context.getUser(), auditEvents);
 
-            if (!overflowFiles.isEmpty() && !allowMultiple)  // too many files; shouldn't happen, but if it does, throw an error
-                throw new ExperimentException("Tried to save too many files: number of keys is " + parameterNames.size() +
-                        ", but " + overflowFiles.size() + " extra file(s) were found.");
+        // now process overflow files, if any
+        for (String unusedParameterName : unusedParameterNames)
+        {
+            if (overflowFiles.isEmpty())
+                break;  // we're done
+
+            files.put(unusedParameterName, overflowFiles.remove());
         }
+
+        if (!overflowFiles.isEmpty() && !allowMultiple)  // too many files; shouldn't happen, but if it does, throw an error
+            throw new ExperimentException("Tried to save too many files: number of keys is " + parameterNames.size() +
+                    ", but " + overflowFiles.size() + " extra file(s) were found.");
+
         return files;
     }
 

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

@@ -48,8 +48,6 @@
 
 /**
  * Information about an assay run that has already been imported into the database.
- * User: jeckels
- * Date: Oct 7, 2011
  */
 public class AssayRunDatabaseContext<ProviderType extends AssayProvider> implements AssayRunUploadContext<ProviderType>
 {
@@ -195,14 +193,6 @@ public FileLike getOriginalFileLocation()
         return null;
     }
 
-    @NotNull
-    @Override
-    public Map<Object, String> getInputDatas()
-    {
-        // CONSIDER: get the run's input datas
-        return Collections.emptyMap();
-    }
-
     @Override
     public ProviderType getProvider()
     {

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

@@ -36,7 +36,6 @@
 import org.labkey.api.writer.ContainerUser;
 import org.labkey.vfs.FileLike;
 
-import java.io.File;
 import java.util.Map;
 import java.util.Set;
 
@@ -113,7 +112,10 @@ default DataIteratorBuilder getRawData()
      * NOTE: These files will not be parsed or imported by the assay's DataHandler -- use {@link #getUploadedData()} instead.
      */
     @NotNull
-    Map<?, String> getInputDatas();
+    default Map<?, String> getInputDatas()
+    {
+        return emptyMap();
+    }
 
     @NotNull
     default Map<?, String> getOutputDatas()
@@ -181,16 +183,19 @@ default ReImportOption getReImportOption()
 
     void uploadComplete(ExpRun run) throws ExperimentException;
 
+    @Nullable
     default String getJobDescription()
     {
         return null;
     }
 
+    @Nullable
     default String getJobNotificationProvider()
     {
         return null;
     }
 
+    @Nullable
     default String getPipelineJobGUID()
     {
         return null;