From 565c8340e96f4d76b8ff15b5c60862b2d59c923d Mon Sep 17 00:00:00 2001 From: John Niang Date: Wed, 28 Sep 2022 15:48:21 +0800 Subject: [PATCH] Fix the problem of not being able to request SinglePage with slug name containing special characters (#2479) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind bug /kind api-change /area core /milestone 2.0 #### What this PR does / why we need it: Support setting special characters on slug name of SinglePage. #### Which issue(s) this PR fixes: Partial Fixes https://github.com/halo-dev/halo/issues/2473 #### Special notes for your reviewer: Steps to test: 1. Create single pags with slug name `中文`, `/a/b/c/d` or `a / b` 2. Request it with permalink generated by Halo #### Does this PR introduce a user-facing change? ```release-note None ``` --- .../reconciler/SinglePageReconciler.java | 7 +- .../engine/SpringWebFluxTemplateEngine.java | 1 - .../app/theme/router/PermalinkIndexer.java | 18 ++++ .../router/strategy/PermalinkPredicates.java | 17 ++++ .../strategy/SinglePageRouteStrategy.java | 8 +- .../theme/router/PermalinkIndexerTest.java | 14 ++++ .../strategy/SinglePageRouteStrategyTest.java | 83 ++++++++++++------- 7 files changed, 113 insertions(+), 35 deletions(-) create mode 100644 src/main/java/run/halo/app/theme/router/strategy/PermalinkPredicates.java diff --git a/src/main/java/run/halo/app/core/extension/reconciler/SinglePageReconciler.java b/src/main/java/run/halo/app/core/extension/reconciler/SinglePageReconciler.java index e40e41626..6619fa4e2 100644 --- a/src/main/java/run/halo/app/core/extension/reconciler/SinglePageReconciler.java +++ b/src/main/java/run/halo/app/core/extension/reconciler/SinglePageReconciler.java @@ -1,5 +1,8 @@ package run.halo.app.core.extension.reconciler; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.springframework.web.util.UriUtils.encodePath; + import java.time.Instant; import java.util.HashSet; import java.util.LinkedHashMap; @@ -150,8 +153,10 @@ public class SinglePageReconciler implements Reconciler { final SinglePage oldPage = JsonUtils.deepCopy(singlePage); permalinkOnDelete(oldPage); + var permalink = encodePath(singlePage.getSpec().getSlug(), UTF_8); + permalink = StringUtils.prependIfMissing(permalink, "/"); singlePage.getStatusOrDefault() - .setPermalink(PathUtils.combinePath(singlePage.getSpec().getSlug())); + .setPermalink(permalink); if (isPublished(singlePage)) { permalinkOnAdd(singlePage); } diff --git a/src/main/java/run/halo/app/theme/engine/SpringWebFluxTemplateEngine.java b/src/main/java/run/halo/app/theme/engine/SpringWebFluxTemplateEngine.java index f123f7e89..09f89356c 100644 --- a/src/main/java/run/halo/app/theme/engine/SpringWebFluxTemplateEngine.java +++ b/src/main/java/run/halo/app/theme/engine/SpringWebFluxTemplateEngine.java @@ -200,7 +200,6 @@ public class SpringWebFluxTemplateEngine extends SpringTemplateEngine try { process(templateName, markupSelectors, context, writer); - Mono.empty().block(); } catch (final Throwable t) { logger.error( diff --git a/src/main/java/run/halo/app/theme/router/PermalinkIndexer.java b/src/main/java/run/halo/app/theme/router/PermalinkIndexer.java index e34cc990d..78b15ffc6 100644 --- a/src/main/java/run/halo/app/theme/router/PermalinkIndexer.java +++ b/src/main/java/run/halo/app/theme/router/PermalinkIndexer.java @@ -157,6 +157,24 @@ public class PermalinkIndexer { } } + /** + * Get extension name by permalink. + * + * @param gvk is GroupVersionKind of extension + * @param permalink is encoded permalink + * @return extension name or null + */ + @Nullable + public String getNameByPermalink(GroupVersionKind gvk, String permalink) { + readWriteLock.readLock().lock(); + try { + var locator = permalinkLocatorLookup.get(permalink); + return locator == null ? null : locator.name(); + } finally { + readWriteLock.readLock().unlock(); + } + } + /** * Only for test. * diff --git a/src/main/java/run/halo/app/theme/router/strategy/PermalinkPredicates.java b/src/main/java/run/halo/app/theme/router/strategy/PermalinkPredicates.java new file mode 100644 index 000000000..3122de0d6 --- /dev/null +++ b/src/main/java/run/halo/app/theme/router/strategy/PermalinkPredicates.java @@ -0,0 +1,17 @@ +package run.halo.app.theme.router.strategy; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.springframework.web.reactive.function.server.RequestPredicates.method; +import static org.springframework.web.reactive.function.server.RequestPredicates.path; + +import org.springframework.http.HttpMethod; +import org.springframework.web.reactive.function.server.RequestPredicate; +import org.springframework.web.util.UriUtils; + +public enum PermalinkPredicates { + ; + + public static RequestPredicate get(String permalink) { + return method(HttpMethod.GET).and(path(UriUtils.decode(permalink, UTF_8))); + } +} diff --git a/src/main/java/run/halo/app/theme/router/strategy/SinglePageRouteStrategy.java b/src/main/java/run/halo/app/theme/router/strategy/SinglePageRouteStrategy.java index 1b03b3f0c..b0dd5376e 100644 --- a/src/main/java/run/halo/app/theme/router/strategy/SinglePageRouteStrategy.java +++ b/src/main/java/run/halo/app/theme/router/strategy/SinglePageRouteStrategy.java @@ -1,14 +1,13 @@ package run.halo.app.theme.router.strategy; import static org.springframework.web.reactive.function.server.RequestPredicates.accept; +import static run.halo.app.theme.router.strategy.PermalinkPredicates.get; import java.util.List; import java.util.Map; -import org.apache.commons.lang3.StringUtils; import org.springframework.http.MediaType; import org.springframework.stereotype.Component; import org.springframework.web.reactive.function.server.RequestPredicate; -import org.springframework.web.reactive.function.server.RequestPredicates; import org.springframework.web.reactive.function.server.RouterFunction; import org.springframework.web.reactive.function.server.RouterFunctions; import org.springframework.web.reactive.function.server.ServerResponse; @@ -49,13 +48,12 @@ public class SinglePageRouteStrategy implements TemplateRouterStrategy { List permalinks = permalinkIndexer.getPermalinks(gvk); for (String permalink : permalinks) { - requestPredicate = requestPredicate.or(RequestPredicates.GET(permalink)); + requestPredicate = requestPredicate.or(get(permalink)); } return RouterFunctions .route(requestPredicate.and(accept(MediaType.TEXT_HTML)), request -> { - String slug = StringUtils.removeStart(request.path(), "/"); - String name = permalinkIndexer.getNameBySlug(gvk, slug); + var name = permalinkIndexer.getNameByPermalink(gvk, request.path()); if (name == null) { return ServerResponse.notFound().build(); } diff --git a/src/test/java/run/halo/app/theme/router/PermalinkIndexerTest.java b/src/test/java/run/halo/app/theme/router/PermalinkIndexerTest.java index 29a016b1d..4fcdd5320 100644 --- a/src/test/java/run/halo/app/theme/router/PermalinkIndexerTest.java +++ b/src/test/java/run/halo/app/theme/router/PermalinkIndexerTest.java @@ -2,6 +2,8 @@ package run.halo.app.theme.router; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import java.util.List; import java.util.NoSuchElementException; @@ -106,4 +108,16 @@ class PermalinkIndexerTest { permalinkIndexer.getNameBySlug(gvk, "nothing"); }).isInstanceOf(NoSuchElementException.class); } + + @Test + void getNameByPermalink() { + ExtensionLocator locator = new ExtensionLocator(gvk, "test-name", "test-slug"); + permalinkIndexer.register(locator, "/test-permalink"); + + var name = permalinkIndexer.getNameByPermalink(gvk, "/test-permalink"); + assertEquals("test-name", name); + + name = permalinkIndexer.getNameByPermalink(gvk, "/invalid-permalink"); + assertNull(name); + } } \ No newline at end of file diff --git a/src/test/java/run/halo/app/theme/router/strategy/SinglePageRouteStrategyTest.java b/src/test/java/run/halo/app/theme/router/strategy/SinglePageRouteStrategyTest.java index 56ea75aed..51551ba0a 100644 --- a/src/test/java/run/halo/app/theme/router/strategy/SinglePageRouteStrategyTest.java +++ b/src/test/java/run/halo/app/theme/router/strategy/SinglePageRouteStrategyTest.java @@ -4,7 +4,9 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static run.halo.app.theme.DefaultTemplateEnum.SINGLE_PAGE; import java.util.List; import org.junit.jupiter.api.BeforeEach; @@ -13,15 +15,11 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.http.HttpStatus; import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.web.reactive.function.server.HandlerStrategies; -import org.springframework.web.reactive.function.server.RouterFunction; -import org.springframework.web.reactive.function.server.ServerResponse; import org.springframework.web.reactive.result.view.ViewResolver; import reactor.core.publisher.Mono; import run.halo.app.extension.ListResult; -import run.halo.app.theme.DefaultTemplateEnum; import run.halo.app.theme.finders.SinglePageFinder; import run.halo.app.theme.router.PermalinkIndexer; @@ -50,37 +48,66 @@ class SinglePageRouteStrategyTest { void setUp() { lenient().when(singlePageFinder.list(anyInt(), anyInt())) .thenReturn(new ListResult<>(1, 10, 0, List.of())); - when(permalinkIndexer.getPermalinks(any())) - .thenReturn(List.of("/fake-slug")); - when(permalinkIndexer.getNameBySlug(any(), eq("fake-slug"))) - .thenReturn("fake-name"); + lenient().when(viewResolver.resolveViewName(eq(SINGLE_PAGE.getValue()), any())) + .thenReturn(Mono.just(new EmptyView())); } @Test - void getRouteFunction() { - RouterFunction routeFunction = - strategy.getRouteFunction(DefaultTemplateEnum.SINGLE_PAGE.getValue(), - null); + void shouldResponse404IfNoPermalinkFound() { + createClient().get() + .uri("/nothing") + .exchange() + .expectStatus().isNotFound(); + } - WebTestClient client = WebTestClient.bindToRouterFunction(routeFunction) - .handlerStrategies(HandlerStrategies.builder() - .viewResolver(viewResolver) - .build()) - .build(); - - when(viewResolver.resolveViewName(eq(DefaultTemplateEnum.SINGLE_PAGE.getValue()), any())) - .thenReturn(Mono.just(new EmptyView())); - - client.get() + @Test + void shouldResponse200IfPermalinkFound() { + when(permalinkIndexer.getPermalinks(any())) + .thenReturn(List.of("/fake-slug")); + when(permalinkIndexer.getNameByPermalink(any(), eq("/fake-slug"))) + .thenReturn("fake-name"); + createClient().get() .uri("/fake-slug") .exchange() .expectStatus() .isOk(); - client.get() - .uri("/nothing") - .exchange() - .expectStatus() - .isEqualTo(HttpStatus.NOT_FOUND); + verify(permalinkIndexer).getNameByPermalink(any(), eq("/fake-slug")); } -} \ No newline at end of file + + @Test + void shouldResponse200IfSlugNameContainsSpecialChars() { + when(permalinkIndexer.getPermalinks(any())) + .thenReturn(List.of("/fake%20/%20slug")); + when(permalinkIndexer.getNameByPermalink(any(), eq("/fake%20/%20slug"))) + .thenReturn("fake-name"); + createClient().get() + .uri("/fake / slug") + .exchange() + .expectStatus().isOk(); + verify(permalinkIndexer).getNameByPermalink(any(), eq("/fake%20/%20slug")); + } + + @Test + void shouldResponse200IfSlugNameContainsChineseChars() { + when(permalinkIndexer.getPermalinks(any())) + .thenReturn(List.of("/%E4%B8%AD%E6%96%87")); + when(permalinkIndexer.getNameByPermalink(any(), eq("/%E4%B8%AD%E6%96%87"))) + .thenReturn("fake-name"); + createClient().get() + .uri("/中文") + .exchange() + .expectStatus().isOk(); + verify(permalinkIndexer).getNameByPermalink(any(), eq("/%E4%B8%AD%E6%96%87")); + } + + WebTestClient createClient() { + var routeFunction = strategy.getRouteFunction(SINGLE_PAGE.getValue(), null); + + return WebTestClient.bindToRouterFunction(routeFunction) + .handlerStrategies(HandlerStrategies.builder() + .viewResolver(viewResolver) + .build()) + .build(); + } +}