summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEvan Prodromou <evan@prodromou.name>2008-05-21 07:27:07 -0400
committerEvan Prodromou <evan@prodromou.name>2008-05-21 07:27:07 -0400
commit764a391d196287a9400ee597019c3e5207c5a5f0 (patch)
tree159f2cc26f3c415f67a4da821076865ec9cf6396
parent46b3f1c3a746044ae868c06bf3027e0a3ea27433 (diff)
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
-rw-r--r--actions/newnotice.php19
-rw-r--r--actions/profilesettings.php93
-rw-r--r--actions/register.php30
-rw-r--r--doc/TODO10
-rw-r--r--lib/action.php5
5 files changed, 99 insertions, 58 deletions
diff --git a/actions/newnotice.php b/actions/newnotice.php
index faf9b21ef..6e6c3ff2c 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 a8ae2c97a..5146126bf 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 5da867b0f..c67235f9d 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 61d86fd7c..f5609a35c 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 8d4a0b7ab..9b7988a19 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) {