From 1c8094cabfeb09907e6a17922313e73cab7c50c2 Mon Sep 17 00:00:00 2001 From: Shish Date: Sun, 4 Jan 2009 06:01:59 -0800 Subject: [PATCH] replace the veto system with exceptions --- contrib/ban_words/main.php | 6 ++---- contrib/bulk_add/main.php | 10 ++++++---- contrib/danbooru_api/main.php | 19 ++++++++++--------- contrib/handle_archive/main.php | 10 ++++++---- contrib/handle_flash/main.php | 6 +++--- contrib/handle_ico/main.php | 11 ++--------- contrib/handle_mp3/main.php | 3 +-- contrib/handle_svg/main.php | 3 +-- contrib/image_hash_ban/main.php | 2 +- contrib/res_limit/main.php | 12 +++++++----- core/event.class.php | 7 ------- core/imageboard.pack.php | 2 +- core/util.inc.php | 1 - ext/alias_editor/main.php | 16 +++++++++------- ext/comment/main.php | 33 ++++++++++++++++++--------------- ext/handle_pixel/main.php | 9 ++------- ext/image/main.php | 2 +- ext/index/main.php | 1 - ext/upload/main.php | 24 +++++++++++++++--------- ext/user/main.php | 24 +++++++++++++----------- 20 files changed, 98 insertions(+), 103 deletions(-) diff --git a/contrib/ban_words/main.php b/contrib/ban_words/main.php index 02a758a9..3d560d15 100644 --- a/contrib/ban_words/main.php +++ b/contrib/ban_words/main.php @@ -30,15 +30,13 @@ porn else if($word[0] == '/') { // lines that start with slash are regex if(preg_match($word, $comment)) { - $event->veto("Comment contains banned terms"); - break; + throw new CommentPostingException("Comment contains banned terms"); } } else { // other words are literal if(strpos($comment, $word) !== false) { - $event->veto("Comment contains banned terms"); - break; + throw new CommentPostingException("Comment contains banned terms"); } } } diff --git a/contrib/bulk_add/main.php b/contrib/bulk_add/main.php index 9f57d9a8..18638434 100644 --- a/contrib/bulk_add/main.php +++ b/contrib/bulk_add/main.php @@ -35,10 +35,12 @@ class BulkAdd implements Extension { $metadata['extension'] = $pathinfo['extension']; $metadata['tags'] = $tags; $metadata['source'] = null; - $event = new DataUploadEvent($user, $tmpname, $metadata); - send_event($event); - if($event->vetoed) { - return $event->veto_reason; + try { + $event = new DataUploadEvent($user, $tmpname, $metadata); + send_event($event); + } + catch(Exception $ex) { + return $ex->getMessage(); } } } diff --git a/contrib/danbooru_api/main.php b/contrib/danbooru_api/main.php index 678f487a..9765adde 100644 --- a/contrib/danbooru_api/main.php +++ b/contrib/danbooru_api/main.php @@ -223,15 +223,10 @@ class DanbooruApi implements Extension $metadata['tags'] = $posttags; $metadata['source'] = $source; - $nevent = new DataUploadEvent($user, $file, $metadata); - send_event($nevent); - // Did something screw up? - if($event->vetoed) { - header("HTTP/1.0 409 Conflict"); - header("X-Danbooru-Errors: $event->veto_reason"); - return; - } else - { // If it went ok, grab the id for the newly uploaded image and pass it in the header + try { + $nevent = new DataUploadEvent($user, $file, $metadata); + send_event($nevent); + // If it went ok, grab the id for the newly uploaded image and pass it in the header $newimg = Image::by_hash($config, $database, $hash); $newid = make_link("post/view/" . $newimg->id); // Did we POST or GET this call? @@ -242,6 +237,12 @@ class DanbooruApi implements Extension else header("Location: $newid"); } + catch(UploadException $ex) { + // Did something screw up? + header("HTTP/1.0 409 Conflict"); + header("X-Danbooru-Errors: ". $ex->getMessage()); + return; + } } else { header("HTTP/1.0 409 Conflict"); diff --git a/contrib/handle_archive/main.php b/contrib/handle_archive/main.php index 031aba0e..26de4112 100644 --- a/contrib/handle_archive/main.php +++ b/contrib/handle_archive/main.php @@ -47,10 +47,12 @@ class ArchiveFileHandler implements Extension { $metadata['extension'] = $pathinfo['extension']; $metadata['tags'] = $tags; $metadata['source'] = null; - $event = new DataUploadEvent($user, $tmpname, $metadata); - send_event($event); - if($event->vetoed) { - return $event->veto_reason; + try { + $event = new DataUploadEvent($user, $tmpname, $metadata); + send_event($event); + } + catch(UploadException $ex) { + return $ex->getMessage(); } } } diff --git a/contrib/handle_flash/main.php b/contrib/handle_flash/main.php index e4328482..5a0d3298 100644 --- a/contrib/handle_flash/main.php +++ b/contrib/handle_flash/main.php @@ -18,9 +18,9 @@ class FlashFileHandler implements Extension { send_event(new ThumbnailGenerationEvent($event->hash, $event->type)); $image = $this->create_image_from_data("images/$ha/$hash", $event->metadata); if(is_null($image)) { - $event->veto("Flash Handler failed to create image object from data. ". - "Note: compressed flash files are currently unsupported"); - return; + throw new UploadException( + "Flash Handler failed to create image object from data. ". + "Note: compressed flash files are currently unsupported"); } send_event(new ImageAdditionEvent($event->user, $image)); } diff --git a/contrib/handle_ico/main.php b/contrib/handle_ico/main.php index 2a6c89a4..8508e7c2 100644 --- a/contrib/handle_ico/main.php +++ b/contrib/handle_ico/main.php @@ -18,16 +18,9 @@ class IcoFileHandler implements Extension { send_event(new ThumbnailGenerationEvent($event->hash, $event->type)); $image = $this->create_image_from_data("images/$ha/$hash", $event->metadata); if(is_null($image)) { - $event->veto("Handler failed to create image object from data"); - return; - } - - $iae = new ImageAdditionEvent($event->user, $image); - send_event($iae); - if($iae->vetoed) { - $event->veto($iae->veto_reason); - return; + throw new UploadException("Icon handler failed to create image object from data"); } + send_event(new ImageAdditionEvent($event->user, $image)); } if(($event instanceof ThumbnailGenerationEvent) && $this->supported_ext($event->type)) { diff --git a/contrib/handle_mp3/main.php b/contrib/handle_mp3/main.php index b3ac3c2c..397af4f2 100644 --- a/contrib/handle_mp3/main.php +++ b/contrib/handle_mp3/main.php @@ -18,8 +18,7 @@ class MP3FileHandler implements Extension { send_event(new ThumbnailGenerationEvent($event->hash, $event->type)); $image = $this->create_image_from_data("images/$ha/$hash", $event->metadata); if(is_null($image)) { - $event->veto("MP3 Handler failed to create image object from data"); - return; + throw new UploadException("MP3 handler failed to create image object from data"); } send_event(new ImageAdditionEvent($event->user, $image)); } diff --git a/contrib/handle_svg/main.php b/contrib/handle_svg/main.php index 483f7657..81d8677a 100644 --- a/contrib/handle_svg/main.php +++ b/contrib/handle_svg/main.php @@ -18,8 +18,7 @@ class SVGFileHandler implements Extension { send_event(new ThumbnailGenerationEvent($event->hash, $event->type)); $image = $this->create_image_from_data("images/$ha/$hash", $event->metadata); if(is_null($image)) { - $event->veto("SVG Handler failed to create image object from data"); - return; + throw new UploadException("SVG handler failed to create image object from data"); } send_event(new ImageAdditionEvent($event->user, $image)); } diff --git a/contrib/image_hash_ban/main.php b/contrib/image_hash_ban/main.php index 1cfaaa87..c0f2658e 100644 --- a/contrib/image_hash_ban/main.php +++ b/contrib/image_hash_ban/main.php @@ -48,7 +48,7 @@ class ImageBan implements Extension { $row = $database->db->GetRow("SELECT * FROM image_bans WHERE hash = ?", $event->hash); if($row) { - $event->veto("Image ".html_escape($row["hash"])." has been banned, reason: ".format_text($row["reason"])); + throw new UploadException("Image ".html_escape($row["hash"])." has been banned, reason: ".format_text($row["reason"])); } } diff --git a/contrib/res_limit/main.php b/contrib/res_limit/main.php index b6e64bcc..2d435b1e 100644 --- a/contrib/res_limit/main.php +++ b/contrib/res_limit/main.php @@ -18,10 +18,10 @@ class ResolutionLimit implements Extension { $image = $event->image; - if($min_w > 0 && $image->width < $min_w) $event->veto("Image too small"); - if($min_h > 0 && $image->height < $min_h) $event->veto("Image too small"); - if($max_w > 0 && $image->width > $min_w) $event->veto("Image too large"); - if($max_h > 0 && $image->height > $min_h) $event->veto("Image too large"); + if($min_w > 0 && $image->width < $min_w) throw new UploadException("Image too small"); + if($min_h > 0 && $image->height < $min_h) throw new UploadException("Image too small"); + if($max_w > 0 && $image->width > $min_w) throw new UploadExceptiono("Image too large"); + if($max_h > 0 && $image->height > $min_h) throw new UploadException("Image too large"); if(count($ratios) > 0) { $ok = false; @@ -36,7 +36,9 @@ class ResolutionLimit implements Extension { } } if(!$ok) { - $event->veto("Image needs to be in one of these ratios: ".html_escape($config->get_string("upload_ratios", ""))); + throw new UploadException( + "Image needs to be in one of these ratios: ". + html_escape($config->get_string("upload_ratios", ""))); } } } diff --git a/core/event.class.php b/core/event.class.php index 49d26480..ef30e218 100644 --- a/core/event.class.php +++ b/core/event.class.php @@ -5,17 +5,10 @@ */ abstract class Event { var $context; - var $vetoed = false; - var $veto_reason; public function __construct(RequestContext $context) { $this->context = $context; } - - public function veto($reason="") { - $this->vetoed = true; - $this->veto_reason = $reason; - } } diff --git a/core/imageboard.pack.php b/core/imageboard.pack.php index 4a8cff32..b5b02ce0 100644 --- a/core/imageboard.pack.php +++ b/core/imageboard.pack.php @@ -485,7 +485,7 @@ function move_upload_to_archive($event) { $hash = $event->hash; $ha = substr($hash, 0, 2); if(!@copy($event->tmpname, "images/$ha/$hash")) { - $event->veto("Failed to copy file from uploads ({$event->tmpname}) to archive (images/$ha/$hash)"); + throw new UploadException("Failed to copy file from uploads ({$event->tmpname}) to archive (images/$ha/$hash)"); return false; } return true; diff --git a/core/util.inc.php b/core/util.inc.php index 1ae7f073..a0946b66 100644 --- a/core/util.inc.php +++ b/core/util.inc.php @@ -380,7 +380,6 @@ function send_event(Event $event) { ksort($my_event_listeners); foreach($my_event_listeners as $listener) { $listener->receive_event($event); - if($event->vetoed) break; } $_event_count++; } diff --git a/ext/alias_editor/main.php b/ext/alias_editor/main.php index e7db9aff..47cb857a 100644 --- a/ext/alias_editor/main.php +++ b/ext/alias_editor/main.php @@ -10,6 +10,8 @@ class AddAliasEvent extends Event { } } +class AddAliasException extends SCoreException {} + class AliasEditor implements Extension { var $theme; @@ -20,15 +22,15 @@ class AliasEditor implements Extension { if($event->get_arg(0) == "add") { if($event->user->is_admin()) { if(isset($_POST['oldtag']) && isset($_POST['newtag'])) { - $aae = new AddAliasEvent($_POST['oldtag'], $_POST['newtag']); - send_event($aae); - if($aae->vetoed) { - $this->theme->display_error($event->page, "Error adding alias", $aae->veto_reason); - } - else { + try { + $aae = new AddAliasEvent($_POST['oldtag'], $_POST['newtag']); + send_event($aae); $event->page->set_mode("redirect"); $event->page->set_redirect(make_link("alias/list")); } + catch(AddAliasException $ex) { + $this->theme->display_error($event->page, "Error adding alias", $ex->getMessage()); + } } } } @@ -80,7 +82,7 @@ class AliasEditor implements Extension { global $database; $pair = array($event->oldtag, $event->newtag); if($database->db->GetRow("SELECT * FROM aliases WHERE oldtag=? AND lower(newtag)=lower(?)", $pair)) { - $event->veto("That alias already exists"); + throw new AddAliasException("That alias already exists"); } else { $database->Execute("INSERT INTO aliases(oldtag, newtag) VALUES(?, ?)", $pair); diff --git a/ext/comment/main.php b/ext/comment/main.php index c909c169..a16537a3 100644 --- a/ext/comment/main.php +++ b/ext/comment/main.php @@ -35,6 +35,7 @@ class CommentDeletionEvent extends Event { } } // }}} +class CommentPostingException extends SCoreException {} class Comment { // {{{ public function Comment($row) { @@ -68,15 +69,15 @@ class CommentList implements Extension { if(($event instanceof PageRequestEvent) && $event->page_matches("comment")) { if($event->get_arg(0) == "add") { - $cpe = new CommentPostingEvent($_POST['image_id'], $event->user, $_POST['comment']); - send_event($cpe); - if($cpe->vetoed) { - $this->theme->display_error($event->page, "Comment Blocked", $cpe->veto_reason); - } - else { + try { + $cpe = new CommentPostingEvent($_POST['image_id'], $event->user, $_POST['comment']); + send_event($cpe); $event->page->set_mode("redirect"); $event->page->set_redirect(make_link("post/view/".int_escape($_POST['image_id']))); } + catch(CommentPostingException $ex) { + $this->theme->display_error($event->page, "Comment Blocked", $ex->getMessage()); + } } else if($event->get_arg(0) == "delete") { if($event->user->is_admin()) { @@ -346,37 +347,39 @@ class CommentList implements Extension { // basic sanity checks if(!$config->get_bool('comment_anon') && $user->is_anonymous()) { - $event->veto("Anonymous posting has been disabled"); + throw new CommentPostingException("Anonymous posting has been disabled"); } else if(is_null(Image::by_id($config, $database, $image_id))) { - $event->veto("The image does not exist"); + throw new CommentPostingException("The image does not exist"); } else if(trim($comment) == "") { - $event->veto("Comments need text..."); + throw new CommentPostingException("Comments need text..."); } else if(strlen($comment) > 9000) { - $event->veto("Comment too long~"); + throw new CommentPostingException("Comment too long~"); } // advanced sanity checks else if(strlen($comment)/strlen(gzcompress($comment)) > 10) { - $event->veto("Comment too repetitive~"); + throw new CommentPostingException("Comment too repetitive~"); } else if($user->is_anonymous() && !$this->hash_match()) { - $event->veto("Comment submission form is out of date; refresh the comment form to show you aren't a spammer~"); + throw new CommentPostingException( + "Comment submission form is out of date; refresh the ". + "comment form to show you aren't a spammer~"); } // database-querying checks else if($this->is_comment_limit_hit()) { - $event->veto("You've posted several comments recently; wait a minute and try again..."); + throw new CommentPostingException("You've posted several comments recently; wait a minute and try again..."); } else if($this->is_dupe($image_id, $comment)) { - $event->veto("Someone already made that comment on that image -- try and be more original?"); + throw new CommentPostingException("Someone already made that comment on that image -- try and be more original?"); } // rate-limited external service checks last else if($user->is_anonymous() && $this->is_spam($comment)) { - $event->veto("Akismet thinks that your comment is spam. Try rewriting the comment, or logging in."); + throw new CommentPostingException("Akismet thinks that your comment is spam. Try rewriting the comment, or logging in."); } // all checks passed diff --git a/ext/handle_pixel/main.php b/ext/handle_pixel/main.php index 30e6afdc..c4091dce 100644 --- a/ext/handle_pixel/main.php +++ b/ext/handle_pixel/main.php @@ -18,16 +18,11 @@ class PixelFileHandler implements Extension { send_event(new ThumbnailGenerationEvent($event->hash, $event->type)); $image = $this->create_image_from_data("images/$ha/$hash", $event->metadata); if(is_null($image)) { - $event->veto("Pixel Handler failed to create image object from data"); - return; + throw new UploadException("Pixel Handler failed to create image object from data"); } $iae = new ImageAdditionEvent($event->user, $image); - send_event($iae); - if($iae->vetoed) { - $event->veto($iae->veto_reason); - return; - } + send_event($iae); // this might raise an exception, but all we'd do is re-throw it... } if(($event instanceof ThumbnailGenerationEvent) && $this->supported_ext($event->type)) { diff --git a/ext/image/main.php b/ext/image/main.php index 0c9d80ee..0f764f76 100644 --- a/ext/image/main.php +++ b/ext/image/main.php @@ -107,7 +107,7 @@ class ImageIO implements Extension { if($event instanceof ImageAdditionEvent) { $error = $this->add_image($event->image); - if(!empty($error)) $event->veto($error); + if(!empty($error)) throw new UploadException($error); } if($event instanceof ImageDeletionEvent) { diff --git a/ext/index/main.php b/ext/index/main.php index bab61055..9010b28b 100644 --- a/ext/index/main.php +++ b/ext/index/main.php @@ -59,7 +59,6 @@ class Index implements Extension { else { $event->page->set_mode("redirect"); $event->page->set_redirect(make_link("post/list/$search/1")); - //$event->veto(); } return; } diff --git a/ext/upload/main.php b/ext/upload/main.php index f4668c6e..6a617c3f 100644 --- a/ext/upload/main.php +++ b/ext/upload/main.php @@ -24,6 +24,8 @@ class DataUploadEvent extends Event { } } +class UploadException extends SCoreException {} + class Upload implements Extension { var $theme; // event handling {{{ @@ -113,12 +115,12 @@ class Upload implements Extension { if($event instanceof DataUploadEvent) { global $config; if($is_full) { - $event->veto("Upload failed; disk nearly full"); + throw new UploadException("Upload failed; disk nearly full"); } if(filesize($event->tmpname) > $config->get_int('upload_size')) { $size = to_shorthand_int(filesize($event->tmpname)); $limit = to_shorthand_int($config->get_int('upload_size')); - $event->veto("File too large ($size > $limit)"); + throw new UploadException("File too large ($size > $limit)"); } } } @@ -146,10 +148,12 @@ class Upload implements Extension { $metadata['tags'] = $tags; $metadata['source'] = $source; $event = new DataUploadEvent($user, $file['tmp_name'], $metadata); - send_event($event); - if($event->vetoed) { + try { + send_event($event); + } + catch(UploadException $ex) { $this->theme->display_upload_error($page, "Error with ".html_escape($file['name']), - $event->veto_reason); + $ex->getMessage()); $ok = false; } } @@ -224,10 +228,12 @@ class Upload implements Extension { $metadata['tags'] = $tags; $metadata['source'] = $source; $event = new DataUploadEvent($user, $tmp_filename, $metadata); - send_event($event); - if($event->vetoed) { + try { + send_event($event); + } + catch(UploadException $ex) { $this->theme->display_upload_error($page, "Error with ".html_escape($url), - $event->veto_reason); + $ex->getMessage()); $ok = false; } } @@ -238,5 +244,5 @@ class Upload implements Extension { } // }}} } -add_event_listener(new Upload(), 40); // early, so it can veto the DataUploadEvent before any data handlers see it +add_event_listener(new Upload(), 40); // early, so it can stop the DataUploadEvent before any data handlers see it ?> diff --git a/ext/user/main.php b/ext/user/main.php index 4ef5f244..d259a499 100644 --- a/ext/user/main.php +++ b/ext/user/main.php @@ -36,6 +36,8 @@ class UserCreationEvent extends Event { } } +class UserCreationException extends SCoreException {} + class UserPage implements Extension { var $theme; @@ -81,16 +83,16 @@ class UserPage implements Extension { $this->theme->display_error($event->page, "Password Mismatch", "Passwords don't match"); } else { - $uce = new UserCreationEvent($_POST['name'], $_POST['pass1'], $_POST['email']); - send_event($uce); - if($uce->vetoed) { - $this->theme->display_error($event->page, "User Creation Error", $uce->veto_reason); - } - else { + try { + $uce = new UserCreationEvent($_POST['name'], $_POST['pass1'], $_POST['email']); + send_event($uce); $this->set_login_cookie($uce->username, $uce->password); $event->page->set_mode("redirect"); $event->page->set_redirect(make_link("user")); } + catch(UserCreationException $ex) { + $this->theme->display_error($event->page, "User Creation Error", $ex->getMessage()); + } } } else if($event->get_arg(0) == "set_more") { @@ -210,16 +212,16 @@ class UserPage implements Extension { global $database; if(strlen($name) < 1) { - $event->veto("Username must be at least 1 character"); + throw new UserCreationException("Username must be at least 1 character"); } else if(!preg_match('/^[a-zA-Z0-9-_]+$/', $name)) { - $event->veto("Username contains invalid characters. Allowed characters are letters, numbers, dash, and underscore"); + throw new UserCreationException( + "Username contains invalid characters. Allowed characters are ". + "letters, numbers, dash, and underscore"); } else if($database->db->GetRow("SELECT * FROM users WHERE name = ?", array($name))) { - $event->veto("That username is already taken"); + throw new UserCreationException("That username is already taken"); } - - return (!$event->vetoed); } private function create_user($event) {