Merge branch 'main' into perf/improve-social-login

api/src/main/java/run/halo/app/plugin/BasePlugin.java

@@ -2,7 +2,6 @@
 
 import lombok.extern.slf4j.Slf4j;
 import org.pf4j.Plugin;
-import org.pf4j.PluginManager;
 import org.pf4j.PluginWrapper;
 
 /**
@@ -15,12 +14,12 @@
 @Slf4j
 public class BasePlugin extends Plugin {
 
+    @Deprecated
     public BasePlugin(PluginWrapper wrapper) {
         super(wrapper);
         log.info("Initialized plugin {}", wrapper.getPluginId());
     }
 
-    private PluginManager getPluginManager() {
-        return getWrapper().getPluginManager();
+    public BasePlugin() {
     }
 }

application/build.gradle

@@ -51,7 +51,6 @@ ext {
     base62 = "0.1.3"
     pf4j = '3.9.0'
     javaDiffUtils = "4.12"
-    guava = "31.1-jre"
     jsoup = '1.15.3'
     jsonPatch = "1.13"
     springDocOpenAPI = "2.0.2"

application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java

@@ -289,8 +289,7 @@ void stopAction(String name) {
 
     void stateTransition(String name, Function<PluginState, Boolean> stateAction,
         PluginState desiredState) {
-        PluginWrapper pluginWrapper = getPluginWrapper(name);
-        PluginState currentState = pluginWrapper.getPluginState();
+        PluginState currentState = getPluginWrapper(name).getPluginState();
         int maxRetries = PluginState.values().length;
         for (int i = 0; i < maxRetries && currentState != desiredState; i++) {
             try {
@@ -303,7 +302,7 @@ void stateTransition(String name, Function<PluginState, Boolean> stateAction,
                     break;
                 }
                 // update current state
-                currentState = pluginWrapper.getPluginState();
+                currentState = getPluginWrapper(name).getPluginState();
             } catch (Throwable e) {
                 persistenceFailureStatus(name, e);
                 throw e;

application/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java

@@ -40,11 +40,7 @@ public Result reconcile(Request request) {
         client.fetch(Attachment.class, request.name()).ifPresent(attachment -> {
             // TODO Handle the finalizer
             if (attachment.getMetadata().getDeletionTimestamp() != null) {
-                attachmentService.delete(attachment)
-                    .doOnNext(deletedAttachment -> {
-                        removeFinalizer(attachment.getMetadata().getName());
-                    })
-                    .blockOptional();
+                removeFinalizer(attachment);
                 return;
             }
             // add finalizer
@@ -89,14 +85,25 @@ void updateStatus(String attachmentName, AttachmentStatus status) {
             });
     }
 
-    void removeFinalizer(String attachmentName) {
-        client.fetch(Attachment.class, attachmentName).ifPresent(attachment -> {
-            var finalizers = attachment.getMetadata().getFinalizers();
-            if (finalizers != null && finalizers.remove(Constant.FINALIZER_NAME)) {
-                // update it
-                client.update(attachment);
-            }
-        });
+    void removeFinalizer(Attachment oldAttachment) {
+        if (!hasFinalizer(oldAttachment, Constant.FINALIZER_NAME)) {
+            return;
+        }
+        attachmentService.delete(oldAttachment).block();
+        client.fetch(Attachment.class, oldAttachment.getMetadata().getName())
+            .ifPresent(attachment -> {
+                var finalizers = attachment.getMetadata().getFinalizers();
+                if (hasFinalizer(attachment, Constant.FINALIZER_NAME)
+                    && finalizers.remove(Constant.FINALIZER_NAME)) {
+                    // update it
+                    client.update(attachment);
+                }
+            });
+    }
+
+    boolean hasFinalizer(Attachment attachment, String finalizer) {
+        var finalizers = attachment.getMetadata().getFinalizers();
+        return finalizers != null && finalizers.contains(finalizer);
     }
 
     void addFinalizerIfNotSet(String attachmentName, Set<String> existingFinalizers) {

application/src/main/java/run/halo/app/plugin/BasePluginFactory.java

@@ -29,7 +29,7 @@ public Plugin create(PluginWrapper pluginWrapper) {
                         "No bean named 'basePlugin' found in the context create default instance");
                     DefaultListableBeanFactory beanFactory =
                         context.getDefaultListableBeanFactory();
-                    BasePlugin pluginInstance = new BasePlugin(pluginWrapper);
+                    BasePlugin pluginInstance = new BasePlugin();
                     beanFactory.registerSingleton(Plugin.class.getName(), pluginInstance);
                     return pluginInstance;
                 }

application/src/main/java/run/halo/app/plugin/HaloPluginManager.java

@@ -229,7 +229,11 @@ public boolean validatePluginVersion(PluginWrapper pluginWrapper) {
     private PluginState doStartPlugin(String pluginId) {
         checkPluginId(pluginId);
 
-        PluginWrapper pluginWrapper = getPlugin(pluginId);
+        // refresh plugin to ensure cache object of PluginWrapper.plugin is up-to-date
+        // see gh-4016 to know why we need this
+        // TODO if has a better way to do this?
+        PluginWrapper pluginWrapper = refreshPluginWrapper(pluginId);
+
         checkExtensionFinderReady(pluginWrapper);
 
         PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor();
@@ -423,6 +427,37 @@ private void removePluginComponentsCache(String pluginId) {
         }
     }
 
+    /**
+     * <p>Refresh plugin wrapper by plugin name.</p>
+     *
+     * <p>It will be create a new plugin wrapper and replace old plugin wrapper to clean
+     * {@link PluginWrapper#getPlugin()} cache object.</p>
+     *
+     * @param pluginName plugin name
+     * @return refreshed plugin wrapper instance, plugin cache object will be null
+     * @throws IllegalArgumentException if plugin not found
+     */
+    protected synchronized PluginWrapper refreshPluginWrapper(String pluginName) {
+        checkPluginId(pluginName);
+        // get old plugin wrapper
+        PluginWrapper pluginWrapper = getPlugin(pluginName);
+        // create new plugin wrapper to replace old plugin wrapper
+        PluginWrapper refreshed = copyPluginWrapper(pluginWrapper);
+        this.plugins.put(pluginName, refreshed);
+        return refreshed;
+    }
+
+    @NonNull
+    PluginWrapper copyPluginWrapper(@NonNull PluginWrapper pluginWrapper) {
+        PluginWrapper refreshed =
+            createPluginWrapper(pluginWrapper.getDescriptor(), pluginWrapper.getPluginPath(),
+                pluginWrapper.getPluginClassLoader());
+        refreshed.setPluginFactory(getPluginFactory());
+        refreshed.setPluginState(pluginWrapper.getPluginState());
+        refreshed.setFailedException(pluginWrapper.getFailedException());
+        return refreshed;
+    }
+
     @Override
     public void destroy() throws Exception {
         stopPlugins();

application/src/main/java/run/halo/app/plugin/resources/BundleResourceUtils.java

@@ -5,6 +5,8 @@
 import org.springframework.core.io.Resource;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
+import run.halo.app.infra.utils.FileUtils;
 import run.halo.app.infra.utils.PathUtils;
 import run.halo.app.plugin.HaloPluginManager;
 import run.halo.app.plugin.PluginConst;
@@ -71,7 +73,9 @@ public static Resource getJsBundleResource(HaloPluginManager pluginManager, Stri
             return null;
         }
         String path = PathUtils.combinePath(CONSOLE_BUNDLE_LOCATION, bundleName);
-        Resource resource = resourceLoader.getResource(path);
+        String simplifyPath = StringUtils.cleanPath(path);
+        FileUtils.checkDirectoryTraversal("/" + CONSOLE_BUNDLE_LOCATION, simplifyPath);
+        Resource resource = resourceLoader.getResource(simplifyPath);
         return resource.exists() ? resource : null;
     }
 

