Convert tags from user-supplied string to array once, on input

This results in a fuckton of refactoring and code cancelling out
other code -- we no longer have a whole bunch of places trying
to support string params and array params, and doing their own
esaping and unescaping, never being quite sure if the data they've
been passed is escaped or not.

Also adds a bunch of type hinting, since we can now know what
data we're dealing with better.

core/event.class.php

@@ -133,7 +133,7 @@ class PageRequestEvent extends Event {
 	public function get_search_terms() {
 		$search_terms = array();
 		if($this->count_args() === 2) {
-			$search_terms = explode(' ', $this->get_arg(0));
+			$search_terms = Tag::explode($this->get_arg(0));
 		}
 		return $search_terms;
 	}

core/extension.class.php

@@ -174,7 +174,7 @@ abstract class DataHandlerExtension extends Extension {
 		$supported_ext = $this->supported_ext($event->type);
 		$check_contents = $this->check_contents($event->tmpname);
 		if($supported_ext && $check_contents) {
-			if(!move_upload_to_archive($event)) return;
+			move_upload_to_archive($event);
 			send_event(new ThumbnailGenerationEvent($event->hash, $event->type));
 
 			/* Check if we are replacing an image */

core/imageboard.pack.php

@@ -198,7 +198,6 @@ class Image {
 	public function get_accelerated_result($tags, $offset, $limit) {
 		global $database;
 
-		$tags = Tag::resolve_aliases($tags);
 		if(!Image::validate_accel($tags)) {
 			return null;
 		}
@@ -263,10 +262,9 @@ class Image {
 			return $total;
 		}
 		else if($tag_count === 1 && !preg_match("/[:=><\*\?]/", $tags[0])) {
-			$term = Tag::resolve_alias($tags[0]);
 			return $database->get_one(
 				$database->scoreql_to_sql("SELECT count FROM tags WHERE SCORE_STRNORM(tag) = SCORE_STRNORM(:tag)"),
-				array("tag"=>$term));
+				array("tag"=>$tags[0]));
 		}
 		else {
 			$querylet = Image::build_search_querylet($tags);
@@ -802,7 +800,6 @@ class Image {
 			}
 		}
 
-		$terms = Tag::resolve_aliases($terms);
 		foreach ($terms as $term) {
 			$positive = true;
 			if (is_string($term) && !empty($term) && ($term[0] == '-')) {
@@ -819,10 +816,18 @@ class Image {
 				foreach ($stpe->get_querylets() as $querylet) {
 					$img_querylets[] = new ImgQuerylet($querylet, $positive);
 				}
-			} else {
+			}
-				$tag_querylets[] = new TagQuerylet(Tag::sanitise_wildcard($term), $positive);
+			else {
-				if ($positive) $positive_tag_count++;
+				// if the whole match is wild, skip this;
-				else $negative_tag_count++;
+				// if not, translate into SQL
+				if(str_replace("*", "", $term) != "") {
+					$term = str_replace('_', '\_', $term);
+					$term = str_replace('%', '\%', $term);
+					$term = str_replace('*', '%', $term);
+					$tag_querylets[] = new TagQuerylet($term, $positive);
+					if ($positive) $positive_tag_count++;
+					else $negative_tag_count++;
+				}
 			}
 		}
 
@@ -1063,146 +1068,85 @@ class Image {
  */
 class Tag {
 	/**
-	 * Remove any excess fluff from a user-input tag
+	 * @param string[] $tags
-	 *
-	 * @param string $tag
-	 * @return string
-	 */
-	public static function sanitise($tag) {
-		assert('is_string($tag)');
-		$tag = preg_replace("/[\s?*]/", "", $tag);            # whitespace
-		$tag = preg_replace('/\x20(\x0e|\x0f)/', '', $tag);   # unicode RTL
-		$tag = preg_replace("/\.+/", ".", $tag);              # strings of dots?
-		$tag = preg_replace("/^(\.+[\/\\\\])+/", "", $tag);   # trailing slashes?
-		return $tag;
-	}
-
-	/**
-	 * Turn any string or array into a valid tag array.
-	 *
-	 * @param string|string[] $tags
-	 * @param bool $tagme
-	 * @return string[]
-	 */
-	public static function explode($tags, $tagme=true) {
-		assert('is_string($tags) || is_array($tags)');
-
-		if(is_string($tags)) {
-			$tags = explode(' ', trim($tags));
-		}
-		//else if(is_array($tags)) {
-			// do nothing
-		//}
-
-		$tag_array = array();
-		foreach($tags as $tag) {
-			$tag = trim($tag, ", \t\n\r\0\x0B");
-			if(is_string($tag) && !empty($tag)) {
-				$tag_array[] = $tag;
-			}
-		}
-
-		if(count($tag_array) === 0 && $tagme) {
-			$tag_array = array("tagme");
-		}
-
-		$tag_array = array_iunique($tag_array); //remove duplicate tags
-
-		sort($tag_array);
-
-		return $tag_array;
-	}
-
-	/**
-	 * @param string|string[] $tags
 	 * @return string
 	 */
 	public static function implode($tags) {
-		assert('is_string($tags) || is_array($tags)');
+		assert('is_array($tags)');
 
-		if(is_array($tags)) {
+		sort($tags);
-			sort($tags);
+		$tags = implode(' ', $tags);
-			$tags = implode(' ', $tags);
-		}
-		//else if(is_string($tags)) {
-			// do nothing
-		//}
 
 		return $tags;
 	}
 
 	/**
-	 * @param string $tag
+	 * Turn a human-supplied string into a valid tag array.
-	 * @return string
-	 */
-	public static function resolve_alias($tag) {
-		assert('is_string($tag)');
-		global $database;
-
-		$negative = false;
-		if(!empty($tag) && ($tag[0] == '-')) {
-			$negative = true;
-			$tag = substr($tag, 1);
-		}
-
-
-		$newtag = $database->get_one(
-			$database->scoreql_to_sql("SELECT newtag FROM aliases WHERE SCORE_STRNORM(oldtag)=SCORE_STRNORM(:tag)"),
-			array("tag"=>$tag)
-		);
-
-		if(empty($newtag)) {
-			//tag has no alias, use old tag
-			$newtag = $tag;
-		}
-
-		return !$negative ? $newtag : preg_replace("/(\S+)/", "-$1", $newtag);
-	}
-
-	/**
-	 * @param string $tag
-	 * @return string
-	 */
-	public static function sanitise_wildcard($tag) {
-		// if there is no wildcard, return the tag
-		if(strpos($tag, "*") === false) {
-			return $tag;
-		}
-
-		// if the whole match is wild, save the database
-		else if(str_replace("*", "", $tag) == "") {
-			return "invalid-wildcard";
-		}
-
-		// else translate to sql
-		else {
-			$tag = str_replace("%", "\%", $tag);
-			$tag = str_replace("*", "%", $tag);
-			return $tag;
-		}
-	}
-
-	/**
-	 * This function takes a list (array) of tags and changes any tags that have aliases
 	 *
-	 * @param string[] $tags Array of tags
+	 * @param string $tags
-	 * @return array
+	 * @param bool $tagme add "tagme" if the string is empty
+	 * @return string[]
 	 */
-	public static function resolve_aliases($tags) {
+	public static function explode($tags, $tagme=true) {
-		assert('is_array($tags)');
+		global $database;
+		assert('is_string($tags)');
 
+		$tags = explode(' ', trim($tags));
+
+		/* sanitise by removing invisible / dodgy characters */
+		$tag_array = array();
+		foreach($tags as $tag) {
+			$tag = preg_replace("/\s/", "", $tag);                # whitespace
+			$tag = preg_replace('/\x20(\x0e|\x0f)/', '', $tag);   # unicode RTL
+			$tag = preg_replace("/\.+/", ".", $tag);              # strings of dots?
+			$tag = preg_replace("/^(\.+[\/\\\\])+/", "", $tag);   # trailing slashes?
+			$tag = trim($tag, ", \t\n\r\0\x0B");
+
+			if(!empty($tag)) {
+				$tag_array[] = $tag;
+			}
+		}
+
+		/* if user supplied a blank string, add "tagme" */
+		if(count($tag_array) === 0 && $tagme) {
+			$tag_array = array("tagme");
+		}
+
+		/* resolve aliases */
 		$new = array();
-
 		$i = 0;
-		$tag_count = count($tags);
+		$tag_count = count($tag_array);
 		while($i<$tag_count) {
-			$aliases = Tag::explode(Tag::resolve_alias($tags[$i]), FALSE);
+			$tag = $tag_array[$i];
-			foreach($aliases as $alias){
+			$negative = '';
-				if(!in_array($alias, $new)){
+			if(!empty($tag) && ($tag[0] == '-')) {
-					if($tags[$i] == $alias){
+				$negative = '-';
-						$new[] = $alias;
+				$tag = substr($tag, 1);
-					}elseif(!in_array($alias, $tags)){
+			}
-						$tags[] = $alias;
+
+			$newtags = $database->get_one(
+				$database->scoreql_to_sql("
+					SELECT newtag
+					FROM aliases
+					WHERE SCORE_STRNORM(oldtag)=SCORE_STRNORM(:tag)
+				"),
+				array("tag"=>$tag)
+			);
+			if(empty($newtags)) {
+				//tag has no alias, use old tag
+				$aliases = array($tag);
+			}
+			else {
+				$aliases = Tag::explode($newtags);
+			}
+
+			foreach($aliases as $alias) {
+				if(!in_array($alias, $new)) {
+					if($tag == $alias) {
+						$new[] = $negative.$alias;
+					}
+					elseif(!in_array($alias, $tag_array)) {
+						$tag_array[] = $negative.$alias;
 						$tag_count++;
 					}
 				}
@@ -1210,8 +1154,13 @@ class Tag {
 			$i++;
 		}
 
-		$new = array_iunique($new); // remove any duplicate tags
+		/* remove any duplicate tags */
-		return $new;
+		$tag_array = array_iunique($new);
+
+		/* tidy up */
+		sort($tag_array);
+
+		return $tag_array;
 	}
 }
 
@@ -1225,16 +1174,17 @@ class Tag {
  * hierarchy, or throw an exception trying.
  *
  * @param DataUploadEvent $event
- * @return bool
  * @throws UploadException
  */
 function move_upload_to_archive(DataUploadEvent $event) {
 	$target = warehouse_path("images", $event->hash);
 	if(!@copy($event->tmpname, $target)) {
-		$errors = error_get_last(); // note: requires php 5.2
+		$errors = error_get_last();
-		throw new UploadException("Failed to copy file from uploads ({$event->tmpname}) to archive ($target): {$errors['type']} / {$errors['message']}");
+		throw new UploadException(
+			"Failed to copy file from uploads ({$event->tmpname}) to archive ($target): ".
+			"{$errors['type']} / {$errors['message']}"
+		);
 	}
-	return true;
 }
 
 /**

ext/handle_ico/main.php

@@ -10,7 +10,7 @@ class IcoFileHandler extends Extension {
 		if($this->supported_ext($event->type) && $this->check_contents($event->tmpname)) {
 			$hash = $event->hash;
 			$ha = substr($hash, 0, 2);
-			if(!move_upload_to_archive($event)) return;
+			move_upload_to_archive($event);
 			send_event(new ThumbnailGenerationEvent($event->hash, $event->type));
 			$image = $this->create_image_from_data("images/$ha/$hash", $event->metadata);
 			if(is_null($image)) {

ext/handle_svg/main.php

@@ -10,7 +10,7 @@ class SVGFileHandler extends Extension {
 	public function onDataUpload(DataUploadEvent $event) {
 		if($this->supported_ext($event->type) && $this->check_contents($event->tmpname)) {
 			$hash = $event->hash;
-			if(!move_upload_to_archive($event)) return;
+			move_upload_to_archive($event);
 			send_event(new ThumbnailGenerationEvent($event->hash, $event->type));
 			$image = $this->create_image_from_data(warehouse_path("images", $hash), $event->metadata);
 			if(is_null($image)) {

ext/index/main.php

@@ -161,16 +161,16 @@
 class SearchTermParseEvent extends Event {
 	/** @var null|string  */
 	public $term = null;
-	/** @var null|array */
+	/** @var null */
-	public $context = null;
+	public $context = array();
 	/** @var \Querylet[] */
 	public $querylets = array();
 
 	/**
 	 * @param string|null $term
-	 * @param array|null $context
+	 * @param array $context
 	 */
-	public function __construct($term, $context) {
+	public function __construct($term, array $context) {
 		$this->term = $term;
 		$this->context = $context;
 	}
@@ -201,16 +201,16 @@ class SearchTermParseException extends SCoreException {
 }
 
 class PostListBuildingEvent extends Event {
-	/** @var null|array */
+	/** @var array */
-	public $search_terms = null;
+	public $search_terms = array();
 
 	/** @var array */
 	public $parts = array();
 
 	/**
-	 * @param array|null $search
+	 * @param string[] $search
 	 */
-	public function __construct($search) {
+	public function __construct(array $search) {
 		$this->search_terms = $search;
 	}
 
@@ -238,7 +238,8 @@ class Index extends Extension {
 		global $database, $page;
 		if($event->page_matches("post/list")) {
 			if(isset($_GET['search'])) {
-				$search = url_escape(Tag::implode(Tag::resolve_aliases(Tag::explode($_GET['search'], false))));
+				// implode(explode()) to resolve aliases and sanitise
+				$search = url_escape(Tag::implode(Tag::explode($_GET['search'], false)));
 				if(empty($search)) {
 					$page->set_mode("redirect");
 					$page->set_redirect(make_link("post/list/1"));
@@ -310,7 +311,7 @@ class Index extends Extension {
 		$event->panel->add_block($sb);
 	}
 
-	public function onImageInfoSet($event) {
+	public function onImageInfoSet(ImageInfoSetEvent $event) {
 		global $database;
 		if(SPEED_HAX) {
 			$database->cache->delete("thumb-block:{$event->image->id}");

ext/livefeed/main.php

@@ -15,11 +15,11 @@ class LiveFeed extends Extension {
 		$event->panel->add_block($sb);
 	}
 
-	public function onUserCreation($event) {
+	public function onUserCreation(UserCreationEvent $event) {
 		$this->msg("New user created: {$event->username}");
 	}
 
-	public function onImageAddition($event) {
+	public function onImageAddition(ImageAdditionEvent $event) {
 		global $user;
 		$this->msg(
 			make_http(make_link("post/view/".$event->image->id))." - ".
@@ -27,14 +27,14 @@ class LiveFeed extends Extension {
 		);
 	}
 
-	public function onTagSet($event) {
+	public function onTagSet(TagSetEvent $event) {
 		$this->msg(
 			make_http(make_link("post/view/".$event->image->id))." - ".
 			"tags set to: ".Tag::implode($event->tags)
 		);
 	}
 
-	public function onCommentPosting($event) {
+	public function onCommentPosting(CommentPostingEvent $event) {
 		global $user;
 		$this->msg(
 			make_http(make_link("post/view/".$event->image_id))." - ".
@@ -42,14 +42,19 @@ class LiveFeed extends Extension {
 		);
 	}
 
-	public function onImageInfoSet($event) {
+	public function onImageInfoSet(ImageInfoSetEvent $event) {
 #		$this->msg("Image info set");
 	}
 
 	public function get_priority() {return 99;}
 
+	/**
+	 * @param string $data
+	 */
     private function msg($data) {
 		global $config;
+		assert('is_string($data)');
+
 		$host = $config->get_string("livefeed_host", "127.0.0.1:25252");
 
         if(!$host) { return; }

ext/ouroboros_api/main.php

@@ -271,12 +271,8 @@ class OuroborosPost extends _SafeOuroborosImage
     public function __construct(array $post, $md5 = '')
     {
         if (array_key_exists('tags', $post)) {
-            $this->tags = Tag::implode(
+            // implode(explode()) to resolve aliases and sanitise
-                array_map(
+            $this->tags = Tag::implode(Tag::explode(urldecode($post['tags'])));
-                    array('Tag', 'sanitise'),
-                    Tag::explode(urldecode($post['tags']))
-                )
-            );
         }
         if (array_key_exists('file', $post)) {
             if (!is_null($post['file'])) {

ext/tag_edit/main.php

@@ -100,20 +100,15 @@ class TagSetEvent extends Event {
 
 	/**
 	 * @param Image $image
-	 * @param string $tags
+	 * @param string[] $tags
 	 */
-	public function __construct(Image $image, $tags) {
+	public function __construct(Image $image, array $tags) {
 		$this->image    = $image;
 
 		$this->tags     = array();
 		$this->metatags = array();
 
-		//tags need to be sanitised, alias checked & have metatags removed before being passed to onTagSet
+		foreach($tags as $tag) {
-		$tag_array = Tag::explode($tags);
-		$tag_array = array_map(array('Tag', 'sanitise'), $tag_array);
-		$tag_array = Tag::resolve_aliases($tag_array);
-
-		foreach($tag_array as $tag) {
 			if((strpos($tag, ':') === FALSE) && (strpos($tag, '=') === FALSE)) {
 				//Tag doesn't contain : or =, meaning it can't possibly be a metatag.
 				//This should help speed wise, as it avoids running every single tag through a bunch of preg_match instead.
@@ -134,12 +129,6 @@ class TagSetEvent extends Event {
 	}
 }
 
-/*
- * LockSetEvent:
- *   $image_id
- *   $locked
- *
- */
 class LockSetEvent extends Event {
 	/** @var \Image */
 	public $image;
@@ -151,7 +140,8 @@ class LockSetEvent extends Event {
 	 * @param bool $locked
 	 */
 	public function __construct(Image $image, $locked) {
-		assert(is_bool($locked));
+		assert('is_bool($locked)');
+
 		$this->image = $image;
 		$this->locked = $locked;
 	}
@@ -175,6 +165,10 @@ class TagTermParseEvent extends Event {
 	 * @param bool $parse
 	 */
 	public function __construct($term, $id, $parse) {
+		assert('is_string($term)');
+		assert('is_int($id)');
+		assert('is_bool($parse)');
+
 		$this->term  = $term;
 		$this->id    = $id;
 		$this->parse = $parse;
@@ -229,7 +223,7 @@ class TagEdit extends Extension {
 			}
 		}
 		if($this->can_tag($event->image) && isset($_POST['tag_edit__tags'])) {
-			send_event(new TagSetEvent($event->image, $_POST['tag_edit__tags']));
+			send_event(new TagSetEvent($event->image, Tag::explode($_POST['tag_edit__tags'])));
 		}
 		if($this->can_source($event->image) && isset($_POST['tag_edit__source'])) {
 			if(isset($_POST['tag_edit__tags']) ? !preg_match('/source[=|:]/', $_POST["tag_edit__tags"]) : TRUE){
@@ -387,10 +381,13 @@ class TagEdit extends Extension {
 	}
 
 	/**
-	 * @param string|string[] $tags
+	 * @param string $tags
 	 * @param string $source
 	 */
 	private function mass_source_edit($tags, $source) {
+		assert('is_string($tags)');
+		assert('is_string($source)');
+
 		$tags = Tag::explode($tags);
 
 		$last_id = -1;

ext/tag_history/main.php

@@ -153,7 +153,7 @@ class Tag_History extends Extension {
 
 		log_debug("tag_history", 'Reverting tags of Image #'.$stored_image_id.' to ['.$stored_tags.']');
 		// all should be ok so we can revert by firing the SetUserTags event.
-		send_event(new TagSetEvent($image, $stored_tags));
+		send_event(new TagSetEvent($image, Tag::explode($stored_tags)));
 		
 		// all should be done now so redirect the user back to the image
 		$page->set_mode("redirect");
@@ -339,7 +339,7 @@ class Tag_History extends Extension {
 
 				log_debug("tag_history", 'Reverting tags of Image #'.$stored_image_id.' to ['.$stored_tags.']');
 				// all should be ok so we can revert by firing the SetTags event.
-				send_event(new TagSetEvent($image, $stored_tags));
+				send_event(new TagSetEvent($image, Tag::explode($stored_tags)));
 				$this->theme->add_status('Reverted Change','Reverted Image #'.$image_id.' to Tag History #'.$stored_result_id.' ('.$row['tags'].')');
 			}
 		}
@@ -351,13 +351,14 @@ class Tag_History extends Extension {
 	 * This function is called just before an images tag are changed.
 	 *
 	 * @param Image $image
-	 * @param string|string[] $tags
+	 * @param string[] $tags
 	 */
 	private function add_tag_history(Image $image, $tags) {
 		global $database, $config, $user;
+		assert('is_array($tags)');
 
 		$new_tags = Tag::implode($tags);
-		$old_tags = Tag::implode($image->get_tag_array());
+		$old_tags = $image->get_tag_list();
 		
 		if($new_tags == $old_tags) { return; }
 		

ext/tag_history/theme.php

@@ -39,7 +39,7 @@ class Tag_HistoryTheme extends Themelet {
 			foreach($current_tags as $tag){
 				$taglinks[] = "<a href='".make_link("post/list/".$tag."/1")."'>".$tag."</a>";
 			}
-			$current_tags = Tag::implode($taglinks);
+			$current_tags = implode(' ', $taglinks);
 
 			$history_list .= "
 				<li>

ext/tag_list/main.php

@@ -491,10 +491,11 @@ class TagList extends Extension {
 	 * @param Page $page
 	 * @param string[] $search
 	 */
-	private function add_refine_block(Page $page, /*array(string)*/ $search) {
+	private function add_refine_block(Page $page, $search) {
 		global $database, $config;
+		assert('is_array($search)');
 
-		$wild_tags = Tag::explode($search);
+		$wild_tags = $search;
 		$str_search = Tag::implode($search);
 		$related_tags = $database->cache->get("related_tags:$str_search");