Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

EVG-20817: Restrict special characters in project name & ID input #2099

Merged
merged 7 commits into from
Oct 13, 2023

Conversation

SupaJoon
Copy link
Contributor

EVG-20817

Description

The validation matches that in EVG so none of the characters require escaping in the URL.

Screenshots

Screenshot 2023-10-12 at 2 35 23 PM

Testing

Evergreen PR

@SupaJoon SupaJoon requested a review from a team October 12, 2023 19:06
@cypress
Copy link

cypress bot commented Oct 12, 2023

1 failed test on run #13273 ↗︎

1 600 7 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

add formatting
Project: Spruce Commit: 616e40b30d
Status: Failed Duration: 18:50 💡
Started: Oct 13, 2023 5:37 PM Ended: Oct 13, 2023 5:56 PM

Review all test suite changes for PR #2099 ↗︎

Comment on lines 140 to 141
const validateNoSpecialCharacters = (str: string): boolean => {
const noSpecialCharacters = /^[0-9a-zA-Z-._~()]*$/;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be cool to define these special chars as a var that can be used in the error message so that any updates are reflected. I think this would work

Suggested change
const validateNoSpecialCharacters = (str: string): boolean => {
const noSpecialCharacters = /^[0-9a-zA-Z-._~()]*$/;
const specialChars = "-._~()";
const validateNoSpecialCharacters = (str: string): boolean => {
const noSpecialCharacters = new RegExp(`^[0-9a-zA-Z${specialChars}]*$`);

}

return `${paths.project}/${projectId}/${PageNames.Settings}/${tab}`;
const encodedProjectId = encodeURIComponent(projectId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description says we don't need to encode the URL, should this be included

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for backwards compatibility, I'll make a comment in the code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice -- since the ticket mentions that we'll work with project admins to change their project identifiers, can we update the comment to note that we'll delete this when that is done?

@SupaJoon SupaJoon requested a review from sophstad October 13, 2023 14:59
}

return `${paths.project}/${projectId}/${PageNames.Settings}/${tab}`;
const encodedProjectId = encodeURIComponent(projectId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice -- since the ticket mentions that we'll work with project admins to change their project identifiers, can we update the comment to note that we'll delete this when that is done?

@SupaJoon SupaJoon merged commit 13b55b0 into evergreen-ci:main Oct 13, 2023
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants