diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/PageControllerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/PageControllerCE.java index a16c31cb0b..a40f17d7b3 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/PageControllerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/PageControllerCE.java @@ -8,6 +8,7 @@ import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.CRUDPageResourceDTO; import com.appsmith.server.dtos.CRUDPageResponseDTO; import com.appsmith.server.dtos.PageDTO; +import com.appsmith.server.dtos.PageUpdateDTO; import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.newpages.base.NewPageService; import com.appsmith.server.services.ApplicationPageService; @@ -166,7 +167,7 @@ public class PageControllerCE { @PutMapping("/{defaultPageId}") public Mono> updatePage( @PathVariable String defaultPageId, - @RequestBody PageDTO resource, + @RequestBody @Valid PageUpdateDTO resource, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) { log.debug("Going to update page with id: {}, branchName: {}", defaultPageId, branchName); return newPageService 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 new file mode 100644 index 0000000000..120aeac683 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageUpdateDTO.java @@ -0,0 +1,18 @@ +package com.appsmith.server.dtos; + +import jakarta.validation.constraints.Pattern; +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) { + + public PageDTO toPageDTO() { + final PageDTO page = new PageDTO(); + page.setName(name); + page.setIcon(icon); + page.setCustomSlug(customSlug); + return page; + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java index ae8b5105f6..b749eb1ee1 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/TextUtils.java @@ -15,14 +15,14 @@ public class TextUtils { * NON_LATIN regex matches any letter that is not a ASCII i.e. A-Za-z0-9, `-` and `_` * It'll match the unicode letters. * */ - private static final Pattern NON_LATIN = Pattern.compile("[^\\w_-]"); + private static final Pattern NON_LATIN = Pattern.compile("[^\\w-]"); - /* - * The SEPARATORS pattern matches those characters which cane be replaced with `-` - * This includes Punctuation characters, `&`, space and not `-` itself. Details on `Punct` - * http://www.gnu.org/software/grep/manual/html_node/Character-Classes-and-Bracket-Expressions.html - * */ - private static final Pattern SEPARATORS = Pattern.compile("[\\s\\p{Punct}&&[^-]]"); + /** + * The SEPARATORS pattern matches those characters which will be replaced with `-`. This includes spaces and + * punctuation characters. + * Details on {@code Punct}. + */ + private static final Pattern SEPARATORS = Pattern.compile("[\\s\\p{Punct}]+"); /** * Creates URL safe text aka slug from the input text. It supports english locale only. @@ -36,7 +36,8 @@ public class TextUtils { if (inputText == null) { return ""; } - // remove all the separators with a `-` + + // Replace all spaces and punctuation with a `-`. String noseparators = SEPARATORS.matcher(inputText).replaceAll("-"); String normalized = Normalizer.normalize(noseparators, Normalizer.Form.NFD); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCE.java index 5e8ab93fa5..9dd7ec150f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCE.java @@ -6,6 +6,7 @@ import com.appsmith.server.domains.Layout; import com.appsmith.server.domains.NewPage; import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.PageDTO; +import com.appsmith.server.dtos.PageUpdateDTO; import com.appsmith.server.services.CrudService; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -62,7 +63,7 @@ public interface NewPageServiceCE extends CrudService { Mono updatePage(String pageId, PageDTO page); - Mono updatePageByDefaultPageIdAndBranch(String defaultPageId, PageDTO page, String branchName); + Mono updatePageByDefaultPageIdAndBranch(String defaultPageId, PageUpdateDTO page, String branchName); Mono save(NewPage page); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java index 369b20a251..98006d9bea 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java @@ -13,6 +13,7 @@ import com.appsmith.server.domains.NewPage; import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.dtos.PageNameIdDTO; +import com.appsmith.server.dtos.PageUpdateDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.ResponseUtils; @@ -527,10 +528,11 @@ public class NewPageServiceCEImpl extends BaseService updatePageByDefaultPageIdAndBranch(String defaultPageId, PageDTO page, String branchName) { + public Mono updatePageByDefaultPageIdAndBranch( + String defaultPageId, PageUpdateDTO page, String branchName) { return repository .findPageByBranchNameAndDefaultPageId(branchName, defaultPageId, pagePermission.getEditPermission()) - .flatMap(newPage -> updatePage(newPage.getId(), page)) + .flatMap(newPage -> updatePage(newPage.getId(), page.toPageDTO())) .map(responseUtils::updatePageDTOWithDefaultResources); } 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 new file mode 100644 index 0000000000..2ad066bc2a --- /dev/null +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/controllers/PageControllerTest.java @@ -0,0 +1,61 @@ +package com.appsmith.server.controllers; + +import com.appsmith.server.configurations.SecurityTestConfig; +import com.appsmith.server.newpages.base.NewPageService; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +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 java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verifyNoInteractions; + +@SpringBootTest +@AutoConfigureWebTestClient +@Import({SecurityTestConfig.class}) +public class PageControllerTest { + + @Autowired + private WebTestClient client; + + @MockBean + private NewPageService newPageService; + + @Test + @WithMockUser + void noBody() { + client.put().uri("/api/v1/pages/abcdef").exchange().expectStatus().isBadRequest(); + } + + @ParameterizedTest + @ValueSource( + strings = { + "../mal", "..\\mal", "/mal", "C:\\mal", "mal/", "/mal/", "\\mal", "mal\\", "\\mal\\", ":mal", ":mal/", + ":mal\\" + }) + @WithMockUser + void invalidName(String name) { + client.put() + .uri("/api/v1/pages/abcdef") + .contentType(MediaType.APPLICATION_JSON) + .body(BodyInserters.fromValue(Map.of("name", name))) + .exchange() + .expectStatus() + .isBadRequest() + .expectBody() + .jsonPath("$.errorDisplay") + .value(s -> assertThat((String) s).contains("Validation Failure")); + + verifyNoInteractions(newPageService); + } +}