application/src/main/java/run/halo/app/theme/ThemeConfiguration.java

@@ -20,6 +20,7 @@
 import org.thymeleaf.extras.springsecurity6.dialect.SpringSecurityDialect;
 import reactor.core.publisher.Mono;
 import run.halo.app.infra.ThemeRootGetter;
+import run.halo.app.infra.utils.FileUtils;
 import run.halo.app.theme.dialect.HaloSpringSecurityDialect;
 import run.halo.app.theme.dialect.LinkExpressionObjectDialect;
 
@@ -65,12 +66,14 @@ public RouterFunction<ServerResponse> themeAssets(WebProperties webProperties) {
             });
     }
 
-    private Path getThemeAssetsPath(String themeName, String resource) {
-        return themeRoot.get()
+    Path getThemeAssetsPath(String themeName, String resource) {
+        Path basePath = themeRoot.get()
             .resolve(themeName)
             .resolve("templates")
-            .resolve("assets")
-            .resolve(resource);
+            .resolve("assets");
+        Path result = basePath.resolve(resource);
+        FileUtils.checkDirectoryTraversal(basePath, result);
+        return result;
     }
 
     @Bean

application/src/test/java/run/halo/app/plugin/resources/BundleResourceUtilsTest.java

@@ -1,9 +1,9 @@
 package run.halo.app.plugin.resources;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.lenient;
-import static org.mockito.Mockito.when;
 
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -16,6 +16,7 @@
 import org.pf4j.PluginClassLoader;
 import org.pf4j.PluginWrapper;
 import org.springframework.core.io.Resource;
+import run.halo.app.infra.exception.AccessDeniedException;
 import run.halo.app.plugin.HaloPluginManager;
 
 /**
@@ -34,7 +35,7 @@ class BundleResourceUtilsTest {
     void setUp() throws MalformedURLException {
         PluginWrapper pluginWrapper = Mockito.mock(PluginWrapper.class);
         PluginClassLoader pluginClassLoader = Mockito.mock(PluginClassLoader.class);
-        when(pluginWrapper.getPluginClassLoader()).thenReturn(pluginClassLoader);
+        lenient().when(pluginWrapper.getPluginClassLoader()).thenReturn(pluginClassLoader);
         lenient().when(pluginManager.getPlugin(eq("fake-plugin"))).thenReturn(pluginWrapper);
 
         lenient().when(pluginClassLoader.getResource(eq("console/main.js"))).thenReturn(
@@ -77,5 +78,10 @@ void getJsBundleResource() {
         jsBundleResource =
             BundleResourceUtils.getJsBundleResource(pluginManager, "nothing-plugin", "main.js");
         assertThat(jsBundleResource).isNull();
+
+        assertThatThrownBy(() -> {
+            BundleResourceUtils.getJsBundleResource(pluginManager, "fake-plugin",
+                "../test/main.js");
+        }).isInstanceOf(AccessDeniedException.class);
     }
-}
+}

application/src/test/java/run/halo/app/theme/ThemeConfigurationTest.java

@@ -0,0 +1,57 @@
+package run.halo.app.theme;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.when;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import run.halo.app.infra.ThemeRootGetter;
+import run.halo.app.infra.exception.AccessDeniedException;
+
+@ExtendWith(MockitoExtension.class)
+class ThemeConfigurationTest {
+    @Mock
+    private ThemeRootGetter themeRootGetter;
+
+    @InjectMocks
+    private ThemeConfiguration themeConfiguration;
+
+    private final Path themeRoot = Paths.get("/tmp/.halo/themes");
+
+    @BeforeEach
+    void setUp() {
+        when(themeRootGetter.get()).thenReturn(themeRoot);
+    }
+
+    @Test
+    void themeAssets() {
+        Path path = themeConfiguration.getThemeAssetsPath("fake-theme", "hello.jpg");
+        assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/hello.jpg"));
+
+        path = themeConfiguration.getThemeAssetsPath("fake-theme", "./hello.jpg");
+        assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/./hello.jpg"));
+
+        assertThatThrownBy(() -> {
+            themeConfiguration.getThemeAssetsPath("fake-theme", "../../hello.jpg");
+        }).isInstanceOf(AccessDeniedException.class)
+            .hasMessage(
+                "403 FORBIDDEN \"Directory traversal detected: /tmp/"
+                    + ".halo/themes/fake-theme/templates/assets/../../hello.jpg\"");
+
+        path = themeConfiguration.getThemeAssetsPath("fake-theme", "%2e%2e/f.jpg");
+        assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/%2e%2e/f.jpg"));
+
+        path = themeConfiguration.getThemeAssetsPath("fake-theme", "f/./../p.jpg");
+        assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/f/./../p.jpg"));
+
+        path = themeConfiguration.getThemeAssetsPath("fake-theme", "f../p.jpg");
+        assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/f../p.jpg"));
+    }
+}

platform/application/build.gradle

@@ -15,7 +15,7 @@ ext {
     base62 = "0.1.3"
     pf4j = '3.9.0'
     javaDiffUtils = "4.12"
-    guava = "31.1-jre"
+    guava = "32.0.1-jre"
     jsoup = '1.15.3'
     jsonPatch = "1.13"
     springDocOpenAPI = "2.1.0"