Address 52916, and fix allowed extensions list update process

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

@@ -3,6 +3,7 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Logger;
 import org.labkey.api.files.FileContentService;
+import org.labkey.api.util.FileUtil;
 import org.labkey.api.util.SafeToRenderEnum;
 import org.labkey.api.util.logging.LogHelper;
 
@@ -34,6 +35,7 @@ public void setValue(WriteableAppProps writeable, String value)
         public void setValue(WriteableAppProps writeable, String value)
         {
             writeable.setAllowedFileExtensions(Arrays.asList(StringUtils.split(value, AppPropsImpl.EXTERNAL_HOST_DELIMITER)));
+            FileUtil.clearExtensionChecker();  // Not sure this is needed, but better safe than sorry.
         }
     },
     fileUploadDisabled("Disable file upload")

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

@@ -240,7 +240,7 @@ private void setExternalHosts(RandomStartupProperties propName, @NotNull Collect
     public void setAllowedFileExtensions(Collection<String> allowedFileExtensions)
     {
         setExternalHosts(RandomStartupProperties.allowedFileExtensions, allowedFileExtensions);
-        FileUtil.setExtensionChecker(AppProps.getInstance());
+        FileUtil.clearExtensionChecker();
     }
 
     public void setAllowedExternalResourceHosts(String jsonArray)

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

@@ -349,14 +349,19 @@ private static String checkExtension(String filename, AppProps appProps)
         return extensionChecker.matcher(filename).matches() ? null : extension;
     }
 
-    public static void setExtensionChecker(AppProps appProps)
+    private static void setExtensionChecker(AppProps appProps)
     {
         // Regex encode the allowed extensions (escape periods and add '|' optional matcher)
         String allowedExtensions = appProps.getAllowedExtensions().stream().map(Pattern::quote).collect(Collectors.joining("|"));
         // Allow any extension in the list unless it is preceeded by a '.' which we use as a proxy for double/multi extensions
         extensionChecker = Pattern.compile(String.format("^[^\\.]*(%1$s)$", allowedExtensions), Pattern.CASE_INSENSITIVE);
     }
 
+    public static void clearExtensionChecker()
+    {
+        extensionChecker = null;
+    }
+
     public static void checkAllowedFileName(String s, boolean checkFileExtension) throws IOException
     {
         String msg = isAllowedFileName(s, checkFileExtension);

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

@@ -207,6 +207,7 @@
 import org.labkey.core.admin.ActionsTsvWriter;
 import org.labkey.core.admin.AdminConsoleServiceImpl;
 import org.labkey.core.admin.AdminController;
+import org.labkey.core.admin.AllowListType;
 import org.labkey.core.admin.CopyFileRootPipelineJob;
 import org.labkey.core.admin.CustomizeMenuForm;
 import org.labkey.core.admin.DisplayFormatAnalyzer;
@@ -1358,6 +1359,7 @@ public Set<Class> getIntegrationTests()
             AdminController.SerializationTest.class,
             AdminController.TestCase.class,
             AdminController.WorkbookDeleteTestCase.class,
+            AllowListType.TestCase.class,
             AttachmentServiceImpl.TestCase.class,
             CoreController.TestCase.class,
             DataRegion.TestCase.class,

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

@@ -3,16 +3,25 @@
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.validator.routines.UrlValidator;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Test;
 import org.labkey.api.action.LabKeyError;
 import org.labkey.api.data.Container;
 import org.labkey.api.security.User;
 import org.labkey.api.settings.AppProps;
 import org.labkey.api.settings.WriteableAppProps;
+import org.labkey.api.test.TestWhen;
+import org.labkey.api.util.FileUtil;
 import org.labkey.api.util.HtmlString;
 import org.labkey.api.util.URLHelper;
 import org.labkey.api.view.ActionURL;
 import org.springframework.validation.BindException;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 
@@ -109,6 +118,7 @@ public void setValues(Collection<String> allowedExtensions, User user)
             WriteableAppProps props = AppProps.getWriteableInstance();
             props.setAllowedFileExtensions(allowedExtensions);
             props.save(user);
+            FileUtil.clearExtensionChecker(); // Should be redundant, but going to leave this here
         }
 
         @Override
@@ -150,4 +160,42 @@ public URLHelper getSuccessURL(Container container)
     {
         return new ActionURL(AdminController.AllowListAction.class, container).addParameter("type", name());
     }
+
+    public static class TestCase extends Assert
+    {
+        @Test
+        public void testAllowedExtensions()
+        {
+            List<String> existing = FileExtension.getValues();
+            Assume.assumeTrue("Initial allowed extensions list should be empty to prevent overriding existing values", existing.isEmpty());
+
+            try
+            {
+                List<String> newValues = Arrays.asList(".tar.gz", ".bar");
+                FileExtension.setValues(newValues, User.getAdminServiceUser());
+
+                try
+                {
+                    FileUtil.checkAllowedFileName("test.tar.gz", true);
+                    FileUtil.checkAllowedFileName("foo.bar", true);
+                }
+                catch (IOException e)
+                {
+                    fail("Filename should've been accepted: " + e.getMessage());
+                }
+
+                Assert.assertThrows("We dont allow 'extra' extensions", IOException.class, () -> FileUtil.checkAllowedFileName("test.foo.tar.gz", true));
+                Assert.assertThrows("We dont allow partial extensions, first segment", IOException.class, () -> FileUtil.checkAllowedFileName("test.tar", true));
+                Assert.assertThrows("We dont allow partial extensions, second segment", IOException.class, () -> FileUtil.checkAllowedFileName("test.gz", true));
+                Assert.assertThrows("We dont allow files with no extensions", IOException.class, () -> FileUtil.checkAllowedFileName("test", true));
+            }
+            finally
+            {
+                // Verify values were restored to original state.
+                FileExtension.setValues(existing, User.getAdminServiceUser());
+                List<String> current = FileExtension.getValues();
+                assertEquals(existing, current);
+            }
+        }
+    }
 }