Fix unit test errors on Windows environment (#4842)

#### What type of PR is this?

/kind cleanup
/area core

#### What this PR does / why we need it:

This PR fixes the unit test errors on Windows environment, mainly caused by different file systems.

```bash
PS C:\Users\johnniang\IdeaProjects\halo> ./gradlew check

> Task :application:checkstyleTest
Checkstyle rule violations were found. See the report at: file:///C:/Users/johnniang/IdeaProjects/halo/application/build/reports/checkstyle/test.html
Checkstyle files with violations: 16
Checkstyle violations by severity: [warning:43]


> Task :application:checkstyleMain
Checkstyle rule violations were found. See the report at: file:///C:/Users/johnniang/IdeaProjects/halo/application/build/reports/checkstyle/main.html
Checkstyle files with violations: 135
Checkstyle violations by severity: [warning:218]


> Task :application:test


BUILD SUCCESSFUL in 1m 39s
25 actionable tasks: 5 executed, 20 up-to-date
```

#### Does this PR introduce a user-facing change?

```release-note
None
```
pull/4850/head^2
John Niang 2023-11-13 13:32:08 +08:00 committed by GitHub
parent 4a6ce88b7f
commit 70402994fa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 67 additions and 41 deletions

View File

@ -8,7 +8,10 @@ import static run.halo.app.plugin.PluginConst.PLUGIN_PATH;
import static run.halo.app.plugin.PluginConst.RELOAD_ANNO; import static run.halo.app.plugin.PluginConst.RELOAD_ANNO;
import java.io.IOException; import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URI; import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.Paths; import java.nio.file.Paths;
@ -40,6 +43,7 @@ import org.springframework.lang.Nullable;
import org.springframework.retry.support.RetryTemplate; import org.springframework.retry.support.RetryTemplate;
import org.springframework.stereotype.Component; import org.springframework.stereotype.Component;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.ResourceUtils;
import org.springframework.web.util.UriComponentsBuilder; import org.springframework.web.util.UriComponentsBuilder;
import run.halo.app.core.extension.Plugin; import run.halo.app.core.extension.Plugin;
import run.halo.app.core.extension.ReverseProxy; import run.halo.app.core.extension.ReverseProxy;
@ -649,11 +653,19 @@ public class PluginReconciler implements Reconciler<Request> {
if (StringUtils.isBlank(pathString)) { if (StringUtils.isBlank(pathString)) {
return null; return null;
} }
String processedPathString = pathString; try {
if (processedPathString.startsWith("file:")) { var pathURL = new URL(pathString);
processedPathString = processedPathString.substring(7); if (!ResourceUtils.isFileURL(pathURL)) {
throw new IllegalArgumentException("The path cannot be resolved to absolute file"
+ " path because it does not reside in the file system: "
+ pathString);
}
var pathURI = ResourceUtils.toURI(pathURL);
return Paths.get(pathURI);
} catch (MalformedURLException | URISyntaxException ignored) {
// the given path string is not a valid URL.
} }
return Paths.get(processedPathString); return Paths.get(pathString);
} }
URI toUri(String pathString) { URI toUri(String pathString) {

View File

@ -1,6 +1,7 @@
package run.halo.app.config; package run.halo.app.config;
import java.util.List; import java.util.List;
import org.hamcrest.core.StringStartsWith;
import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
@ -30,7 +31,8 @@ class WebFluxConfigTest {
.forEach(uri -> webClient.get().uri(uri) .forEach(uri -> webClient.get().uri(uri)
.exchange() .exchange()
.expectStatus().isOk() .expectStatus().isOk()
.expectBody(String.class).isEqualTo("console index\n")); .expectBody(String.class).value(StringStartsWith.startsWith("console index"))
);
} }
@Test @Test
@ -38,7 +40,7 @@ class WebFluxConfigTest {
webClient.get().uri("/console/assets/fake.txt") webClient.get().uri("/console/assets/fake.txt")
.exchange() .exchange()
.expectStatus().isOk() .expectStatus().isOk()
.expectBody(String.class).isEqualTo("fake.\n"); .expectBody(String.class).value(StringStartsWith.startsWith("fake."));
} }
@Test @Test

View File

