diff --git a/app/client/src/pages/Editor/AppSettingsPane/AppSettings/PageSettings.tsx b/app/client/src/pages/Editor/AppSettingsPane/AppSettings/PageSettings.tsx index f17c5bc473..cb1bfcad44 100644 --- a/app/client/src/pages/Editor/AppSettingsPane/AppSettings/PageSettings.tsx +++ b/app/client/src/pages/Editor/AppSettingsPane/AppSettings/PageSettings.tsx @@ -33,7 +33,7 @@ import TextLoaderIcon from "../Components/TextLoaderIcon"; import { filterAccentedAndSpecialCharacters, getUrlPreview } from "../Utils"; import type { AppState } from "@appsmith/reducers"; import { getUsedActionNames } from "selectors/actionSelectors"; -import { isNameValid, resolveAsSpaceChar } from "utils/helpers"; +import { isNameValid, toValidPageName } from "utils/helpers"; import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; import { FEATURE_FLAG } from "@appsmith/entities/FeatureFlag"; import { getHasManagePagePermission } from "@appsmith/utils/BusinessFeatures/permissionPageHelpers"; @@ -169,7 +169,7 @@ function PageSettings(props: { page: Page }) { } setIsPageNameValid(isValid); - setPageName(resolveAsSpaceChar(value, 30)); + setPageName(toValidPageName(value)); }; const onPageSlugChange = (value: string) => { diff --git a/app/client/src/pages/Editor/IDE/EditorPane/components/PageElement.tsx b/app/client/src/pages/Editor/IDE/EditorPane/components/PageElement.tsx index b2c7e5bdaf..4e0bec6a99 100644 --- a/app/client/src/pages/Editor/IDE/EditorPane/components/PageElement.tsx +++ b/app/client/src/pages/Editor/IDE/EditorPane/components/PageElement.tsx @@ -18,7 +18,7 @@ import { import { getCurrentApplication } from "@appsmith/selectors/applicationSelectors"; import type { AppState } from "@appsmith/reducers"; import { StyledEntity } from "pages/Editor/Explorer/Common/components"; -import { resolveAsSpaceChar } from "utils/helpers"; +import { toValidPageName } from "utils/helpers"; import { updatePage } from "actions/pageActions"; import { useGetPageFocusUrl } from "pages/Editor/IDE/hooks"; import AnalyticsUtil from "utils/AnalyticsUtil"; @@ -113,7 +113,7 @@ const PageElement = ({ isDefaultExpanded={isCurrentPage} key={page.pageId} name={page.pageName} - onNameEdit={resolveAsSpaceChar} + onNameEdit={toValidPageName} ref={ref} searchKeyword={""} step={0} diff --git a/app/client/src/utils/helpers.tsx b/app/client/src/utils/helpers.tsx index b07139d724..9241e8b6fd 100644 --- a/app/client/src/utils/helpers.tsx +++ b/app/client/src/utils/helpers.tsx @@ -352,18 +352,9 @@ function getWidgetElementToScroll( } } -export const resolveAsSpaceChar = (value: string, limit?: number) => { - // ensures that all special characters are disallowed - // while allowing all utf-8 characters - const removeSpecialCharsRegex = - /`|\~|\!|\@|\#|\$|\%|\^|\&|\*|\(|\)|\+|\=|\[|\{|\]|\}|\||\\|\'|\<|\,|\.|\>|\?|\/|\""|\;|\:|\s/; - const duplicateSpaceRegex = /\s+/; - return value - .split(removeSpecialCharsRegex) - .join(" ") - .split(duplicateSpaceRegex) - .join(" ") - .slice(0, limit || 30); +export const toValidPageName = (value: string) => { + // Ensure that `/`, `\` and `:` are not allowed in page names, aligning with server-side validation. + return value.replaceAll(/[/\\:]+/g, "").slice(0, 30); }; export const PLATFORM_OS = { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageUpdateDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageUpdateDTO.java index 120aeac683..2b9c00d4c7 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageUpdateDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageUpdateDTO.java @@ -5,8 +5,8 @@ import jakarta.validation.constraints.Size; public record PageUpdateDTO( @Pattern(regexp = "[^/\\\\:]+", message = "/, \\, : not allowed in page names") @Size(max = 30) String name, - String icon, - @Pattern(regexp = "[-\\w]+") String customSlug) { + @Pattern(regexp = "[-a-z]+") @Size(max = 20) String icon, + @Pattern(regexp = "[-\\w]*") String customSlug) { public PageDTO toPageDTO() { final PageDTO page = new PageDTO(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/PageControllerTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/PageControllerTest.java index 2ad066bc2a..365d84bedc 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/PageControllerTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/PageControllerTest.java @@ -1,6 +1,7 @@ package com.appsmith.server.controllers; import com.appsmith.server.configurations.SecurityTestConfig; +import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.newpages.base.NewPageService; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -14,10 +15,17 @@ import org.springframework.http.MediaType; import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.web.reactive.function.BodyInserters; +import reactor.core.publisher.Mono; import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @SpringBootTest @@ -41,7 +49,7 @@ public class PageControllerTest { @ValueSource( strings = { "../mal", "..\\mal", "/mal", "C:\\mal", "mal/", "/mal/", "\\mal", "mal\\", "\\mal\\", ":mal", ":mal/", - ":mal\\" + ":mal\\", "", }) @WithMockUser void invalidName(String name) { @@ -58,4 +66,55 @@ public class PageControllerTest { verifyNoInteractions(newPageService); } + + @ParameterizedTest + @ValueSource( + strings = { + "../mal", + "..\\mal", + "/mal", + "C:\\mal", + "mal/", + "/mal/", + "\\mal", + "mal\\", + "\\mal\\", + ":mal", + ":mal/", + ":mal\\", + "spaced content", + "newline\ncontent", + " untrimmed ", + }) + @WithMockUser + void invalidCustomSlug(String slug) { + client.put() + .uri("/api/v1/pages/abcdef") + .contentType(MediaType.APPLICATION_JSON) + .body(BodyInserters.fromValue(Map.of("customSlug", slug))) + .exchange() + .expectStatus() + .isBadRequest() + .expectBody() + .jsonPath("$.errorDisplay") + .value(s -> assertThat((String) s).contains("Validation Failure")); + + verifyNoInteractions(newPageService); + } + + @Test + @WithMockUser + void emptyCustomSlugShouldBeOkay() { + doReturn(Mono.just(new PageDTO())) + .when(newPageService) + .updatePageByDefaultPageIdAndBranch(anyString(), any(), anyString()); + + client.put() + .uri("/api/v1/pages/abcdef") + .contentType(MediaType.APPLICATION_JSON) + .body(BodyInserters.fromValue(Map.of("customSlug", ""))) + .exchange(); + + verify(newPageService, times(1)).updatePageByDefaultPageIdAndBranch(eq("abcdef"), any(), eq(null)); + } }