Skip to content

Conversation

@tomako
Copy link
Contributor

@tomako tomako commented Jan 31, 2025

This PR is about adding length validation to fields on registration and user profile forms to prevent abusing and sending garbage data to the application.
The following fields have got length validation:

  • first name: 250
  • last name: 250
  • username: 100
  • password: 100
  • affiliation: 250
  • address: 500
  • phone: 100
  • registration request comment: 500

syncedValues={syncedValues}
lockedFields={lockedFields}
lockedFieldMessage={lockedFieldMessage}
validate={value => value !== undefined && v.maxLength(250)(value)}
Copy link
Member

Choose a reason for hiding this comment

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

why not return early in maxLength if the value is undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because none of the validators deals with the undefined value.
However I noticed it causes issue when the form is loaded the first time. Only for not required fields. Adding this check here solved the problem but I'm not confident it's the best approach.

@ThiefMaster ThiefMaster force-pushed the add_length_validation_to_sign_up_form_fields branch from 4d9a601 to dda5911 Compare March 20, 2025 16:44
@ThiefMaster ThiefMaster force-pushed the add_length_validation_to_sign_up_form_fields branch from dda5911 to c426be4 Compare March 25, 2025 15:41
@ThiefMaster ThiefMaster enabled auto-merge (squash) March 25, 2025 15:42
@ThiefMaster ThiefMaster added this to the v3.3 milestone Mar 25, 2025
@ThiefMaster ThiefMaster merged commit 6bfb8df into indico:master Mar 25, 2025
10 checks passed
@ThiefMaster ThiefMaster deleted the add_length_validation_to_sign_up_form_fields branch March 25, 2025 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants