Merge remote-tracking branch 'origin/develop' into fb_mcp_mvp

Author: labkey-adam

api/src/org/labkey/api/action/AbstractFileUploadAction.java

@@ -15,7 +15,6 @@
  */
 package org.labkey.api.action;
 
-import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import org.jetbrains.annotations.NotNull;
 import org.labkey.api.util.ExceptionUtil;
@@ -42,7 +41,6 @@
 import java.io.Writer;
 import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.Map;
 
 /**
@@ -192,19 +190,16 @@ private void export(FORM form, HttpServletResponse response) throws Exception
                 return;
             }
 
-            HttpServletRequest basicRequest = getViewContext().getRequest();
-
             // Parameter name (String) -> File on disk/original file name Pair
             Map<String, Pair<FileLike, String>> savedFiles = new HashMap<>();
 
-            if (basicRequest instanceof MultipartHttpServletRequest request)
+            if (getViewContext().getRequest() instanceof MultipartHttpServletRequest)
             {
-
-                Iterator<String> nameIterator = request.getFileNames();
-                while (nameIterator.hasNext())
+                Map<String, MultipartFile> fileMap = getFileMap();
+                for (var e : fileMap.entrySet())
                 {
-                    String formElementName = nameIterator.next();
-                    MultipartFile file = request.getFile(formElementName);
+                    var formElementName = e.getKey();
+                    var file = e.getValue();
                     String filename = file.getOriginalFilename();
 
                     try (InputStream input = file.getInputStream())

api/src/org/labkey/api/action/BaseViewAction.java

@@ -76,6 +76,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Predicate;
@@ -185,51 +186,26 @@ public static PropertyValues getPropertyValuesForFormBinding(PropertyValues pvs,
         return ret;
     }
 
-    static final String FORM_DATE_ENCODED_PARAM = "formDataEncoded";
 
-    /**
-     * When a double quote is encountered in a multipart/form-data context, it is encoded as %22 using URL-encoding by browsers.
-     * This process replaces the double quote with its hexadecimal equivalent in a URL-safe format, preventing it from being misinterpreted as the end of a value or a boundary.
-     * The consequence of such encoding is we can't distinguish '"' from the actual '%22' in parameter name.
-     * As a workaround, a client-side util `encodeFormDataQuote` is used to convert %22 to %2522 and " to %22 explicitly, while passing in an additional param formDataEncoded=true.
-     * This class converts those encoded param names back to its decoded form during PropertyValues binding.
-     * See Issue 52827, 52925 and 52119 for more information.
-     */
+    /// Some characters can be mishandled by the browser in multipart/formdata requests (e.g. doublequote and backslask).
+    /// We support an encoding from fields to avoid these characters, see {@link PageFlowUtil#encodeFormName} and {@link PageFlowUtil#decodeFormName}.
     static public class ViewActionParameterPropertyValues extends ServletRequestParameterPropertyValues
     {
-
         public ViewActionParameterPropertyValues(ServletRequest request) {
             this(request, null, null);
         }
 
         public ViewActionParameterPropertyValues(ServletRequest request, @Nullable String prefix, @Nullable String prefixSeparator)
         {
             super(request, prefix, prefixSeparator);
-            if (isFormDataEncoded())
-            {
-                for (int i = 0; i < getPropertyValues().length; i++)
-                {
-                    PropertyValue formDataPropValue = getPropertyValues()[i];
-                    String propValueName = formDataPropValue.getName();
-                    String decoded = PageFlowUtil.decodeQuoteEncodedFormDataKey(propValueName);
-                    if (!propValueName.equals(decoded))
-                        setPropertyValueAt(new PropertyValue(decoded, formDataPropValue.getValue()), i);
-                }
-            }
-        }
-
-        private boolean isFormDataEncoded()
-        {
-            PropertyValue formDataPropValue = getPropertyValue(FORM_DATE_ENCODED_PARAM);
-            if (formDataPropValue != null)
+            for (int i = 0; i < getPropertyValues().length; i++)
             {
-                Object v = formDataPropValue.getValue();
-                String formDataPropValueStr = v == null ? null : String.valueOf(v);
-                if (StringUtils.isNotBlank(formDataPropValueStr))
-                    return (Boolean) ConvertUtils.convert(formDataPropValueStr, Boolean.class);
+                PropertyValue formDataPropValue = getPropertyValues()[i];
+                String propValueName = formDataPropValue.getName();
+                String decoded = PageFlowUtil.decodeFormName(propValueName);
+                if (!propValueName.equals(decoded))
+                    setPropertyValueAt(new PropertyValue(decoded, formDataPropValue.getValue()), i);
             }
-
-            return false;
         }
     }
 