@ -3,7 +3,9 @@ package run.halo.app.core.extension.reconciler;
import static java.util.Objects.requireNonNull; import static java.util.Objects.requireNonNull;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
@ -369,12 +371,15 @@ class PluginReconcilerTest {
@Test @Test
void resolvePluginPathAnnotation() { void resolvePluginPathAnnotation() {
when(haloPluginManager.getPluginsRoot()).thenReturn(Paths.get("/tmp/plugins")); var pluginRoot = Paths.get("tmp", "plugins");
String path = pluginReconciler.resolvePluginPathForAnno("/tmp/plugins/sitemap-1.0.jar"); when(haloPluginManager.getPluginsRoot()).thenReturn(pluginRoot);
var path = pluginReconciler.resolvePluginPathForAnno(
pluginRoot.resolve("sitemap-1.0.jar").toString());
assertThat(path).isEqualTo("sitemap-1.0.jar"); assertThat(path).isEqualTo("sitemap-1.0.jar");
path = pluginReconciler.resolvePluginPathForAnno("/abc/plugins/sitemap-1.0.jar"); var givenPath = Paths.get("abc", "plugins", "sitemap-1.0.jar");
assertThat(path).isEqualTo("/abc/plugins/sitemap-1.0.jar"); path = pluginReconciler.resolvePluginPathForAnno(givenPath.toString());
assertThat(path).isEqualTo(givenPath.toString());
} }
@Nested @Nested
@ -457,17 +462,18 @@ class PluginReconcilerTest {
assertThat(pluginReconciler.toPath("")).isNull(); assertThat(pluginReconciler.toPath("")).isNull();
assertThat(pluginReconciler.toPath(" ")).isNull(); assertThat(pluginReconciler.toPath(" ")).isNull();
Path path = pluginReconciler.toPath("file:///path/to/file.txt"); final var filePath = Paths.get("path", "to", "file.txt").toAbsolutePath();
assertThat(path).isNotNull();
assertThat(path.toString()).isEqualTo("/path/to/file.txt");
assertThat(pluginReconciler.toPath("C:\\Users\\faker\\halo\\plugins").toString()) // test for file:///
.isEqualTo("C:\\Users\\faker\\halo\\plugins"); assertEquals(filePath, pluginReconciler.toPath(filePath.toUri().toString()));
assertThat(pluginReconciler.toPath("C:/Users/faker/halo/plugins").toString()) // test for absolute path /home/xyz or C:\Windows
.isEqualTo("C:/Users/faker/halo/plugins"); assertEquals(filePath, pluginReconciler.toPath(filePath.toString()));
Path windowsPath = Paths.get("C:/Users/username/Documents/file.txt");
assertThat(pluginReconciler.toPath("file://C:/Users/username/Documents/file.txt")) var exception = assertThrows(IllegalArgumentException.class, () -> {
.isEqualTo(windowsPath); var fileUri = filePath.toUri();
pluginReconciler.toPath(fileUri.toString().replaceFirst("file", "http"));
});
assertTrue(exception.getMessage().contains("not reside in the file system"));
} }
@Test @Test
@ -483,8 +489,9 @@ class PluginReconcilerTest {
}); });
// Test with non-empty pathString // Test with non-empty pathString
URI uri = pluginReconciler.toUri("/path/to/file"); var filePath = Paths.get("path", "to", "file");
Assertions.assertEquals("file:///path/to/file", uri.toString()); URI uri = pluginReconciler.toUri(filePath.toString());
assertEquals(filePath.toUri(), uri);
} }
} }

View File

