remove redundant escaping and split load_balance_url into a separate function with testing

core/imageboard/image.php

@@ -530,7 +530,7 @@ class Image
     public function get_tooltip(): string
     {
         global $config;
-        return $this->parse_link_template($config->get_string(ImageConfig::TIP), "no_escape");
+        return $this->parse_link_template($config->get_string(ImageConfig::TIP));
     }
 
     /**
@@ -761,7 +761,7 @@ class Image
         @unlink($this->get_thumb_filename());
     }
 
-    public function parse_link_template(string $tmpl, string $_escape="url_escape", int $n=0): string
+    public function parse_link_template(string $tmpl, int $n=0): string
     {
         global $config;
 
@@ -781,7 +781,7 @@ class Image
         $tmpl = str_replace('$hash_ab', substr($this->hash, 0, 2), $tmpl);
         $tmpl = str_replace('$hash_cd', substr($this->hash, 2, 2), $tmpl);
         $tmpl = str_replace('$hash', $this->hash, $tmpl);
-        $tmpl = str_replace('$tags', $_escape($tags), $tmpl);
+        $tmpl = str_replace('$tags', $tags, $tmpl);
         $tmpl = str_replace('$base', $base_href, $tmpl);
         $tmpl = str_replace('$ext', $this->ext, $tmpl);
         if ($this->width && $this->height && $this->length) {
@@ -794,9 +794,9 @@ class Image
             $tmpl = str_replace('$size', "${s}s", $tmpl);
         }
         $tmpl = str_replace('$filesize', to_shorthand_int($this->filesize), $tmpl);
-        $tmpl = str_replace('$filename', $_escape($base_fname), $tmpl);
+        $tmpl = str_replace('$filename', $base_fname, $tmpl);
-        $tmpl = str_replace('$title', $_escape($config->get_string(SetupConfig::TITLE)), $tmpl);
+        $tmpl = str_replace('$title', $config->get_string(SetupConfig::TITLE), $tmpl);
-        $tmpl = str_replace('$date', $_escape(autodate($this->posted, false)), $tmpl);
+        $tmpl = str_replace('$date', autodate($this->posted, false), $tmpl);
 
         // nothing seems to use this, sending the event out to 50 exts is a lot of overhead
         if (!SPEED_HAX) {
@@ -804,41 +804,7 @@ class Image
             $tmpl = $plte->link;
         }
 
-        static $flexihashes = [];
+        return load_balance_url($tmpl, $this->hash, $n);
-        $matches = [];
-        if (preg_match("/(.*){(.*)}(.*)/", $tmpl, $matches)) {
-            $pre = $matches[1];
-            $opts = $matches[2];
-            $post = $matches[3];
-
-            if (isset($flexihashes[$opts])) {
-                $flexihash = $flexihashes[$opts];
-            } else {
-                $flexihash = new Flexihash\Flexihash();
-                foreach (explode(",", $opts) as $opt) {
-                    $parts = explode("=", $opt);
-                    $parts_count = count($parts);
-                    $opt_val = "";
-                    $opt_weight = 0;
-                    if ($parts_count === 2) {
-                        $opt_val = $parts[0];
-                        $opt_weight = $parts[1];
-                    } elseif ($parts_count === 1) {
-                        $opt_val = $parts[0];
-                        $opt_weight = 1;
-                    }
-                    $flexihash->addTarget($opt_val, $opt_weight);
-                }
-                $flexihashes[$opts] = $flexihash;
-            }
-
-            // $choice = $flexihash->lookup($pre.$post);
-            $choices = $flexihash->lookupList($this->hash, $n+1);  // hash doesn't change
-            $choice = $choices[$n];
-            $tmpl = $pre.$choice.$post;
-        }
-
-        return $tmpl;
     }
 
     private static function tag_or_wildcard_to_ids(string $tag): array
@@ -860,8 +826,6 @@ class Image
         ?int $limit=null,
         ?int $offset=null
     ): Querylet {
-        global $database;
-
         list($tag_conditions, $img_conditions) = self::terms_to_conditions($tags);
 
         $positive_tag_count = 0;
@@ -905,7 +869,8 @@ class Image
             && !is_null($limit)
         ) {
             $in = $positive_tag_count === 1 ? "IN" : "NOT IN";
-            // doing this inline is 100x slower?
+            // IN (SELECT id FROM tags) is 100x slower than doing a separate
+            // query and then a second query for IN(first_query_results)??
             $tag_array = self::tag_or_wildcard_to_ids($tag_conditions[0]->tag);
             if (count($tag_array) == 0) {
                 if ($positive_tag_count == 1) {

core/tests/util.test.php

@@ -62,4 +62,23 @@ class UtilTest extends \PHPUnit\Framework\TestCase
             warehouse_path("base", $hash, false, 10)
         );
     }
+
+    public function test_load_balance_url()
+    {
+        $hash = "7ac19c10d6859415";
+        $ext = "jpg";
+
+        // pseudo-randomly select one of the image servers, balanced in given ratio
+        $this->assertEquals(
+            "https://baz.mycdn.com/7ac19c10d6859415.jpg",
+            load_balance_url("https://{foo=10,bar=5,baz=5}.mycdn.com/$hash.$ext", $hash)
+        );
+
+        // N'th and N+1'th results should be different
+        $this->assertNotEquals(
+            load_balance_url("https://{foo=10,bar=5,baz=5}.mycdn.com/$hash.$ext", $hash, 0),
+            load_balance_url("https://{foo=10,bar=5,baz=5}.mycdn.com/$hash.$ext", $hash, 1)
+        );
+
+    }
 }

core/util.php

@@ -221,6 +221,44 @@ function data_path(string $filename, bool $create = true): string
     return $filename;
 }
 
+function load_balance_url(string $tmpl, string $hash, int $n=0): string
+{
+    static $flexihashes = [];
+    $matches = [];
+    if (preg_match("/(.*){(.*)}(.*)/", $tmpl, $matches)) {
+        $pre = $matches[1];
+        $opts = $matches[2];
+        $post = $matches[3];
+
+        if (isset($flexihashes[$opts])) {
+            $flexihash = $flexihashes[$opts];
+        } else {
+            $flexihash = new Flexihash\Flexihash();
+            foreach (explode(",", $opts) as $opt) {
+                $parts = explode("=", $opt);
+                $parts_count = count($parts);
+                $opt_val = "";
+                $opt_weight = 0;
+                if ($parts_count === 2) {
+                    $opt_val = $parts[0];
+                    $opt_weight = $parts[1];
+                } elseif ($parts_count === 1) {
+                    $opt_val = $parts[0];
+                    $opt_weight = 1;
+                }
+                $flexihash->addTarget($opt_val, $opt_weight);
+            }
+            $flexihashes[$opts] = $flexihash;
+        }
+
+        // $choice = $flexihash->lookup($pre.$post);
+        $choices = $flexihash->lookupList($hash, $n + 1);  // hash doesn't change
+        $choice = $choices[$n];
+        $tmpl = $pre . $choice . $post;
+    }
+    return $tmpl;
+}
+
 function transload(string $url, string $mfile): ?array
 {
     global $config;

ext/rule34/main.php

@@ -1,5 +1,7 @@
 <?php declare(strict_types=1);
 
+use function MicroHTML\{TR,TH,TD,A};
+
 if ( // kill these glitched requests immediately
     !empty($_SERVER["REQUEST_URI"])
     && strpos(@$_SERVER["REQUEST_URI"], "/http") !== false
@@ -29,9 +31,17 @@ class Rule34 extends Extension
     {
         global $config;
         $image_link = $config->get_string(ImageConfig::ILINK);
-        $url0 = $event->image->parse_link_template($image_link, "url_escape", 0);
+        $url0 = $event->image->parse_link_template($image_link, 0);
-        $url1 = $event->image->parse_link_template($image_link, "url_escape", 1);
+        $url1 = $event->image->parse_link_template($image_link, 1);
-        $html = "<tr><th>Links</th><td><a href='$url0'>Image Only</a> (<a href='$url1'>Backup Server</a>)</td></tr>";
+        $html = (string)TR(
+            TH("Links"),
+            TD(
+                A(["href"=>$url0], "Image Only"),
+                " (",
+                A(["href"=>$url1], "Backup Server"),
+                ")"
+            )
+        );
         $event->add_part($html, 90);
     }