Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated with changes #1598

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

updated with changes #1598

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
727de08
updated with changes
yoyounik Jan 21, 2025
c54b2c6
Update validation.js with new changes
yoyounik Jan 21, 2025
ca2be2e
Updated regex pattern
yoyounik Jan 21, 2025
56695b0
Updates
yoyounik Jan 22, 2025
883c47b
Added new pattern
yoyounik Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Client/src/Validation/validation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ const nameSchema = joi
.string()
.max(50)
.trim()
.pattern(/^[A-Za-z]+$/)
.pattern(/^[\p{L}]+(?:[\p{L}\s]+)*$/u)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Yo, the name validation needs more flexibility, dawg! 🔥

The current regex pattern doesn't support common name characters like hyphens (e.g., "Anne-Marie") and apostrophes (e.g., "O'Connor"). Let's make it more inclusive!

Here's a fix that'll make it work better:

-	.pattern(/^[\p{L}]+(?:[\p{L}\s]+)*$/u)
+	.pattern(/^[\p{L}]+(?:[\p{L}\s'\-]+)*$/u)

Also applies to: 14-14

.messages({
"string.empty": "Name is required",
"string.max": "Name must be less than 50 characters long",
"string.pattern.base": "Name must contain only letters",
"string.pattern.base": "Name must contain only letters and spaces.",
});

const passwordSchema = joi
Expand Down
24 changes: 20 additions & 4 deletions Server/validation/joi.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,17 @@ const registrationBodyValidation = joi.object({
firstName: joi
.string()
.required()
.pattern(/^[A-Za-z]+$/),
.pattern(/^[\p{L}]+(?:[\p{L}\s]+)*$/u)
.messages({
"string.pattern.base": "First name must contain only letters and spaces",
}),
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Yo, the regex is dropping the ball on legit names! 🎯

The current pattern is too restrictive and fails to validate common name formats:

  • Compound names with hyphens (Mary-Jane)
  • Names with apostrophes (O'Connor)
  • Names with suffixes (Smith Jr.)

While it correctly handles Unicode letters (José, María, 안녕하세요), it needs to be updated to support these common name components.

🔗 Analysis chain

Verify the regex pattern coverage! 🎯

Let's make sure our regex pattern handles all the edge cases correctly.

Also applies to: 50-53, 81-92

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Test the name validation regex against various test cases

# Create a test file with sample names
cat << 'EOF' > test_names.txt
John
Mary-Jane
O'Connor
José
María
안녕하세요
Zhang-Wei
Jean-François
Smith Jr.
EOF

# Use awk to test the regex pattern against each name
awk '
BEGIN {
    pattern = "^[[:alpha:]]+([[:alpha:][:space:]'\''\\-]+)*$"
}
{
    if ($0 ~ pattern) {
        print "PASS:", $0
    } else {
        print "FAIL:", $0
    }
}' test_names.txt

Length of output: 329

Script:

#!/bin/bash
# Create a Node.js script to test the actual regex pattern
cat << 'EOF' > test_regex.js
const names = [
  'John',
  'Mary-Jane',
  "O'Connor",
  'José',
  'María',
  '안녕하세요',
  'Zhang-Wei',
  'Jean-François',
  'Smith Jr.',
  'John123',
  'Mary!',
  '   Space',
  'Dot.Name'
];

const pattern = /^[\p{L}]+(?:[\p{L}\s]+)*$/u;
console.log('Testing names against pattern:', pattern);
console.log('----------------------------------------');
names.forEach(name => {
  console.log(`${name.padEnd(20)} : ${pattern.test(name) ? 'PASS' : 'FAIL'}`);
});
EOF

# Use Node.js to run the test
node test_regex.js

Length of output: 491

lastName: joi
.string()
.required()
.pattern(/^[A-Za-z]+$/),
.pattern(/^[\p{L}]+(?:[\p{L}\s]+)*$/u)
.messages({
"string.pattern.base": "Last name must contain only letters and spaces",
}),
email: joi
.string()
.email()
Expand Down Expand Up @@ -72,8 +78,18 @@ const editUserParamValidation = joi.object({
});

const editUserBodyValidation = joi.object({
firstName: joi.string().pattern(/^[A-Za-z]+$/),
lastName: joi.string().pattern(/^[A-Za-z]+$/),
firstName: joi
.string()
.pattern(/^[\p{L}]+(?:[\p{L}\s]+)*$/u)
.messages({
"string.pattern.base": "First name must contain only letters and spaces",
}),
lastName: joi
.string()
.pattern(/^[\p{L}]+(?:[\p{L}\s]+)*$/u)
.messages({
"string.pattern.base": "Last name must contain only letters and spaces",
}),
profileImage: joi.any(),
newPassword: joi.string().min(8).pattern(passwordPattern),
password: joi.string().min(8).pattern(passwordPattern),
Expand Down