Issue 53027: Cannot insert a row into a sample type if Allowed File Extensions set, no file is provided

api/src/org/labkey/api/premium/DefaultAVMultipartResolver.java

@@ -3,8 +3,11 @@
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.Part;
+import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.NotNull;
+import org.labkey.api.action.ApiUsageException;
 import org.labkey.api.util.FileUtil;
+import org.labkey.api.view.UnauthorizedException;
 import org.springframework.web.multipart.MultipartException;
 import org.springframework.web.multipart.MultipartHttpServletRequest;
 import org.springframework.web.multipart.support.StandardMultipartHttpServletRequest;
@@ -22,10 +25,17 @@ public class DefaultAVMultipartResolver extends StandardServletMultipartResolver
             for (Part part : request.getParts())
             {
                 // Filter to just file uploads
-                if (part.getSubmittedFileName() != null)
+                if (!StringUtils.isEmpty(part.getSubmittedFileName()))
                 {
-                    FileUtil.checkAllowedFileName(part.getSubmittedFileName(), true);
-                    validate(part);
+                    try
+                    {
+                        FileUtil.checkAllowedFileName(part.getSubmittedFileName(), true);
+                        validate(part);
+                    }
+                    catch (IOException e)
+                    {
+                        throw new InvalidFileExtensionException(e.getMessage());
+                    }
                 }
             }
         }
@@ -40,4 +50,12 @@ protected void validate(Part part)
     {
         //do nothing by default, but give subclasses a chance to override
     }
+
+    public static class InvalidFileExtensionException extends ApiUsageException
+    {
+        public InvalidFileExtensionException(String msg)
+        {
+            super(msg);
+        }
+    }
 }

api/src/org/labkey/api/settings/AppPropsImpl.java

@@ -37,6 +37,7 @@
 import org.labkey.api.util.MothershipReport;
 import org.labkey.api.util.PageFlowUtil;
 import org.labkey.api.util.Path;
+import org.labkey.api.util.SortHelpers;
 import org.labkey.api.util.URLHelper;
 import org.labkey.api.util.UsageReportingLevel;
 import org.labkey.api.util.logging.LogHelper;
@@ -49,6 +50,7 @@
 import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
@@ -74,7 +76,7 @@ class AppPropsImpl extends AbstractWriteableSettingsGroup implements AppProps
     static final String LOOK_AND_FEEL_REVISION = "logoRevision";
     static final String DEFAULT_LSID_AUTHORITY_PROP = "defaultLsidAuthority";
     static final String OPTIONAL_FEATURE_PREFIX = OPTIONAL_FEATURE + ".";
-    static final String EXTERNAL_HOST_DELIMITER = "\n";
+    static final String ALLOW_LIST_DELIMITER = "\n";
 
     private static final String SITE_CONFIG_NAME = "SiteConfig";
     private static final String SERVER_GUID = "serverGUID";
@@ -664,24 +666,26 @@ public boolean isIncludeServerHttpHeader()
     @NotNull
     public List<String> getExternalRedirectHosts()
     {
-        return getExternalHosts(externalRedirectHostURLs);
+        return getAllowList(externalRedirectHostURLs);
     }
 
     @Override
     @NotNull
     public List<String> getAllowedExtensions()
     {
-        return getExternalHosts(allowedFileExtensions);
+        return getAllowList(allowedFileExtensions);
     }
 
-    private List<String> getExternalHosts(RandomStartupProperties propName)
+    private List<String> getAllowList(RandomStartupProperties propName)
     {
         String urls = lookupStringValue(propName, "");
         if (StringUtils.isNotBlank(urls))
         {
-            return new ArrayList<>(Arrays.asList(urls.split(EXTERNAL_HOST_DELIMITER)));
+            List<String> hosts = new ArrayList<>(Arrays.asList(urls.split(ALLOW_LIST_DELIMITER)));
+            hosts.sort(SortHelpers.getNaturalOrderStringComparator());
+            return Collections.unmodifiableList(hosts);
         }
-        return new ArrayList<>();
+        return Collections.emptyList();
     }
 
     public static void populateSiteSettingsWithStartupProps()
@@ -734,7 +738,7 @@ public List<String> getExternalSourceHosts()
         String urls = lookupStringValue("externalSourceHostURLs", "");
         if (StringUtils.isNotBlank(urls))
         {
-            return new ArrayList<>(Arrays.asList(urls.split(EXTERNAL_HOST_DELIMITER)));
+            return new ArrayList<>(Arrays.asList(urls.split(ALLOW_LIST_DELIMITER)));
         }
         return new ArrayList<>();
     }

