From 407eff3a81bd46695676aba06637ce944dd83b5e Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 13 Aug 2025 17:22:26 -0700 Subject: [PATCH 1/3] Issue 53498: reject string attachment values for query insert/update api --- .../api/ExpDataClassDataTableImpl.java | 27 ++++++++++++++----- .../list/model/ListQueryUpdateService.java | 6 +++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 851cd1fa95c..5b0fa46c855 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1315,20 +1315,35 @@ protected Map _update(User user, Container c, Map rowStripped = new CaseInsensitiveHashMap<>(); Map attachments = new CaseInsensitiveHashMap<>(); - row.forEach((name, value) -> { - if (isAttachmentProperty(name) && value instanceof AttachmentFile file) + for (Map.Entry 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 instanceof String strVal) { - rowStripped.put(name, file.getFilename()); - attachments.put(name, value); + if (!StringUtils.isEmpty(strVal)) // Issue 53498: string value for attachment field is not allowed + throw new ValidationException("Can't upload '" + strVal + "' to field " + name + " with type Attachment."); + else + rowStripped.put(name, value); // if blank, remove attachment } + else + rowStripped.put(name, value); // if null, remove attachment } else { rowStripped.put(name, value); } - }); + } for (String vocabularyDomainName : getVocabularyDomainProviders().keySet()) { diff --git a/list/src/org/labkey/list/model/ListQueryUpdateService.java b/list/src/org/labkey/list/model/ListQueryUpdateService.java index f9ae4ae97df..6f6da51862e 100644 --- a/list/src/org/labkey/list/model/ListQueryUpdateService.java +++ b/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,11 @@ protected Map updateRow(User user, Container container, Map Date: Fri, 15 Aug 2025 12:46:19 -0700 Subject: [PATCH 2/3] clean --- .../org/labkey/experiment/api/ExpDataClassDataTableImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 5b0fa46c855..eb8dda494d7 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1323,7 +1323,7 @@ protected Map _update(User user, Container c, Map Date: Sun, 17 Aug 2025 16:51:08 -0700 Subject: [PATCH 3/3] Non string type file path / attachment value --- .../api/dataiterator/AttachmentDataIterator.java | 9 +++++---- .../labkey/api/dataiterator/SimpleTranslator.java | 5 ++--- .../api/query/AbstractQueryUpdateService.java | 13 +++++++------ .../labkey/api/query/DefaultQueryUpdateService.java | 5 +++++ .../src/org/labkey/experiment/ExpDataIterators.java | 4 +--- .../experiment/api/ExpDataClassDataTableImpl.java | 10 ++++------ .../labkey/list/model/ListQueryUpdateService.java | 5 ++--- 7 files changed, 26 insertions(+), 25 deletions(-) diff --git a/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java b/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java index c8caccc0d6c..8b63482792c 100644 --- a/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java +++ b/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 attachmentFiles = null; try { - boolean ret = super.next(); - if (!ret) - return false; for (_AttachmentUploadHelper p : attachmentColumns) { Object attachmentValue = get(p.index); diff --git a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java index b000c828be7..31a60b74b31 100644 --- a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java +++ b/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); } } diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 872719b2918..94c1c726394 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -716,8 +716,8 @@ protected Map coerceTypes(Map 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; } diff --git a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java index 4d4c06109a5..d053b8a6a73 100644 --- a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java +++ b/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()); diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index cee2a9eeb10..d63750e9b50 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/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); }; } } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 7a8027b428f..d9f272dc088 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1329,15 +1329,13 @@ protected Map _update(User user, Container c, Map updateRow(User user, Container container, Map