Issue 53498: reject string attachment values for query insert/update api

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

@@ -80,14 +80,15 @@ public class AttachmentDataIterator extends WrapperDataIterator
     }
 
     @Override
-    public boolean next()
+    public boolean next() throws BatchValidationException
     {
+        boolean ret = super.next();
+        if (!ret)
+            return false;
+
         ArrayList<AttachmentFile> attachmentFiles = null;
         try
         {
-            boolean ret = super.next();
-            if (!ret)
-                return false;
             for (_AttachmentUploadHelper p : attachmentColumns)
             {
                 Object attachmentValue = get(p.index);

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

@@ -1881,9 +1881,8 @@ else if (file instanceof FileLike fl)
                     value = null;
                 }
             }
-            else if (value instanceof String filePath)
-                return ExpDataFileConverter.convert(filePath);
-            return value;
+
+            return ExpDataFileConverter.convert(value);
         }
     }
 

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

@@ -716,8 +716,8 @@ protected Map<String, Object> coerceTypes(Map<String, Object> row)
             {
                 try
                 {
-                    if (PropertyType.FILE_LINK.equals(col.getPropertyType()) && value instanceof String strVal)
-                        value = ExpDataFileConverter.convert(strVal);
+                    if (PropertyType.FILE_LINK.equals(col.getPropertyType()))
+                        value = ExpDataFileConverter.convert(value);
                     else
                         value = ConvertUtils.convert(value.toString(), col.getJavaObjectClass());
                 }
@@ -1022,6 +1022,9 @@ public static Object saveFile(User user, Container container, String name, Objec
      */
     public static Object saveFile(User user, Container container, String name, Object value, @Nullable FileLike dirPath) throws ValidationException, QueryUpdateServiceException
     {
+        if (!(value instanceof MultipartFile) && !(value instanceof SpringAttachmentFile))
+            throw new ValidationException("Invalid file value");
+
         String auditMessageFormat = "Saved file '%s' for field '%s' in folder %s.";
         FileLike file = null;
         try
@@ -1042,8 +1045,9 @@ public static Object saveFile(User user, Container container, String name, Objec
                 event.setComment(String.format(auditMessageFormat, multipartFile.getOriginalFilename(), name, container.getPath()));
                 event.setProvidedFileName(multipartFile.getOriginalFilename());
             }
-            else if (value instanceof SpringAttachmentFile saf)
+            else
             {
+                SpringAttachmentFile saf = (SpringAttachmentFile) value;
                 file = FileUtil.findUniqueFileName(saf.getFilename(), dir);
                 checkFileUnderRoot(container, file);
                 saf.saveTo(file);
@@ -1060,9 +1064,6 @@ else if (value instanceof SpringAttachmentFile saf)
             throw new QueryUpdateServiceException(e);
         }
 
-        if (file == null)
-            return value;
-
         ensureExpData(user, container, file.toNioPathForRead().toFile());
         return file;
     }

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

@@ -26,6 +26,7 @@
 import org.labkey.api.collections.CaseInsensitiveMapWrapper;
 import org.labkey.api.data.ColumnInfo;
 import org.labkey.api.data.Container;
+import org.labkey.api.data.ConvertHelper;
 import org.labkey.api.data.ExpDataFileConverter;
 import org.labkey.api.data.JdbcType;
 import org.labkey.api.data.MvUtil;
@@ -855,6 +856,10 @@ protected Object convertColumnValue(ColumnInfo col, Object value, User user, Con
                     return ConvertUtils.convert(value.toString(), col.getJdbcType().getJavaClass());
             }
         }
+        catch (ConvertHelper.FileConversionException e)
+        {
+            throw new ValidationException(e.getMessage());
+        }
         catch (ConversionException e)
         {
             String type = ColumnInfo.getFriendlyTypeName(col.getJdbcType().getJavaClass());

experiment/src/org/labkey/experiment/ExpDataIterators.java

@@ -2115,9 +2115,7 @@ public static class FileLinkDataIterator extends WrapperDataIterator
                             }
                         }
 
-                        if (value instanceof String filePath)
-                            return ExpDataFileConverter.convert(filePath);
-                        return value;
+                        return ExpDataFileConverter.convert(value);
                     };
                 }
             }

experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java

@@ -1315,20 +1315,33 @@ protected Map<String, Object> _update(User user, Container c, Map<String, Object
             // Replace attachment columns with filename and keep AttachmentFiles
             Map<String, Object> rowStripped = new CaseInsensitiveHashMap<>();
             Map<String, Object> attachments = new CaseInsensitiveHashMap<>();
-            row.forEach((name, value) -> {
-                if (isAttachmentProperty(name) && value instanceof AttachmentFile file)
+            for (Map.Entry<String, Object> entry : row.entrySet())
+            {
+                String name = entry.getKey();
+                Object value = entry.getValue();
+                if (isAttachmentProperty(name))
                 {
-                    if (null != file.getFilename())
+                    if (value instanceof AttachmentFile file)
+                    {
+                        if (null != file.getFilename())
+                        {
+                            rowStripped.put(name, file.getFilename());
+                            attachments.put(name, value);
+                        }
+                    }
+                    else if (value != null && !StringUtils.isEmpty(String.valueOf(value)))
                     {
-                        rowStripped.put(name, file.getFilename());
-                        attachments.put(name, value);
+                        // Issue 53498: string value for attachment field is not allowed
+                        throw new ValidationException("Can't upload '" + value + "' to field " + name + " with type Attachment.");
                     }
+                    else
+                        rowStripped.put(name, value); // if null or empty, remove attachment
                 }
                 else
                 {
                     rowStripped.put(name, value);
                 }
-            });
+            }
 
             for (String vocabularyDomainName : getVocabularyDomainProviders().keySet())
             {

list/src/org/labkey/list/model/ListQueryUpdateService.java

@@ -15,6 +15,7 @@
  */
 package org.labkey.list.model;
 
+import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.labkey.api.announcements.DiscussionService;
@@ -343,6 +344,10 @@ protected Map<String, Object> updateRow(User user, Container container, Map<Stri
                         if (null != file.getFilename())
                             attachmentFiles.add(file);
                     }
+                    else if (r.getValue() != null && !StringUtils.isEmpty(String.valueOf(r.getValue())))
+                    {
+                        throw new ValidationException("Can't upload '" + r.getValue() + "' to field " + r.getKey() + " with type Attachment.");
+                    }
                 }
             }
         }