Issue 52827: File/attachment fields with special characters

api/src/org/labkey/api/data/TableViewForm.java

@@ -34,6 +34,8 @@
 import org.labkey.api.action.NullSafeBindException;
 import org.labkey.api.action.SpringActionController;
 import org.labkey.api.collections.CaseInsensitiveHashMap;
+import org.labkey.api.dataiterator.DataIteratorUtil;
+import org.labkey.api.query.QueryUpdateForm;
 import org.labkey.api.query.SchemaKey;
 import org.labkey.api.security.permissions.DeletePermission;
 import org.labkey.api.security.permissions.InsertPermission;
@@ -572,6 +574,26 @@ public Map<String,Object> getTypedColumns(boolean includeUntyped)
                 values.put(column.getName(), getTypedValue(column));
             else if (includeUntyped && contains(column))
                 values.put(column.getName(), get(column));
+            else if (column.getName().contains("\"") && getViewContext().getRequest() instanceof MultipartHttpServletRequest)
+            {
+                String quoteEncodedFieldName = getMultiPartFormFieldName(column);
+                boolean isFileColFileRemoved = false;
+                if (values.get(column.getName()) == null && File.class.equals(column.getJavaClass()))
+                {
+                    MultipartHttpServletRequest request = (MultipartHttpServletRequest) getRequest();
+                    MultipartFile f = request.getFile(quoteEncodedFieldName);
+                    isFileColFileRemoved = f != null && (f.getOriginalFilename() == null || f.getOriginalFilename().isEmpty());
+                }
+
+                if (isFileColFileRemoved)
+                    values.put(column.getName(), null);
+                else
+                {
+                    Object value = getTypedValues().get(quoteEncodedFieldName);
+                    if (value != null)
+                        values.put(column.getName(), value);
+                }
+            }
 
             // Check if there was a file uploaded for the column's value
             if (values.get(column.getName()) == null && File.class.equals(column.getJavaClass()) && getRequest() instanceof MultipartHttpServletRequest)
@@ -765,6 +787,12 @@ public String getFormFieldName(@NotNull ColumnInfo column)
         return column.getPropertyName();
     }
 