@ -207,8 +207,9 @@ class PluginServiceImplTest {
String pluginName = "test-plugin"; String pluginName = "test-plugin";
PluginWrapper pluginWrapper = mock(PluginWrapper.class); PluginWrapper pluginWrapper = mock(PluginWrapper.class);
when(pluginManager.getPlugin(pluginName)).thenReturn(pluginWrapper); when(pluginManager.getPlugin(pluginName)).thenReturn(pluginWrapper);
var pluginPath = Paths.get("tmp", "plugins", "fake-plugin.jar");
when(pluginWrapper.getPluginPath()) when(pluginWrapper.getPluginPath())
.thenReturn(Paths.get("/tmp/plugins/fake-plugin.jar")); .thenReturn(pluginPath);
Plugin plugin = new Plugin(); Plugin plugin = new Plugin();
plugin.setMetadata(new Metadata()); plugin.setMetadata(new Metadata());
plugin.getMetadata().setName(pluginName); plugin.getMetadata().setName(pluginName);
@ -224,7 +225,7 @@ class PluginServiceImplTest {
verify(client, times(1)).update( verify(client, times(1)).update(
argThat(p -> { argThat(p -> {
String reloadPath = p.getMetadata().getAnnotations().get(PluginConst.RELOAD_ANNO); String reloadPath = p.getMetadata().getAnnotations().get(PluginConst.RELOAD_ANNO);
assertThat(reloadPath).isEqualTo("/tmp/plugins/fake-plugin.jar"); assertThat(reloadPath).isEqualTo(pluginPath.toString());
return true; return true;
}) })
); );

View File

@ -64,8 +64,9 @@ class FileUtilsTest {
unzip(zis, unzipTarget); unzip(zis, unzipTarget);
} }
var content = Files.readString(unzipTarget.resolve("examplefile")); var lines = Files.readAllLines(unzipTarget.resolve("examplefile"));
assertEquals("Here is an example file.\n", content); assertEquals(1, lines.size());
assertEquals("Here is an example file.", lines.get(0));
} }
@Test @Test
@ -79,9 +80,9 @@ class FileUtilsTest {
try (var zis = new ZipInputStream(Files.newInputStream(zipPath))) { try (var zis = new ZipInputStream(Files.newInputStream(zipPath))) {
unzip(zis, unzipTarget); unzip(zis, unzipTarget);
} }
var lines = Files.readAllLines(unzipTarget.resolve("examplefile"));
var content = Files.readString(unzipTarget.resolve("examplefile")); assertEquals(1, lines.size());
assertEquals("Here is an example file.\n", content); assertEquals("Here is an example file.", lines.get(0));
} }
@Test @Test

View File

@ -23,7 +23,7 @@ class ThemeConfigurationTest {
@InjectMocks @InjectMocks
private ThemeConfiguration themeConfiguration; private ThemeConfiguration themeConfiguration;
private final Path themeRoot = Paths.get("/tmp/.halo/themes"); private final Path themeRoot = Paths.get("tmp", ".halo", "themes");
@BeforeEach @BeforeEach
void setUp() { void setUp() {
@ -33,25 +33,28 @@ class ThemeConfigurationTest {
@Test @Test
void themeAssets() { void themeAssets() {
Path path = themeConfiguration.getThemeAssetsPath("fake-theme", "hello.jpg"); Path path = themeConfiguration.getThemeAssetsPath("fake-theme", "hello.jpg");
assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/hello.jpg")); assertThat(path).isEqualTo(
themeRoot.resolve(Paths.get("fake-theme", "templates", "assets", "hello.jpg")));
path = themeConfiguration.getThemeAssetsPath("fake-theme", "./hello.jpg"); path = themeConfiguration.getThemeAssetsPath("fake-theme", "./hello.jpg");
assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/./hello.jpg")); assertThat(path).isEqualTo(
themeRoot.resolve(Paths.get("fake-theme", "templates", "assets", ".", "hello.jpg")));
assertThatThrownBy(() -> { assertThatThrownBy(() ->
themeConfiguration.getThemeAssetsPath("fake-theme", "../../hello.jpg"); themeConfiguration.getThemeAssetsPath("fake-theme", "../../hello.jpg"))
}).isInstanceOf(AccessDeniedException.class) .isInstanceOf(AccessDeniedException.class)
.hasMessage( .hasMessageContaining("Directory traversal detected");
"403 FORBIDDEN \"Directory traversal detected: /tmp/"
+ ".halo/themes/fake-theme/templates/assets/../../hello.jpg\"");
path = themeConfiguration.getThemeAssetsPath("fake-theme", "%2e%2e/f.jpg"); path = themeConfiguration.getThemeAssetsPath("fake-theme", "%2e%2e/f.jpg");
assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/%2e%2e/f.jpg")); assertThat(path).isEqualTo(
themeRoot.resolve(Paths.get("fake-theme", "templates", "assets", "%2e%2e", "f.jpg")));
path = themeConfiguration.getThemeAssetsPath("fake-theme", "f/./../p.jpg"); path = themeConfiguration.getThemeAssetsPath("fake-theme", "f/./../p.jpg");
assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/f/./../p.jpg")); assertThat(path).isEqualTo(themeRoot.resolve(
Paths.get("fake-theme", "templates", "assets", "f", ".", "..", "p.jpg")));
path = themeConfiguration.getThemeAssetsPath("fake-theme", "f../p.jpg"); path = themeConfiguration.getThemeAssetsPath("fake-theme", "f../p.jpg");
assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/f../p.jpg")); assertThat(path).isEqualTo(
themeRoot.resolve(Paths.get("fake-theme", "templates", "assets", "f..", "p.jpg")));
} }
} }