chore: Page name validation closer to file name validation (#32830)
1. Introducing a custom validation annotation, `@FileName` to be used on fields that are used as file/folder names as part of the product. 2. Updated the validation logic to only accept valid file names on Linux, macOS as well as on Windows. /ok-to-test tags="@tag.Settings, @tag.IDE" <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/8784587443> > Commit: a13f700d490845a9d3c53d0da9c1a763fde6be9b > Cypress dashboard url: <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=8784587443&attempt=1" target="_blank">Click here!</a> <!-- end of auto-generated comment: Cypress test results --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced page name validation to include more special characters and reserved names, ensuring compatibility and preventing errors. - **Bug Fixes** - Updated regular expression for replacing disallowed characters in page names to improve system stability and user experience. - **Tests** - Expanded testing for invalid page names to cover a broader range of scenarios, enhancing reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
94ad2b9eb2
commit
71867f9bcd
|
|
@ -354,7 +354,7 @@ function getWidgetElementToScroll(
|
|||
|
||||
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);
|
||||
return value.replaceAll(/[\\/:<>"|?*\x00-\x1f]+/g, "").slice(0, 30);
|
||||
};
|
||||
|
||||
export const PLATFORM_OS = {
|
||||
|
|
|
|||
|
|
@ -1,10 +1,11 @@
|
|||
package com.appsmith.server.dtos;
|
||||
|
||||
import com.appsmith.server.meta.validations.FileName;
|
||||
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,
|
||||
@FileName(message = "Page names must be valid file names") @Size(max = 30) String name,
|
||||
@Pattern(regexp = "[-a-z]+") @Size(max = 20) String icon,
|
||||
@Pattern(regexp = "[-\\w]*") String customSlug,
|
||||
Boolean isHidden) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,28 @@
|
|||
package com.appsmith.server.meta.validations;
|
||||
|
||||
import jakarta.validation.Constraint;
|
||||
import jakarta.validation.Payload;
|
||||
|
||||
import java.lang.annotation.Documented;
|
||||
import java.lang.annotation.ElementType;
|
||||
import java.lang.annotation.Retention;
|
||||
import java.lang.annotation.RetentionPolicy;
|
||||
import java.lang.annotation.Target;
|
||||
|
||||
/**
|
||||
* Validate field as a valid file name. Nulls are treated as valid by default, but can be changed with
|
||||
* {@link FileName#isNullValid()}.
|
||||
*/
|
||||
@Documented
|
||||
@Constraint(validatedBy = {FileNameValidator.class})
|
||||
@Target({ElementType.FIELD, ElementType.METHOD})
|
||||
@Retention(RetentionPolicy.RUNTIME)
|
||||
public @interface FileName {
|
||||
String message() default "Invalid file name";
|
||||
|
||||
Class<?>[] groups() default {};
|
||||
|
||||
Class<? extends Payload>[] payload() default {};
|
||||
|
||||
boolean isNullValid() default true;
|
||||
}
|
||||
|
|
@ -0,0 +1,43 @@
|
|||
package com.appsmith.server.meta.validations;
|
||||
|
||||
import jakarta.validation.ConstraintValidator;
|
||||
import jakarta.validation.ConstraintValidatorContext;
|
||||
|
||||
import java.util.Set;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
public class FileNameValidator implements ConstraintValidator<FileName, String> {
|
||||
|
||||
// Checks built to exclude characters listed in https://stackoverflow.com/a/31976060/151048.
|
||||
private static final Pattern DISALLOWED_CHARS = Pattern.compile("[\\\\/:<>\"|?*\\x00-\\x1f]");
|
||||
|
||||
// These are not allowed on Windows, even with an extension.
|
||||
private static final Set<String> DISALLOWED_NAMES = Set.of(
|
||||
"CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1",
|
||||
"LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9");
|
||||
|
||||
private boolean isNullValid;
|
||||
|
||||
@Override
|
||||
public void initialize(FileName constraintAnnotation) {
|
||||
ConstraintValidator.super.initialize(constraintAnnotation);
|
||||
isNullValid = constraintAnnotation.isNullValid();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean isValid(String value, ConstraintValidatorContext context) {
|
||||
if (value == null) {
|
||||
return isNullValid;
|
||||
}
|
||||
|
||||
if (value.isBlank() || value.startsWith(".") || value.endsWith(".") || value.length() > 255) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (DISALLOWED_CHARS.matcher(value).find()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return !DISALLOWED_NAMES.contains(value.split("\\.")[0].toUpperCase());
|
||||
}
|
||||
}
|
||||
|
|
@ -48,8 +48,103 @@ public class PageControllerTest {
|
|||
@ParameterizedTest
|
||||
@ValueSource(
|
||||
strings = {
|
||||
"../mal", "..\\mal", "/mal", "C:\\mal", "mal/", "/mal/", "\\mal", "mal\\", "\\mal\\", ":mal", ":mal/",
|
||||
":mal\\", "",
|
||||
"../mal",
|
||||
"..\\mal",
|
||||
"/mal",
|
||||
"C:\\mal",
|
||||
"mal/",
|
||||
"/mal/",
|
||||
"\\mal",
|
||||
"mal\\",
|
||||
"\\mal\\",
|
||||
":mal",
|
||||
":mal/",
|
||||
":mal\\",
|
||||
"",
|
||||
" ",
|
||||
"\t",
|
||||
"<",
|
||||
">",
|
||||
"\"",
|
||||
"|",
|
||||
"?",
|
||||
"*",
|
||||
"\0",
|
||||
"\1",
|
||||
"\2",
|
||||
"\3",
|
||||
"\4",
|
||||
"\5",
|
||||
"\6",
|
||||
"\7",
|
||||
"\10",
|
||||
"\11",
|
||||
"\12",
|
||||
"\13",
|
||||
"\14",
|
||||
"\15",
|
||||
"\16",
|
||||
"\17",
|
||||
"\20",
|
||||
"\21",
|
||||
"\22",
|
||||
"\23",
|
||||
"\24",
|
||||
"\25",
|
||||
"\26",
|
||||
"\27",
|
||||
"\30",
|
||||
"\31",
|
||||
"\32",
|
||||
"\33",
|
||||
"\34",
|
||||
"\35",
|
||||
"\36",
|
||||
"\37",
|
||||
"CON",
|
||||
"PRN",
|
||||
"AUX",
|
||||
"NUL",
|
||||
"COM1",
|
||||
"COM2",
|
||||
"COM3",
|
||||
"COM4",
|
||||
"COM5",
|
||||
"COM6",
|
||||
"COM7",
|
||||
"COM8",
|
||||
"COM9",
|
||||
"LPT1",
|
||||
"LPT2",
|
||||
"LPT3",
|
||||
"LPT4",
|
||||
"LPT5",
|
||||
"LPT6",
|
||||
"LPT7",
|
||||
"LPT8",
|
||||
"LPT9",
|
||||
"CON.txt",
|
||||
"PRN.txt",
|
||||
"AUX.txt",
|
||||
"NUL.txt",
|
||||
"COM1.txt",
|
||||
"COM2.txt",
|
||||
"COM3.txt",
|
||||
"COM4.txt",
|
||||
"COM5.txt",
|
||||
"COM6.txt",
|
||||
"COM7.txt",
|
||||
"COM8.txt",
|
||||
"COM9.txt",
|
||||
"LPT1.txt",
|
||||
"LPT2.txt",
|
||||
"LPT3.txt",
|
||||
"LPT4.txt",
|
||||
"LPT5.txt",
|
||||
"LPT6.txt",
|
||||
"LPT7.txt",
|
||||
"LPT8.txt",
|
||||
"LPT9.txt",
|
||||
})
|
||||
@WithMockUser
|
||||
void invalidName(String name) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user