api/src/org/labkey/api/settings/RandomStartupProperties.java

@@ -26,15 +26,15 @@ public void setValue(WriteableAppProps writeable, String value)
         @Override
         public void setValue(WriteableAppProps writeable, String value)
         {
-            writeable.setExternalRedirectHosts(Arrays.asList(StringUtils.split(value, AppPropsImpl.EXTERNAL_HOST_DELIMITER)));
+            writeable.setExternalRedirectHosts(Arrays.asList(StringUtils.split(value, AppPropsImpl.ALLOW_LIST_DELIMITER)));
         }
     },
     allowedFileExtensions("Allowed file extensions")
     {
         @Override
         public void setValue(WriteableAppProps writeable, String value)
         {
-            writeable.setAllowedFileExtensions(Arrays.asList(StringUtils.split(value, AppPropsImpl.EXTERNAL_HOST_DELIMITER)));
+            writeable.setAllowedFileExtensions(Arrays.asList(StringUtils.split(value, AppPropsImpl.ALLOW_LIST_DELIMITER)));
             FileUtil.clearExtensionChecker();  // Not sure this is needed, but better safe than sorry.
         }
     },

api/src/org/labkey/api/settings/WriteableAppProps.java

@@ -229,17 +229,17 @@ public void setXFrameOption(String option)
 
     public void setExternalRedirectHosts(@NotNull Collection<String> externalRedirectHosts)
     {
-        setExternalHosts(externalRedirectHostURLs, externalRedirectHosts);
+        setAllowList(externalRedirectHostURLs, externalRedirectHosts);
     }
 
-    private void setExternalHosts(RandomStartupProperties propName, @NotNull Collection<String> externalSourceHosts)
+    private void setAllowList(RandomStartupProperties propName, @NotNull Collection<String> externalSourceHosts)
     {
-        storeStringValue(propName, String.join(EXTERNAL_HOST_DELIMITER, externalSourceHosts));
+        storeStringValue(propName, String.join(ALLOW_LIST_DELIMITER, externalSourceHosts));
     }
 
     public void setAllowedFileExtensions(Collection<String> allowedFileExtensions)
     {
-        setExternalHosts(RandomStartupProperties.allowedFileExtensions, allowedFileExtensions);
+        setAllowList(RandomStartupProperties.allowedFileExtensions, allowedFileExtensions);
         FileUtil.clearExtensionChecker();
     }
 

api/src/org/labkey/api/util/FileUtil.java

@@ -326,7 +326,7 @@ static String isAllowedFileName(String s, boolean checkFileExtension, AppProps a
         {
             String badExtension = checkExtension(s, AppProps.getInstance());
             if (badExtension != null)
-                return "This file type [" + badExtension + "] is not allowed.";
+                return "This file type [" + badExtension + "] is not allowed. Accepted file extensions: " + AppProps.getInstance().getAllowedExtensions();
         }
         return null;
     }
@@ -1533,11 +1533,6 @@ else if (ch == '-' &&
         if (ch == ' ' || ch == '.')
             ret[lastIndex] = '_';
 
-        String result = new String(ret);
-
-        assert !AppProps.getWriteableInstance().isInvalidFilenameBlocked() || isAllowedFileName(result, true) == null :
-                "Failed to make filename safe. Original: " + name + ", transformed: " + result + ", error: " + isAllowedFileName(result, true);
-
         return new String(ret);
     }
 

api/src/org/labkey/api/view/MockHttpResponseWithRealPassthrough.java

@@ -15,6 +15,7 @@
  */
 package org.labkey.api.view;
 
+import org.jetbrains.annotations.NotNull;
 import org.labkey.api.action.ApiUsageException;
 import org.springframework.mock.web.MockHttpServletResponse;
 
@@ -50,7 +51,7 @@ public HttpServletResponse getResponse()
     }
 
     @Override