@@ -725,9 +701,7 @@ public <T> T convertIfNecessary(Object value, Class<T> requiredType, MethodParam
      */
     protected Map<String, MultipartFile> getFileMap()
     {
-        if (getViewContext().getRequest() instanceof MultipartHttpServletRequest)
-            return ((MultipartHttpServletRequest)getViewContext().getRequest()).getFileMap();
-        return Collections.emptyMap();
+        return PageFlowUtil.getFileMap(getViewContext().getRequest());
     }
 
     protected List<AttachmentFile> getAttachmentFileList()

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

@@ -31,6 +31,7 @@
 import org.labkey.api.query.AbstractQueryUpdateService;
 import org.labkey.api.util.FileUtil;
 import org.labkey.api.util.NetworkDrive;
+import org.labkey.api.util.PageFlowUtil;
 import org.labkey.api.view.ViewContext;
 import org.labkey.vfs.FileLike;
 import org.springframework.web.multipart.MultipartFile;
@@ -233,7 +234,7 @@ public Map<String, FileLike> savePostedFiles(ContextType context, @NotNull Set<S
         Set<String> originalFileNames = new HashSet<>();
         if (context.getRequest() instanceof MultipartHttpServletRequest multipartRequest)
         {
-            Iterator<Map.Entry<String, List<MultipartFile>>> iter = multipartRequest.getMultiFileMap().entrySet().iterator();
+            Iterator<Map.Entry<String, List<MultipartFile>>> iter = PageFlowUtil.getMultiFileMap(context.getRequest()).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())

api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java

@@ -23,6 +23,7 @@
 import org.apache.logging.log4j.Logger;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.labkey.api.action.BaseViewAction;
 import org.labkey.api.assay.AbstractAssayProvider;
 import org.labkey.api.assay.AssayDataCollector;
 import org.labkey.api.assay.AssayFileWriter;
@@ -61,6 +62,7 @@
 import org.labkey.api.study.assay.ParticipantVisitResolverType;
 import org.labkey.api.util.FileUtil;
 import org.labkey.api.util.GUID;
+import org.labkey.api.util.PageFlowUtil;
 import org.labkey.api.view.ActionURL;
 import org.labkey.api.view.NotFoundException;
 import org.labkey.api.view.UnauthorizedException;
@@ -360,10 +362,11 @@ public Map<DomainProperty, FileLike> getAdditionalPostedFiles(List<? extends Dom
             File assayDirectory = getAssayDirectory(getContainer(), null);
 
             // Hidden values in form containing previously uploaded files if the previous upload resulted in error
+            var fileMap = PageFlowUtil.getFileMap(request);
             for (String fileParam : filePdNames)
             {
                 DomainProperty domainProperty = fileParameters.get(fileParam);
-                MultipartFile multiFile = request.getFileMap().get(fileParam);
+                MultipartFile multiFile = fileMap.get(fileParam);
 
                 // If the file is removed from form after error, override hidden file name with an empty file
                 if (null != multiFile && multiFile.getOriginalFilename().isEmpty())

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

@@ -553,24 +553,21 @@ public CaseInsensitiveHashMap<Object> getTypedColumns(boolean includeUntyped)
 
         for (ColumnInfo column : getTable().getColumns())
         {
+            var fieldName = getFormFieldName(column);
+
             if (hasTypedValue(column))
             {
                 values.put(column.getName(), getTypedValue(column));
             }
-            else if (includeUntyped && _stringValues.containsKey(getFormFieldName(column)))
+            else if (includeUntyped && _stringValues.containsKey(fieldName))
             {
-                values.put(column.getName(), _stringValues.get(getFormFieldName(column)));
+                values.put(column.getName(), _stringValues.get(fieldName));
             }
             else if (getRequest() instanceof MultipartHttpServletRequest request)
             {
-                String fieldName = getMultiPartFormFieldName(column);
-                Object typedValue = _getTypedValues().get(fieldName);
-
-                if (typedValue != null)
-                    values.put(column.getName(), typedValue);
-                else if (File.class.equals(column.getJavaClass()))
+                if (File.class.equals(column.getJavaClass()))
                 {
-                    MultipartFile file = request.getFile(fieldName);
+                    MultipartFile file = PageFlowUtil.getFileMap(request).get(fieldName);
                     if (file != null)
                     {
                         // Check if the file was removed
@@ -587,10 +584,11 @@ else if (File.class.equals(column.getJavaClass()))
                 ColumnInfo mvColumn = getTable().getColumn(column.getMvColumnName());
                 if (null != mvColumn)
                 {
+                    var mvFieldName = getFormFieldName(mvColumn);
                     if (hasTypedValue(mvColumn))
                         values.put(mvColumn.getName(), getTypedValue(mvColumn));
-                    else if (includeUntyped && _stringValues.containsKey(getFormFieldName(mvColumn)))
-                        values.put(mvColumn.getName(), _stringValues.get(getFormFieldName(mvColumn)));
+                    else if (includeUntyped && _stringValues.containsKey(mvFieldName))
+                        values.put(mvColumn.getName(), _stringValues.get(mvFieldName));
                 }
             }
         }
@@ -739,11 +737,6 @@ public String getFormFieldName(@NotNull ColumnInfo column)
         return column.getName();
     }
 
-    public String getMultiPartFormFieldName(@NotNull ColumnInfo column)
-    {
-        return getFormFieldName(column);
-    }
-
     @Nullable
     public ColumnInfo getColumnByFormFieldName(@NotNull String name)
     {

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

@@ -141,57 +141,14 @@ 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
-    public enum MatchType
+    private 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
-         * @return If rowMap has been updated
-         */
-        public boolean updateRowMap(@NotNull ColumnInfo col, Map<String, Object> rowMap)
-        {
-            return false;
-        }
+        low
     }
 
 
@@ -209,9 +166,6 @@ 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/dataiterator/StandardDataIteratorBuilder.java

@@ -25,7 +25,6 @@
 import org.labkey.api.data.TableInfo;
 import org.labkey.api.data.validator.ColumnValidator;
 import org.labkey.api.data.validator.ColumnValidators;
-import org.labkey.api.data.validator.RequiredValidator;
 import org.labkey.api.exp.PropertyDescriptor;
 import org.labkey.api.exp.PropertyType;
 import org.labkey.api.exp.api.ExperimentService;

api/src/org/labkey/api/defaults/SetDefaultValuesAction.java

@@ -448,7 +448,7 @@ public boolean handlePost(FormType domainIdForm, BindException errors) throws Ex
      */
     protected String encodePropertyValues(FormType domainIdForm, String propName) throws IOException
     {
-        return domainIdForm.getRequest().getParameter(propName);
+        return (String)getProperty(propName);
     }
 
     @Override

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

@@ -1587,8 +1587,6 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N
                 else
                 {
                     altNameMap.put(name, name);
-                    altNameMap.put(DataIteratorUtil.MatchType.multiPartFormData.getMatchedName(name), name);
-                    altNameMap.put(name.replaceAll("%22", "\""), name);
                 }
             }
 

api/src/org/labkey/api/jsp/JspBase.java

@@ -211,6 +211,11 @@ public static HtmlString h(URLHelper url)
         return h(url == null ? null : url.toString());
     }
 
+    public static HtmlString hname(String name)
+    {
+        return HtmlString.of(PageFlowUtil.encodeFormName(name));
+    }
+
     // Note: If you have a stream, use LabKeyCollectors.toJsonArray()
     public static JSONArray toJsonArray(Collection<?> c)
     {