fix: Input validations on page update API #32692
Conversation
Failed server tests
|
WalkthroughWalkthroughThe recent update enhances page management in the Appsmith server by introducing the Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| public record PageUpdateDTO( | ||
| @Pattern(regexp = "[^/\\\\:]+", message = "/, \\, : not allowed in page names") @Size(max = 30) String name, | ||
| String icon, | ||
| @Pattern(regexp = "[-\\w]+") String customSlug) { |
There was a problem hiding this comment.
Tip
Codebase Verification
The regex pattern for customSlug does not strictly enforce the intended restrictions of only allowing hyphens, word characters, and underscores. The test results indicate that it also matches characters like periods and asterisks, which should not be allowed. Here are the recommended actions:
- Adjust the regex pattern to
^[-\w]+$to ensure it only matches strings composed entirely of hyphens, word characters (alphanumeric and underscores), and does not allow other characters.
Analysis chain
Validate the regex pattern for customSlug to ensure it only allows hyphens, word characters, and underscores.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the regex pattern for 'customSlug' by testing it against various inputs.
echo -e "slug-1\nslug_2\nslug.3\nslug*4" | grep -P "[-\\w]+"
Length of output: 89
There was a problem hiding this comment.
@sharat87 is this suggestion from code rabbit worth looking at?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@NilanshBansal, yep, already confirming it. My hunch is that its not valid, but am trying to see if I can add a test to verify this.
There was a problem hiding this comment.
Yeah we're good. The Spring validation checks for a match, not a find. What @coderabbitai verified with is with a find, not a match.
Just pushed additional test cases to verify this as well.
There was a problem hiding this comment.
@sharat87, thank you for the clarification regarding the regex validation behavior in Spring. It's a valuable distinction between a complete match and a partial find, and I appreciate the additional test cases you've added to confirm this behavior. I'll keep this in mind for future reviews!
New Learnings Added
User: sharat87"
URL: https://github.com/appsmithorg/appsmith/pull/32692
Timestamp: 2024-04-18T04:23:38.058Z
Learning: In Spring validation, regex checks for a complete match (`match`) rather than finding a match within a string (`find`). This distinction affects how regex patterns are interpreted in validation scenarios.
Note: Learnings are effective only in the context of specific code segments. To apply general coding guidelines, please configure review instructions.
Slack thread with details. Not including details here.
Sanity passes on EE.
/ok-to-test tags="@tag.Sanity"
Warning
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/8704837594
Commit: fb1724a
Cypress dashboard url: Click here!
It seems like no tests ran 😔. We are not able to recognize it, please check workflow here.
Summary by CodeRabbit