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

username/display_name/video_channel_name min length 1 and max length 50 #1265

Merged
merged 4 commits into from
Dec 7, 2018
Merged

username/display_name/video_channel_name min length 1 and max length 50 #1265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11d86c3
make username, display_name and video_channel_name min length 1 and m…
McFlat Oct 17, 2018
5a04ba3
fix merge conflict
McFlat Oct 19, 2018
286949d
change 1 characters to 1 character
McFlat Oct 19, 2018
4c8aff2
Merge branch 'develop' of github.com:Chocobozzz/PeerTube into feature…
McFlat Oct 19, 2018
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
20 changes: 10 additions & 10 deletions client/src/app/shared/forms/form-validators/user-validators.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ export class UserValidatorsService {
this.USER_USERNAME = {
VALIDATORS: [
Validators.required,
Validators.minLength(3),
Validators.maxLength(20),
Validators.pattern(/^[a-z0-9._]+$/)
Validators.minLength(1),
Validators.maxLength(50),
Validators.pattern(/^[a-z0-9][a-z0-9-._]*$/)
],
MESSAGES: {
'required': this.i18n('Username is required.'),
'minlength': this.i18n('Username must be at least 3 characters long.'),
'maxlength': this.i18n('Username cannot be more than 20 characters long.'),
'pattern': this.i18n('Username should be only lowercase alphanumeric characters.')
'minlength': this.i18n('Username must be at least 1 character long.'),
'maxlength': this.i18n('Username cannot be more than 50 characters long.'),
'pattern': this.i18n('Username should be lowercase alphanumeric; dots, dashes and underscores are allowed.')
}
}

Expand Down Expand Up @@ -88,13 +88,13 @@ export class UserValidatorsService {
this.USER_DISPLAY_NAME = {
VALIDATORS: [
Validators.required,
Validators.minLength(3),
Validators.maxLength(120)
Validators.minLength(1),
Validators.maxLength(50)
],
MESSAGES: {
'required': this.i18n('Display name is required.'),
'minlength': this.i18n('Display name must be at least 3 characters long.'),
'maxlength': this.i18n('Display name cannot be more than 120 characters long.')
'minlength': this.i18n('Display name must be at least 1 character long.'),
'maxlength': this.i18n('Display name cannot be more than 50 characters long.')
}
}

Expand Down
20 changes: 10 additions & 10 deletions client/src/app/shared/forms/form-validators/video-channel-validators.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,28 @@ export class VideoChannelValidatorsService {
this.VIDEO_CHANNEL_NAME = {
VALIDATORS: [
Validators.required,
Validators.minLength(3),
Validators.maxLength(20),
Validators.pattern(/^[a-z0-9._]+$/)
Validators.minLength(1),
Validators.maxLength(50),
Validators.pattern(/^[a-z0-9][a-z0-9-._]*$/)
Copy link
Owner

Choose a reason for hiding this comment

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

- char is not accepted by mastodon i think

Copy link
Contributor Author

@McFlat McFlat Nov 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'll have to fix that, or maybe we can have mastodon accept it?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is two issues at least:

but that doesn't seems to be planned for the moment..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To me it seems that mastodon accepts everything accept the . char, which it removes if it sees it.

https://github.com/tootsuite/mastodon/blob/master/app/validators/unique_username_validator.rb#L7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

screen shot 2018-11-25 at 6 04 44 am

Copy link
Contributor Author

Choose a reason for hiding this comment

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

screen shot 2018-11-25 at 6 06 07 am

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd say it's safe to merge the pull request without any other changes to it

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still not safe, see my previous comment.
You are only showing basic HTML5 validation (field filled or not).
Validate the form and it will refuse it:
image

There is two issues still opened, and Gargron stated three days ago that dashes are not accepted from remote usernames.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a scenario where a user can have spaces in their very long username, even beyond the 50 chars limit we have with this pull request. very-long username-a user likes just because he likes it
screen shot 2018-11-25 at 6 14 14 am

Copy link
Contributor

Choose a reason for hiding this comment

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

No, you didn't submitted the form, the HTML5 shows a green border only because the field is filled and not empty (because of the "required" html attribute).

],
MESSAGES: {
'required': this.i18n('Name is required.'),
'minlength': this.i18n('Name must be at least 3 characters long.'),
'maxlength': this.i18n('Name cannot be more than 20 characters long.'),
'pattern': this.i18n('Name should be only lowercase alphanumeric characters.')
'minlength': this.i18n('Name must be at least 1 character long.'),
'maxlength': this.i18n('Name cannot be more than 50 characters long.'),
'pattern': this.i18n('Name should be lowercase alphanumeric; dots, dashes and underscores are allowed.')
}
}

this.VIDEO_CHANNEL_DISPLAY_NAME = {
VALIDATORS: [
Validators.required,
Validators.minLength(3),
Validators.maxLength(120)
Validators.minLength(1),
Validators.maxLength(50)
],
MESSAGES: {
'required': i18n('Display name is required.'),
'minlength': i18n('Display name must be at least 3 characters long.'),
'maxlength': i18n('Display name cannot be more than 120 characters long.')
'minlength': i18n('Display name must be at least 1 character long.'),
'maxlength': i18n('Display name cannot be more than 50 characters long.')
}
}

Expand Down
6 changes: 3 additions & 3 deletions server/initializers/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,9 @@ const CONFIG = {

const CONSTRAINTS_FIELDS = {
USERS: {
NAME: { min: 3, max: 120 }, // Length
NAME: { min: 1, max: 50 }, // Length
DESCRIPTION: { min: 3, max: 1000 }, // Length
USERNAME: { min: 3, max: 20 }, // Length
USERNAME: { min: 1, max: 50 }, // Length
PASSWORD: { min: 6, max: 255 }, // Length
VIDEO_QUOTA: { min: -1 },
VIDEO_QUOTA_DAILY: { min: -1 },
Expand All @@ -310,7 +310,7 @@ const CONSTRAINTS_FIELDS = {
REASON: { min: 2, max: 300 } // Length
},
VIDEO_CHANNELS: {
NAME: { min: 3, max: 120 }, // Length
NAME: { min: 1, max: 50 }, // Length
DESCRIPTION: { min: 3, max: 1000 }, // Length
SUPPORT: { min: 3, max: 1000 }, // Length
URL: { min: 3, max: 2000 } // Length
Expand Down
8 changes: 4 additions & 4 deletions server/tests/api/check-params/users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,13 @@ describe('Test users API validators', function () {
}

it('Should fail with a too small username', async function () {
const fields = immutableAssign(baseCorrectParams, { username: 'fi' })
const fields = immutableAssign(baseCorrectParams, { username: '' })

await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields })
})

it('Should fail with a too long username', async function () {
const fields = immutableAssign(baseCorrectParams, { username: 'my_super_username_which_is_very_long' })
const fields = immutableAssign(baseCorrectParams, { username: 'super'.repeat(11) })

await makePostBodyRequest({ url: server.url, path, token: server.accessToken, fields })
})
Expand Down Expand Up @@ -541,13 +541,13 @@ describe('Test users API validators', function () {
}

it('Should fail with a too small username', async function () {
const fields = immutableAssign(baseCorrectParams, { username: 'ji' })
const fields = immutableAssign(baseCorrectParams, { username: '' })

await makePostBodyRequest({ url: server.url, path: registrationPath, token: server.accessToken, fields })
})

it('Should fail with a too long username', async function () {
const fields = immutableAssign(baseCorrectParams, { username: 'my_super_username_which_is_very_long' })
const fields = immutableAssign(baseCorrectParams, { username: 'super'.repeat(11) })

await makePostBodyRequest({ url: server.url, path: registrationPath, token: server.accessToken, fields })
})
Expand Down