+    public String getMultiPartFormFieldName(@NotNull ColumnInfo column)
+    {
+        return getFormFieldName(column);
+    }
+
+
     @Nullable
     public ColumnInfo getColumnByFormFieldName(@NotNull String name)
     {

api/src/org/labkey/api/dataiterator/DataIteratorUtil.java

@@ -21,6 +21,7 @@
 import org.apache.commons.io.IOUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.junit.Assert;
 import org.junit.Test;
@@ -56,7 +57,6 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.Spliterator;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -141,7 +141,60 @@ public static Map<String,ColumnInfo> createTableMap(TableInfo target, boolean us
 
     // rank of a match of import column NAME matching various properties of target column
     // MatchType.low is used for matches based on something other than name
-    enum MatchType {propertyuri, name, alias, jdbcname, tsvColumn, low}
+    public enum MatchType
+    {
+        propertyuri,
+        name,
+        alias,
+        jdbcname,
+        tsvColumn,
+        multiPartFormData()
+                {
+                    @Override
+                    public String getMatchedName(@Nullable String name)
+                    {
+                        if (name == null)
+                            return null;
+                        // " is encoded as %22 when content-type is "multipart/form-data" (but is not otherwise encoded so decode() does not work)
+                        return name.replaceAll("\"", "%22");
+                    }
+
+                    @Override
+                    public boolean updateRowMap(@NotNull ColumnInfo col, Map<String, Object> rowMap)
+                    {
+                        if (col.getName().contains("\"") && File.class.equals(col.getJavaClass()))
+                        {
+                            // Issue 52827: File/attachment fields with special characters
+                            String quoteEncodedName = DataIteratorUtil.MatchType.multiPartFormData.getMatchedName(col.getName());
+                            if (rowMap.containsKey(quoteEncodedName))
+                            {
+                                rowMap.put(col.getName(), rowMap.get(quoteEncodedName));
+                                rowMap.remove(quoteEncodedName);
+                                return true;
+                            }
+                        }
+                        return false;
+                    }
+                },
+        low;
+
+        public String getMatchedName(@Nullable String name)
+        {
+            return name;
+        }
+
+        /**
+         * Update rowMap content based on passed in col.
+         * For example, the original rowMap may contain encoded field name. This util substitute the key in rowMap to reflect the actual col name
+         * @param col
+         * @param rowMap
+         * @return If rowMap has been updated
+         */
+        public boolean updateRowMap(@NotNull ColumnInfo col, Map<String, Object> rowMap)
+        {
+            return false;
+        }
+    }
 
 
     /**
@@ -158,6 +211,9 @@ protected static Map<String,Pair<ColumnInfo,MatchType>> _createTableMap(TableInf
 
         Map<String, Pair<ColumnInfo,MatchType>> targetAliasesMap = new CaseInsensitiveHashMap<>(cols.size()*4);
 
+        for (ColumnInfo col : cols)
+            targetAliasesMap.put(MatchType.multiPartFormData.getMatchedName(col.getName()), new Pair<>(col, MatchType.multiPartFormData));
+
         // should this be under the useImportAliases flag???
         for (ColumnInfo col : cols)
         {

api/src/org/labkey/api/exp/property/DomainUtil.java

@@ -38,6 +38,7 @@
 import org.labkey.api.data.SimpleFilter;
 import org.labkey.api.data.TableInfo;
 import org.labkey.api.data.TableSelector;
+import org.labkey.api.dataiterator.DataIteratorUtil;
 import org.labkey.api.defaults.DefaultValueService;
 import org.labkey.api.exp.ChangePropertyDescriptorException;
 import org.labkey.api.exp.DomainDescriptor;
@@ -1337,6 +1338,7 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N
                 : new CaseInsensitiveHashSet(updates.getReservedFieldNames());
         Set<String> reservedPrefixes = (null != domain && null != domainKind) ? domainKind.getReservedPropertyNamePrefixes() : updates.getReservedFieldNamePrefixes();
         Map<String, Integer> namePropertyIdMap = new CaseInsensitiveHashMap<>();
+        Map<String, String> altNameMap = new CaseInsensitiveHashMap<>();
         ValidationException exception = new ValidationException();
         Map<Integer, String> propertyIdNameMap = getOriginalFieldPropertyIdNameMap(orig);//key: orig property id, value : orig field name
 
@@ -1346,7 +1348,7 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N
 
             if (null == name || name.trim().isEmpty())
             {
-                exception.addError(new SimpleValidationError(getDomainErrorMessage(updates,"Please provide a name for each field.")));
+                exception.addError(new SimpleValidationError(getDomainErrorMessage(updates, "Please provide a name for each field.")));
                 continue;
             }
 
@@ -1374,7 +1376,7 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N
                 String origFieldName = (null != propertyIdNameMap ? propertyIdNameMap.get(field.getPropertyId()) : null);
                 if (field.getPropertyId() <= 0 || !name.equalsIgnoreCase(origFieldName))
                 {
-                    exception.addFieldError(name, getDomainErrorMessage(updates,("'" + name + "' is a reserved field name in '" + updates.getName() + "'.")));
+                    exception.addFieldError(name, getDomainErrorMessage(updates, ("'" + name + "' is a reserved field name in '" + updates.getName() + "'.")));
                 }
                 continue;
             }
@@ -1391,18 +1393,32 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N
 
             if (namePropertyIdMap.containsKey(name))
             {
-                String errorMsg = getDomainErrorMessage(updates,"The field name '" + name + "' is already taken. Please provide a unique name for each field.");
+                String errorMsg = getDomainErrorMessage(updates, "The field name '" + name + "' is already taken. Please provide a unique name for each field.");
                 PropertyValidationError propertyValidationError = new PropertyValidationError(errorMsg, name, field.getPropertyId());
                 exception.addError(propertyValidationError);
             }
             else
             {
                 namePropertyIdMap.put(name, field.getPropertyId());
+
+                // Issue 52827: File/attachment fields with special characters
+                if (altNameMap.containsKey(name))
+                {
+                    String errorMsg = getDomainErrorMessage(updates, "The field name '" + name + "' cannot be used with another field '" + altNameMap.get(name) + "'. Please provide a different name for the field.");
+                    PropertyValidationError propertyValidationError = new PropertyValidationError(errorMsg, name, field.getPropertyId());
+                    exception.addError(propertyValidationError);
+                }
+                else
+                {
+                    altNameMap.put(name, name);
+                    altNameMap.put(DataIteratorUtil.MatchType.multiPartFormData.getMatchedName(name), name);
+                    altNameMap.put(name.replaceAll("%22", "\""), name);
+                }
             }
 
             if (field.getValueExpression() != null && field.getValueExpression().trim().length() > 4000)
             {
-                exception.addFieldError(name, getDomainErrorMessage(updates,"The value expression for '" + name
+                exception.addFieldError(name, getDomainErrorMessage(updates, "The value expression for '" + name
                         + "' is too long (" + field.getValueExpression().trim().length() + " characters). Please limit to 4000 characters."));
             }
         }

api/src/org/labkey/api/query/QueryUpdateForm.java

@@ -21,6 +21,7 @@
 import org.labkey.api.data.ColumnInfo;
 import org.labkey.api.data.TableInfo;
 import org.labkey.api.data.TableViewForm;
+import org.labkey.api.dataiterator.DataIteratorUtil;
 import org.labkey.api.view.ViewContext;
 import org.springframework.validation.BindException;
 
@@ -83,4 +84,10 @@ public String getFormFieldName(@NotNull ColumnInfo column)
     {
         return (_ignorePrefix ? "" : PREFIX) + column.getName();
     }
+
+    @Override
+    public String getMultiPartFormFieldName(@NotNull ColumnInfo column)
+    {
+        return (_ignorePrefix ? "" : PREFIX) + DataIteratorUtil.MatchType.multiPartFormData.getMatchedName(column.getName());
+    }
 }

assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java

@@ -229,6 +229,9 @@ private List<Map<String, Object>> resolveRows(
                 ColumnInfo col = columnInfoMap.get(entry.getKey());
                 if (col != null && !row.containsKey(entry.getKey()))
                 {
+                    if (DataIteratorUtil.MatchType.multiPartFormData.updateRowMap(col, row))
+                        continue;
+
                     // use column names for existing row values
                     row.put(col.getName(), entry.getValue());
                 }

query/src/org/labkey/query/controllers/QueryController.java

@@ -67,6 +67,7 @@
 import org.labkey.api.data.dialect.SqlDialect;
 import org.labkey.api.dataiterator.DataIteratorBuilder;
 import org.labkey.api.dataiterator.DataIteratorContext;
+import org.labkey.api.dataiterator.DataIteratorUtil;
 import org.labkey.api.dataiterator.DetailedAuditLogDataIterator;
 import org.labkey.api.dataiterator.ListofMapsDataIterator;
 import org.labkey.api.exceptions.OptimisticConflictException;
@@ -4491,7 +4492,7 @@ protected JSONObject executeJson(JSONObject json, CommandType commandType, boole
                     // Use shallow copy since jsonObj.toMap() will translate contained JSONObjects into Maps, which we don't want
                     JsonUtil.fillMapShallow(jsonObj, rowMap);
                     if (allowRowAttachments())
-                        addRowAttachments(rowMap, idx, commandIndex);
+                        addRowAttachments(table, rowMap, idx, commandIndex);
 
                     rowsToProcess.add(rowMap);
                     rowsAffected++;
@@ -4643,34 +4644,42 @@ protected boolean allowRowAttachments()
             return false;
         }
 
-        private void addRowAttachments(Map<String, Object> rowMap, int rowIndex, @Nullable Integer commandIndex)
+        private void addRowAttachments(TableInfo tableInfo, Map<String, Object> rowMap, int rowIndex, @Nullable Integer commandIndex)
         {
             if (getFileMap() != null)
             {
                 for (Map.Entry<String, MultipartFile> fileEntry : getFileMap().entrySet())
                 {
                     // Allow for the fileMap key to include the row index, and optionally command index, for defining
                     // which row to attach this file to
-                    String fieldKey = fileEntry.getKey();
-                    int delimIndex = fieldKey.lastIndexOf(ROW_ATTACHMENT_INDEX_DELIM);
-                    if (delimIndex > -1)
+                    String fullKey = fileEntry.getKey();
+                    String fieldKey = fullKey;
+                    // Issue 52827: Cannot attach a file if the field name contains ::
+                    // use lastIndexOf instead of split to get the proper parts
+                    int lastDelimIndex = fullKey.lastIndexOf(ROW_ATTACHMENT_INDEX_DELIM);
+                    if (lastDelimIndex > -1)
                     {
-                        String[] parts = fileEntry.getKey().split(ROW_ATTACHMENT_INDEX_DELIM);
+                        String fieldKeyExcludeIndex = fullKey.substring(0, lastDelimIndex);
+                        String fieldRowIndex = fullKey.substring(lastDelimIndex + ROW_ATTACHMENT_INDEX_DELIM.length());
+                        if (!fieldRowIndex.equals(rowIndex+"")) continue;
 
                         if (commandIndex == null)
                         {
                             // Single command, so we're parsing file names in the format of: FileField::0
-                            fieldKey = parts[0];
-                            String fieldRowIndex = parts[1];
-                            if (!fieldRowIndex.equals(rowIndex+"")) continue;
+                            fieldKey = fieldKeyExcludeIndex;
                         }
                         else
                         {
                             // Multi-command, so we're parsing file names in the format of: FileField::0::1
-                            fieldKey = parts[0];
-                            String fieldCommandIndex = parts[1];
-                            String fieldRowIndex = parts[2];
-                            if (!fieldCommandIndex.equals(commandIndex+"") || !fieldRowIndex.equals(rowIndex+""))
+                            int subDelimIndex = fieldKeyExcludeIndex.lastIndexOf(ROW_ATTACHMENT_INDEX_DELIM);
+                            if (subDelimIndex > -1)
+                            {
+                                fieldKey = fieldKeyExcludeIndex.substring(0, subDelimIndex);
+                                String fieldCommandIndex = fieldKeyExcludeIndex.substring(subDelimIndex + ROW_ATTACHMENT_INDEX_DELIM.length());
+                                if (!fieldCommandIndex.equals(commandIndex+""))
+                                    continue;
+                            }
+                            else
                                 continue;
                         }
                     }
@@ -4679,6 +4688,9 @@ private void addRowAttachments(Map<String, Object> rowMap, int rowIndex, @Nullab
                     rowMap.put(fieldKey, file.isEmpty() ? null : file);
                 }
             }
+
+            for (ColumnInfo col : tableInfo.getColumns())
+                DataIteratorUtil.MatchType.multiPartFormData.updateRowMap(col, rowMap);
         }
 
         protected boolean isSuccessOnValidationError()