From 764a391d196287a9400ee597019c3e5207c5a5f0 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Wed, 21 May 2008 07:27:07 -0400 Subject: [PATCH] validation in form handlers Moved validation code from classes to form handlers. Probably better in the classes, but I can't quite grok the validate() method in DB_DataObject, so for now I'm going to do it the old-fashioned way. darcs-hash:20080521112707-84dde-38e27199b977ae81171b8391fbdb93ebb54494f9.gz --- actions/newnotice.php | 19 ++++---- actions/profilesettings.php | 93 ++++++++++++++++++++++++++----------- actions/register.php | 30 ++++++------ doc/TODO | 10 ++-- lib/action.php | 5 ++ 5 files changed, 99 insertions(+), 58 deletions(-) diff --git a/actions/newnotice.php b/actions/newnotice.php index faf9b21ef6..6e6c3ff2c2 100644 --- a/actions/newnotice.php +++ b/actions/newnotice.php @@ -49,19 +49,22 @@ class NewnoticeAction extends Action { $notice->profile_id = $user->id; # user id *is* profile id $notice->created = DB_DataObject_Cast::dateTime(); # Default theme uses 'content' for something else - $notice->content = trim($this->arg('noticecontent')); + $notice->content = $this->trimmed('noticecontent'); - $val = $notice->validate(); - if ($val === TRUE) { - return $notice->insert(); - } else { - // XXX: display some info - return NULL; + if (!$notice->content) { + $this->show_form(_t('No content!')); + } else if (strlen($notice->content) > 140) { + $this->show_form(_t('Notice content too long.')); } + + return $notice->insert(); } - function show_form() { + function show_form($msg=NULL) { common_show_header(_t('New notice')); + if ($msg) { + common_element('div', 'error', $msg); + } common_notice_form(); common_show_footer(); } diff --git a/actions/profilesettings.php b/actions/profilesettings.php index a8ae2c97a4..5146126bf5 100644 --- a/actions/profilesettings.php +++ b/actions/profilesettings.php @@ -52,30 +52,54 @@ class ProfilesettingsAction extends SettingsAction { } function handle_post() { - $nickname = $this->arg('nickname'); - $fullname = $this->arg('fullname'); - $email = $this->arg('email'); - $homepage = $this->arg('homepage'); - $bio = $this->arg('bio'); - $location = $this->arg('location'); - + + $nickname = $this->trimmed('nickname'); + $fullname = $this->trimmed('fullname'); + $email = $this->trimmed('email'); + $homepage = $this->trimmed('homepage'); + $bio = $this->trimmed('bio'); + $location = $this->trimmed('location'); + + # Some validation + + if (!Validate::email($email, true)) { + $this->show_form(_t('Not a valid email address.')); + return; + } else if (!Validate::string($nickname, array('min_length' => 1, + 'max_length' => 64, + 'format' => VALIDATE_NUM . VALIDATE_ALPHA_LOWER))) { + $this->show_form(_t('Nickname must have only letters and numbers and no spaces.')); + return; + } else if (!is_null($homepage) && (strlen($homepage) > 0) && + !Validate::uri($homepage, array('allowed_schemes' => array('http', 'https')))) { + $this->show_form(_t('Homepage is not a valid URL.')); + return; + } else if (!is_null($fullname) && strlen($fullname) > 255) { + $this->show_form(_t('Fullname is too long (max 255 chars).')); + return; + } else if (!is_null($bio) && strlen($bio) > 140) { + $this->show_form(_t('Bio is too long (max 140 chars).')); + return; + } else if (!is_null($location) && strlen($location) > 255) { + $this->show_form(_t('Location is too long (max 255 chars).')); + return; + } else if ($this->nickname_exists($nickname)) { + $this->show_form(_t('Nickname already exists.')); + return; + } else if ($this->email_exists($email)) { + $this->show_form(_t('Email address already exists.')); + return; + } + $user = common_current_user(); assert(!is_null($user)); # should already be checked - # FIXME: scrub input # FIXME: transaction! $original = clone($user); - $user->nickname = $this->arg('nickname'); - $user->email = $this->arg('email'); - - $val = $user->validate(); - if ($val !== TRUE) { - # XXX: better validation - $this->show_form(_t('Error saving user; invalid.')); - return; - } + $user->nickname = $nickname; + $user->email = $email; if (!$user->update($original)) { common_server_error(_t('Couldnt update user.')); @@ -87,19 +111,12 @@ class ProfilesettingsAction extends SettingsAction { $orig_profile = clone($profile); $profile->nickname = $user->nickname; - $profile->fullname = $this->arg('fullname'); - $profile->homepage = $this->arg('homepage'); - $profile->bio = $this->arg('bio'); - $profile->location = $this->arg('location'); + $profile->fullname = $fullname; + $profile->homepage = $homepage; + $profile->bio = $bio; + $profile->location = $location; $profile->profileurl = common_profile_url($nickname); - $val = $profile->validate(); - if ($val !== TRUE) { - # XXX: some feedback here, please! - $this->show_form(_t('Error saving profile; invalid.')); - return; - } - if (!$profile->update($orig_profile)) { common_server_error(_t('Couldnt save profile.')); return; @@ -107,4 +124,24 @@ class ProfilesettingsAction extends SettingsAction { $this->show_form(_t('Settings saved.'), TRUE); } + + function nickname_exists($nickname) { + $user = common_current_user(); + $other = User::staticGet('nickname', $nickname); + if (!$other) { + return false; + } else { + return $other->id != $user->id; + } + } + + function email_exists($email) { + $user = common_current_user(); + $other = User::staticGet('email', $email); + if (!$other) { + return false; + } else { + return $other->id != $user->id; + } + } } \ No newline at end of file diff --git a/actions/register.php b/actions/register.php index 5da867b0f9..c67235f9d8 100644 --- a/actions/register.php +++ b/actions/register.php @@ -34,18 +34,27 @@ class RegisterAction extends Action { } function try_register() { - $nickname = $this->arg('nickname'); + $nickname = $this->trimmed('nickname'); + $email = $this->trimmed('email'); + + # We don't trim these... whitespace is OK in a password! + $password = $this->arg('password'); $confirm = $this->arg('confirm'); - $email = $this->arg('email'); # Input scrubbing $nickname = common_canonical_nickname($nickname); $email = common_canonical_email($email); - if ($this->nickname_exists($nickname)) { - $this->show_form(_t('Username already exists.')); + if (!Validate::email($email, true)) { + $this->show_form(_t('Not a valid email address.')); + } else if (!Validate::string($nickname, array('min_length' => 1, + 'max_length' => 64, + 'format' => VALIDATE_NUM . VALIDATE_ALPHA_LOWER))) { + $this->show_form(_t('Nickname must have only letters and numbers and no spaces.')); + } else if ($this->nickname_exists($nickname)) { + $this->show_form(_t('Nickname already exists.')); } else if ($this->email_exists($email)) { $this->show_form(_t('Email address already exists.')); } else if ($password != $confirm) { @@ -84,11 +93,6 @@ class RegisterAction extends Action { $profile->profileurl = common_profile_url($nickname); $profile->created = DB_DataObject_Cast::dateTime(); # current time - $val = $profile->validate(); - if ($val !== TRUE) { - # XXX: some feedback here, please! - return FALSE; - } $id = $profile->insert(); if (!$id) { return FALSE; @@ -100,14 +104,6 @@ class RegisterAction extends Action { $user->email = $email; $user->created = DB_DataObject_Cast::dateTime(); # current time - $val = $user->validate(); - if ($val !== TRUE) { - # XXX: some feedback here, please! - # Try to clean up... - $profile->delete(); - return FALSE; - } - $result = $user->insert(); if (!$result) { # Try to clean up... diff --git a/doc/TODO b/doc/TODO index 61d86fd7c4..f5609a35c8 100644 --- a/doc/TODO +++ b/doc/TODO @@ -38,10 +38,10 @@ + save profile URL on registration + require valid nicknames + reject empty notices -- validate registration form results -- validate profilesettings form results -- validate newnotice form results -- remove validation code from classes ++ validate registration form results ++ validate profilesettings form results ++ validate newnotice form results ++ remove validation code from classes + use only canonical usernames - use only canonical email addresses - RSS 1.0 feeds of a user's notices @@ -55,7 +55,7 @@ - pretty URLs - instructions - deal with PHP quotes escaping -- fix layout of textarea ++ fix layout of textarea + make notices into "big links" - fix spacing on notices - limit entry in textarea to 140 chars diff --git a/lib/action.php b/lib/action.php index 8d4a0b7ab5..9b7988a190 100644 --- a/lib/action.php +++ b/lib/action.php @@ -34,6 +34,11 @@ class Action { // lawsuit } } + function trimmed($key) { + $arg = $this->arg($key); + return (is_string($arg)) ? trim($arg) : $arg; + } + function handle($argarray) { $this->args = array(); foreach ($argarray as $k => $v) { -- 2.39.5