diff --git a/lib/Travelynx/Controller/Account.pm b/lib/Travelynx/Controller/Account.pm index 31ba434..9c161e0 100644 --- a/lib/Travelynx/Controller/Account.pm +++ b/lib/Travelynx/Controller/Account.pm @@ -85,8 +85,8 @@ sub register { return; } - if ( not length($user) ) { - $self->render( 'register', invalid => 'user_empty' ); + if ( my $error = $self->users->is_name_invalid( name => $user ) ) { + $self->render( 'register', invalid => $error ); return; } @@ -95,16 +95,6 @@ sub register { return; } - if ( $user !~ m{ ^ [0-9a-zA-Z_-]+ $ }x ) { - $self->render( 'register', invalid => 'user_format' ); - return; - } - - if ( $self->users->user_name_exists( name => $user ) ) { - $self->render( 'register', invalid => 'user_collision' ); - return; - } - if ( $self->users->mail_is_blacklisted( email => $email ) ) { $self->render( 'register', invalid => 'mail_blacklisted' ); return; @@ -485,13 +475,8 @@ sub change_name { return; } - if ( not length($new_name) ) { - $self->render( 'change_name', invalid => 'user_empty' ); - return; - } - - if ( $new_name !~ m{ ^ [0-9a-zA-Z_-]+ $ }x ) { - $self->render( 'change_name', invalid => 'user_format' ); + if ( my $error = $self->users->is_name_invalid( name => $new_name ) ) { + $self->render( 'change_name', invalid => $error ); return; } @@ -500,16 +485,10 @@ sub change_name { return; } - # This call is technically superfluous. The users table has a unique - # constraint on the "name" column, so having two users with the same name - # is not possible. However, to minimize the number of failed SQL - # queries, we first do a select check here and only attempt an update - # if it succeeded. - if ( $self->users->user_name_exists( name => $new_name ) ) { - $self->render( 'change_name', invalid => 'user_collision' ); - return; - } - + # The users table has a unique constraint on the "name" column, so having + # two users with the same name is not possible. The race condition + # between the user_name_exists check in is_name_invalid and this + # change_name call is harmless. my $success = $self->users->change_name( uid => $self->current_user->{id}, name => $new_name diff --git a/lib/Travelynx/Model/Users.pm b/lib/Travelynx/Model/Users.pm index 8bd60f0..535b938 100644 --- a/lib/Travelynx/Model/Users.pm +++ b/lib/Travelynx/Model/Users.pm @@ -201,6 +201,32 @@ sub change_mail_with_token { return; } +sub is_name_invalid { + my ( $self, %opt ) = @_; + my $db = $opt{db} // $self->{pg}->db; + my $name = $opt{name}; + + if ( not length($name) ) { + return 'user_empty'; + } + + if ( $name !~ m{ ^ [0-9a-zA-Z_-]+ $ }x ) { + return 'user_format'; + } + + if ( + $self->user_name_exists( + db => $db, + name => $name + ) + ) + { + return 'user_collision'; + } + + return; +} + sub change_name { my ( $self, %opt ) = @_; my $db = $opt{db} // $self->{pg}->db;