Merge remote-tracking branch 'origin/develop' into 26.3_fb_blank_sql

Author: labkey-matthewb

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/admin/AdminUrls.java

@@ -68,6 +68,7 @@ public interface AdminUrls extends UrlProvider
     ActionURL getCspReportToURL();
 
     ActionURL getAllowedExternalRedirectHostsURL();
+    ActionURL getDeleteEncryptedContentURL();
 
     /**
      * Simply adds an "Admin Console" link to nav trail if invoked in the root container. Otherwise, root is unchanged.

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

@@ -867,6 +867,7 @@ else if (entry.getKey().equalsIgnoreCase(ProvenanceService.PROVENANCE_INPUT_PROP
                     if (PropertyType.MULTI_CHOICE == pd.getPropertyType())
                     {
                         o = MultiChoice.Converter.getInstance().convert(MultiChoice.Array.class, o);
+                        map.put(pd.getName(), o);
                     }
                     else if (o instanceof String)
                     {

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/attachments/AttachmentService.java

@@ -140,6 +140,10 @@ static AttachmentService get()
 
     HttpView<?> getFindAttachmentParentsView();
 
+    void logOrphanedAttachments();
+
+    void deleteOrphanedAttachments();
+
     class DuplicateFilenameException extends IOException implements SkipMothershipLogging
     {
         private final List<String> _errors = new ArrayList<>();

api/src/org/labkey/api/audit/AuditHandler.java

@@ -91,8 +91,6 @@ static Pair<Map<String, Object>, Map<String, Object>> getOldAndNewRecordForMerge
             if (col != null && (col.isMultiValued() || col.getFk() instanceof MultiValuedForeignKey))
                 isMultiValued = true;
 
-            boolean isMultiChoice = col != null && col.getPropertyType() == PropertyType.MULTI_CHOICE;
-
             String nameFromAlias = key;
             if (null != col)
                 nameFromAlias = col.getName();
@@ -104,9 +102,13 @@ static Pair<Map<String, Object>, Map<String, Object>> getOldAndNewRecordForMerge
                 {
                     if (aliasColumn.getFk() != null && (aliasColumn.isMultiValued() || aliasColumn.getFk() instanceof MultiValuedForeignKey))
                         isMultiValued = true;
+                    col = aliasColumn; // GitHub Issue 913: Updating a sample details page shows an update to the MVTC field
                     nameFromAlias = aliasColumn.getName();
                 }
             }
+
+            boolean isMultiChoice = col != null && col.getPropertyType() == PropertyType.MULTI_CHOICE;
+
             String lcName = nameFromAlias.toLowerCase();
             // Preserve casing of inputs so we can show the names properly
             boolean isExpInput = false; // TODO: extract lineage handling out of this generic method

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

@@ -1003,6 +1003,10 @@ public Pair<SQLFragment, SQLFragment> getSqlFragments(Map<FieldKey, ? extends Co
                 return null;
 
             ColumnInfo colInfo = columnMap != null ? columnMap.get(_fieldKey) : null;
+
+            if (colInfo != null && colInfo.getJdbcType() != JdbcType.ARRAY)
+                throw new RuntimeSQLException(new SQLGenerationException("Invalid filter type for column '" + _fieldKey.toDisplayString() + "'."));
+
             var alias = SimpleFilter.getAliasForColumnFilter(dialect, colInfo, _fieldKey);
 
             SQLFragment valuesFragment = dialect.array_construct(paramValues);
@@ -1040,6 +1044,10 @@ public ArrayIsEmptyClause(@NotNull FieldKey fieldKey)
         public SQLFragment toSQLFragment(Map<FieldKey, ? extends ColumnInfo> columnMap, SqlDialect dialect)
         {
             ColumnInfo colInfo = columnMap != null ? columnMap.get(_fieldKey) : null;
+
+            if (colInfo != null && colInfo.getJdbcType() != JdbcType.ARRAY)
+                throw new RuntimeSQLException(new SQLGenerationException("Invalid filter type for column '" + _fieldKey.toDisplayString() + "'."));
+
             var alias = SimpleFilter.getAliasForColumnFilter(dialect, colInfo, _fieldKey);
 
             SQLFragment columnFragment = new SQLFragment().appendIdentifier(alias);
@@ -1426,30 +1434,30 @@ protected static void parseParams(String value, String separator, Collection<Str
         // check for JSON marker
         if (value.startsWith(JSON_MARKER_START) && value.endsWith(JSON_MARKER_END))
         {
-            value = value.substring(JSON_MARKER_START.length(), value.length()-JSON_MARKER_END.length());
-            if (value.startsWith("[") && value.endsWith("]"))
+            String cleanedValue = value.substring(JSON_MARKER_START.length(), value.length()-JSON_MARKER_END.length());
+            if (cleanedValue.startsWith("[") && cleanedValue.endsWith("]"))
             {
                 try
                 {
-                    // TODO what do we do with malformed parameters???
-                    JSONArray array = new JSONArray(value);
+                    JSONArray array = new JSONArray(cleanedValue);
                     for (int i = 0; i < array.length(); i++)
                     {
                         Object jsonVal = array.get(i);
                         collection.add(JSONObject.NULL.equals(jsonVal) ? null : Objects.toString(jsonVal));
                     }
+                    return;
                 }
                 catch (JSONException ex)
                 {
-                    // pass
+                    // GH Issue #948
+                    // Intentionally do nothing, so we revert to parsing by regex. Otherwise, valid filters like an IN
+                    // filter on "{json:123;abc}" will be skipped instead of parsed as ["{json:123", "abc}"]
                 }
             }
         }
-        else
-        {
-            String[] st = value.split("\\s*" + separator + "\\s*", -1);
-            Collections.addAll(collection, st);
-        }
+
+        String[] st = value.split("\\s*" + separator + "\\s*", -1);
+        Collections.addAll(collection, st);
     }
 
     protected static Set<String> parseParams(Object value_, String separator)
@@ -1747,6 +1755,9 @@ protected String toURLParamValue()
         public SQLFragment toSQLFragment(Map<FieldKey, ? extends ColumnInfo> columnMap, SqlDialect dialect)
         {
             ColumnInfo colInfo = columnMap != null ? columnMap.get(_fieldKey) : null;
+            if (colInfo != null && colInfo.getJdbcType() == JdbcType.ARRAY)
+                throw new RuntimeSQLException(new SQLGenerationException("Invalid filter type for column '" + _fieldKey.toDisplayString() + "'."));
+
             var alias = SimpleFilter.getAliasForColumnFilter(dialect, colInfo, _fieldKey);
 
             SQLFragment fragment = toWhereClause(dialect, alias);