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

diff --git a/actions/newnotice.php b/actions/newnotice.php
index faf9b21ef6ae80248efa658fe5fdca9ee8081956..6e6c3ff2c2f1f80a6e688fcc115ba0990965e810 100644 (file)
--- 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 a8ae2c97a484477349b5d114714e6a02d44a1a5d..5146126bf5ac925d64aca372ea6c1741e4d42ecf 100644 (file)
--- 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 5da867b0f9e8d0b0d4ae6b642a1fde5f935e7614..c67235f9d8d8d2e618a67764f54e459c15ca5376 100644 (file)
--- 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 61d86fd7c49821762d67371710f9207cb9de786b..f5609a35c8d48b8d1d8f3ccd83e159342bf180e9 100644 (file)
--- 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 8d4a0b7ab53134a0deb4c395dd2243440f060b9c..9b7988a190e031bf69c449f6808dfd08b076c967 100644 (file)
--- 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) {