-    public PrintWriter getWriter() throws UnsupportedEncodingException
+    public @NotNull PrintWriter getWriter() throws UnsupportedEncodingException
     {
         if (_writer == null)
         {
@@ -76,7 +77,7 @@ public SizeLimitExceededException()
     /** Checks the size of the current output to make sure it hasn't exceeded the limit before appending the new content */
     private class SizeLimitingPrintWriter extends PrintWriter
     {
-        private PrintWriter _out;
+        private final PrintWriter _out;
 
         public SizeLimitingPrintWriter(PrintWriter out)
         {
@@ -100,28 +101,28 @@ private void checkSize()
         }
 
         @Override
-        public void write(char[] buf, int off, int len)
+        public void write(char @NotNull [] buf, int off, int len)
         {
             checkSize();
             _out.write(buf, off, len);
         }
 
         @Override
-        public void write(char[] buf)
+        public void write(char @NotNull [] buf)
         {
             checkSize();
             _out.write(buf);
         }
 
         @Override
-        public void write(String s, int off, int len)
+        public void write(@NotNull String s, int off, int len)
         {
             checkSize();
             _out.write(s, off, len);
         }
 
         @Override
-        public void write(String s)
+        public void write(@NotNull String s)
         {
             checkSize();
             _out.write(s);
@@ -170,7 +171,7 @@ public void print(double d)
         }
 
         @Override
-        public void print(char[] s)
+        public void print(char @NotNull [] s)
         {
             checkSize();
             _out.print(s);
@@ -240,7 +241,7 @@ public void println(double x)
         }
 
         @Override
-        public void println(char[] x)
+        public void println(char @NotNull [] x)
         {
             checkSize();
             _out.println(x);
@@ -261,28 +262,28 @@ public void println(Object x)
         }
 
         @Override
-        public PrintWriter printf(String format, Object... args)
+        public PrintWriter printf(@NotNull String format, Object... args)
         {
             checkSize();
             return _out.printf(format, args);
         }
 
         @Override
-        public PrintWriter printf(Locale l, String format, Object... args)
+        public PrintWriter printf(Locale l, @NotNull String format, Object... args)
         {
             checkSize();
             return _out.printf(l, format, args);
         }
 
         @Override
-        public PrintWriter format(String format, Object... args)
+        public PrintWriter format(@NotNull String format, Object... args)
         {
             checkSize();
             return _out.format(format, args);
         }
 
         @Override
-        public PrintWriter format(Locale l, String format, Object... args)
+        public PrintWriter format(Locale l, @NotNull String format, Object... args)
         {
             checkSize();
             return _out.format(l, format, args);

core/src/org/labkey/core/CoreModule.java

@@ -1212,6 +1212,14 @@ public void moduleStartupComplete(ServletContext servletContext)
             roleAssignments.put("dataClassDesignerCount", new SqlSelector(CoreSchema.getInstance().getSchema(), roleCountSql, "org.labkey.experiment.security.DataClassDesignerRole").getObject(Long.class));
             roleAssignments.put("sampleTypeDesignerCount", new SqlSelector(CoreSchema.getInstance().getSchema(), roleCountSql, "org.labkey.experiment.security.SampleTypeDesignerRole").getObject(Long.class));
             results.put("roleAssignments", roleAssignments);
+
+            Map<String, Integer> allowListCounts = new HashMap<>();
+            for (AllowListType type : AllowListType.values())
+            {
+                allowListCounts.put(type.name(), type.getValues().size());
+            }
+            results.put("allowListCounts", allowListCounts);
+
             return results;
         });
 

core/src/org/labkey/core/admin/AdminController.java

@@ -10917,7 +10917,7 @@ public boolean handlePost(AllowListForm form, BindException errors) throws Excep
             if (form.isDelete())
             {
                 String urlToDelete = form.getExistingValue();
-                List<String> values = allowListType.getValues();
+                List<String> values = new ArrayList<>(allowListType.getValues());
                 for (String value : values)
                 {
                     if (null != urlToDelete && urlToDelete.trim().equalsIgnoreCase(value.trim()))

core/src/org/labkey/core/admin/existingListValues.jsp

@@ -68,7 +68,6 @@
     <%
         AllowListForm bean = (AllowListForm) HttpView.currentModel();
         List<String> exitingValues = bean.getExistingValuesList();
-        exitingValues.sort(SortHelpers.getNaturalOrderStringComparator());
     %>
     <table class="labkey-data-region-legacy labkey-show-borders">
         <tr>

search/src/org/labkey/search/model/LuceneSearchServiceImpl.java

@@ -2035,7 +2035,7 @@ private Map<String, Pair<Integer, String[]>> getExpectations()
             add(map, "xml_sample.xml", 444, "The Search module offers full-text search of server contents", "The Awesome LabKey Team");
             add(map, "zip_sample.zip", 1935, "map a source tsv column", "if there are NO explicit import definitions", "SequenceNum\toriginal_column\toriginal_column_numeric");
             add(map, "zip_sample.zip", 1935, "map a source tsv column", "if there are NO explicit import definitions", "SequenceNum\toriginal_column\toriginal_column_numeric");
-
+            add(map, "targz_sample.tar.gz", 74, "targz_sample/._hello.txt", "targz_sample/hello.txt");
             return map;
         }