Separate out GET and POST more explicitly

- No longer allow uploading directly via GET, that is terrible for
  security. Instead, use the GET parameters to pre-fill the upload form.
- PageRequestEvent has a `method` property that can be checked in
  extensions

core/event.php

@@ -46,6 +46,7 @@ class InitExtEvent extends Event
  */
 class PageRequestEvent extends Event
 {
+    public string $method;
     /**
      * @var string[]
      */
@@ -53,11 +54,13 @@ class PageRequestEvent extends Event
     public int $arg_count;
     public int $part_count;
 
-    public function __construct(string $path)
+    public function __construct(string $method, string $path)
     {
         parent::__construct();
         global $config;
 
+        $this->method = $method;
+
         // trim starting slashes
         $path = ltrim($path, "/");
 

core/testcase.php

@@ -86,7 +86,7 @@ if(class_exists("\\PHPUnit\\Framework\\TestCase")) {
             return $args;
         }
 
-        protected static function request($page_name, $get_args = null, $post_args = null): Page
+        protected static function request($method, $page_name, $get_args = null, $post_args = null): Page
         {
             // use a fresh page
             global $page;
@@ -100,7 +100,7 @@ if(class_exists("\\PHPUnit\\Framework\\TestCase")) {
             $_GET = $get_args;
             $_POST = $post_args;
             $page = new Page();
-            send_event(new PageRequestEvent($page_name));
+            send_event(new PageRequestEvent($method, $page_name));
             if ($page->mode == PageMode::REDIRECT) {
                 $page->code = 302;
             }
@@ -109,12 +109,12 @@ if(class_exists("\\PHPUnit\\Framework\\TestCase")) {
 
         protected static function get_page($page_name, $args = null): Page
         {
-            return self::request($page_name, $args, null);
+            return self::request("GET", $page_name, $args, null);
         }
 
         protected static function post_page($page_name, $args = null): Page
         {
-            return self::request($page_name, null, $args);
+            return self::request("POST", $page_name, null, $args);
         }
 
         // page things

ext/admin/main.php

@@ -86,16 +86,15 @@ class AdminPage extends Extension
                 parse_str($event->args[1], $_GET);
                 $_SERVER['REQUEST_URI'] .= "?" . $event->args[1];
             }
-            send_event(new PageRequestEvent($event->args[0]));
+            send_event(new PageRequestEvent("GET", $event->args[0]));
             $page->display();
         }
         if ($event->cmd == "post-page") {
             global $page;
-            $_SERVER['REQUEST_METHOD'] = "POST";
             if (isset($event->args[1])) {
                 parse_str($event->args[1], $_POST);
             }
-            send_event(new PageRequestEvent($event->args[0]));
+            send_event(new PageRequestEvent("POST", $event->args[0]));
             $page->display();
         }
         if ($event->cmd == "get-token") {

ext/image/main.php

@@ -95,18 +95,6 @@ class ImageIO extends Extension
                     }
                 }
             }
-        } elseif ($event->page_matches("image/replace")) {
-            global $page, $user;
-            if ($user->can(Permissions::REPLACE_IMAGE) && isset($_POST['image_id']) && $user->check_auth_token()) {
-                $image = Image::by_id(int_escape($_POST['image_id']));
-                if ($image) {
-                    $page->set_mode(PageMode::REDIRECT);
-                    $page->set_redirect(make_link('upload/replace/'.$image->id));
-                } else {
-                    /* Invalid image ID */
-                    throw new ImageReplaceException("Post to replace does not exist.");
-                }
-            }
         } elseif ($event->page_matches("image")) {
             $num = int_escape($event->get_arg(0));
             $this->send_file($num, "image");

ext/image/test.php

@@ -31,17 +31,7 @@ class ImageIOTest extends ShimmiePHPUnitTestCase
         $this->log_in_as_admin();
         $image_id = $this->post_image("tests/pbx_screenshot.jpg", "test");
         $_POST['image_id'] = "$image_id";
-        send_event(new PageRequestEvent("image/delete"));
+        send_event(new PageRequestEvent("POST", "image/delete"));
         $this->assertTrue(true);  // FIXME: assert image was deleted?
     }
-
-    public function testReplaceRequest()
-    {
-        global $page;
-        $this->log_in_as_admin();
-        $image_id = $this->post_image("tests/pbx_screenshot.jpg", "test");
-        $_POST['image_id'] = "$image_id";
-        send_event(new PageRequestEvent("image/replace"));
-        $this->assertEquals(PageMode::REDIRECT, $page->mode);
-    }
 }

ext/image/theme.php

@@ -26,10 +26,8 @@ class ImageIOTheme extends Themelet
      */
     public function get_replace_html(int $image_id): string
     {
-        return (string)SHM_SIMPLE_FORM(
+        $form = SHM_FORM("replace/$image_id", "GET");
-            "image/replace",
+        $form->appendChild(INPUT(["type" => 'submit', "value" => 'Replace']));
-            INPUT(["type" => 'hidden', "name" => 'image_id', "value" => $image_id]),
+        return (string)$form;
-            INPUT(["type" => 'submit', "value" => 'Replace']),
-        );
     }
 }

ext/log_console/main.php

@@ -20,13 +20,12 @@ class LogConsole extends Extension
 
         if (
             $config->get_bool("log_console_access") &&
-            isset($_SERVER['REQUEST_METHOD']) &&
             isset($_SERVER['REQUEST_URI'])
         ) {
             $this->log(new LogEvent(
                 "access",
                 SCORE_LOG_INFO,
-                "{$_SERVER['REQUEST_METHOD']} {$_SERVER['REQUEST_URI']}"
+                "{$event->method} {$_SERVER['REQUEST_URI']}"
             ));
         }
 

ext/upload/main.php

@@ -192,7 +192,7 @@ class Upload extends Extension
             }
         }
 
-        if ($event->page_matches("upload/replace")) {
+        if ($event->page_matches("replace")) {
             if (!$user->can(Permissions::REPLACE_IMAGE)) {
                 $this->theme->display_error(403, "Error", "{$user->name} doesn't have permission to replace images");
                 return;
@@ -202,35 +202,24 @@ class Upload extends Extension
                 return;
             }
 
-            // Try to get the image ID
+            $image_id = int_escape($event->get_arg(0));
-            if ($event->count_args() >= 1) {
-                $image_id = int_escape($event->get_arg(0));
-            } elseif (isset($_POST['image_id'])) {
-                $image_id = int_escape($_POST['image_id']);
-            } else {
-                throw new UploadException("Can not replace Post: No valid Post ID given.");
-            }
             $image_old = Image::by_id($image_id);
             if (is_null($image_old)) {
                 throw new UploadException("Can not replace Post: No post with ID $image_id");
             }
 
-            $source = $_POST['source'] ?? null;
+            if($event->method == "GET") {
-
-            if (!empty($_POST["url"])) {
-                [$image_ids, $errors] = $this->try_transload($_POST["url"], [], $source, $image_id);
-                $cache->delete("thumb-block:{$image_id}");
-                $this->theme->display_upload_status($page, $image_ids, $errors);
-            } elseif (count($_FILES) > 0) {
-                [$image_ids, $errors] = $this->try_upload($_FILES["data"], [], $source, $image_id);
-                $cache->delete("thumb-block:{$image_id}");
-                $this->theme->display_upload_status($page, $image_ids, $errors);
-            } elseif (!empty($_GET['url'])) {
-                [$image_ids, $errors] = $this->try_transload($_GET['url'], [], $source, $image_id);
-                $cache->delete("thumb-block:{$image_id}");
-                $this->theme->display_upload_status($page, $image_ids, $errors);
-            } else {
                 $this->theme->display_replace_page($page, $image_id);
+            } elseif($event->method == "POST") {
+                $image_ids = [];
+                $errors = [];
+                if (!empty($_POST["url"])) {
+                    [$image_ids, $errors] = $this->try_transload($_POST["url"], [], $_POST['source'] ?? null, $image_id);
+                } elseif (count($_FILES) > 0) {
+                    [$image_ids, $errors] = $this->try_upload($_FILES["data"], [], $_POST['source'] ?? null, $image_id);
+                }
+                $cache->delete("thumb-block:{$image_id}");
+                $this->theme->display_upload_status($page, $image_ids, $errors);
             }
         } elseif ($event->page_matches("upload")) {
             if (!$user->can(Permissions::CREATE_IMAGE)) {
@@ -241,50 +230,39 @@ class Upload extends Extension
                 $this->theme->display_error(507, "Error", "Can't upload images: disk nearly full");
                 return;
             }
-            if(count($_POST) == 0 && empty($_GET['url'])) {
+
+            if($event->method == "GET") {
                 $this->theme->display_page($page);
-                return;
+            } elseif($event->method == "POST") {
-            }
+                $all_image_ids = [];
+                $all_errors = [];
 
-            $all_image_ids = [];
+                $files = array_filter($_FILES, function ($file) {
-            $all_errors = [];
+                    return !empty($file['name']);
-
+                });
-            $files = array_filter($_FILES, function ($file) {
+                foreach ($files as $name => $file) {
-                return !empty($file['name']);
+                    $slot = int_escape(substr($name, 4));
-            });
+                    $tags = $this->tags_for_upload_slot($slot);
-            $urls = array_filter($_POST, function ($value, $key) {
+                    $source = $this->source_for_upload_slot($slot);
-                return str_starts_with($key, "url") && strlen($value) > 0;
+                    [$image_ids, $errors] = $this->try_upload($file, $tags, $source);
-            }, ARRAY_FILTER_USE_BOTH);
+                    $all_image_ids = array_merge($all_image_ids, $image_ids);
-
+                    $all_errors = array_merge($all_errors, $errors);
-            foreach ($files as $name => $file) {
-                $tags = $this->tags_for_upload_slot(int_escape(substr($name, 4)));
-                $source = $_POST['source'] ?? null;
-                [$image_ids, $errors] = $this->try_upload($file, $tags, $source);
-                $all_image_ids = array_merge($all_image_ids, $image_ids);
-                $all_errors = array_merge($all_errors, $errors);
-            }
-            foreach ($urls as $name => $value) {
-                $tags = $this->tags_for_upload_slot(int_escape(substr($name, 3)));
-                $source = $_POST['source'] ?? $value;
-                [$image_ids, $errors] = $this->try_transload($value, $tags, $source);
-                $all_image_ids = array_merge($all_image_ids, $image_ids);
-                $all_errors = array_merge($all_errors, $errors);
-            }
-
-            if(!empty($_GET['url'])) {
-                $url = $_GET['url'];
-                $source = $_GET['source'] ?? $url;
-                $tags = ['tagme'];
-                if (!empty($_GET['tags']) && $_GET['tags'] != "null") {
-                    $tags = Tag::explode($_GET['tags']);
                 }
 
-                [$image_ids, $errors] = $this->try_transload($url, $tags, $source);
+                $urls = array_filter($_POST, function ($value, $key) {
-                $all_image_ids = array_merge($all_image_ids, $image_ids);
+                    return str_starts_with($key, "url") && strlen($value) > 0;
-                $all_errors = array_merge($all_errors, $errors);
+                }, ARRAY_FILTER_USE_BOTH);
-            }
+                foreach ($urls as $name => $value) {
+                    $slot = int_escape(substr($name, 3));
+                    $tags = $this->tags_for_upload_slot($slot);
+                    $source = $this->source_for_upload_slot($slot);
+                    [$image_ids, $errors] = $this->try_transload($value, $tags, $source);
+                    $all_image_ids = array_merge($all_image_ids, $image_ids);
+                    $all_errors = array_merge($all_errors, $errors);
+                }
 
-            $this->theme->display_upload_status($page, $all_image_ids, $all_errors);
+                $this->theme->display_upload_status($page, $all_image_ids, $all_errors);
+            }
         }
     }
 
@@ -299,6 +277,11 @@ class Upload extends Extension
         );
     }
 
+    private function source_for_upload_slot(int $id): ?string
+    {
+        return $_POST["source$id"] ?? $_POST['source'] ?? null;
+    }
+
     /**
      * Returns a descriptive error message for the specified PHP error code.
      *

ext/upload/test.php

@@ -67,7 +67,7 @@ class UploadTest extends ShimmiePHPUnitTestCase
             ]
         ];
 
-        $page = $this->post_page("upload/replace", ["image_id" => $image_id]);
+        $page = $this->post_page("replace/$image_id");
         $this->assert_response(302);
         $this->assertEquals("/test/post/view/$image_id", $page->redirect);
 

ext/upload/theme.php

@@ -52,15 +52,15 @@ class UploadTheme extends Themelet
                 ["id" => "large_upload_form", "class" => "vert"],
                 TR(
                     TD(["width" => "20"], rawHTML("Common Tags")),
-                    TD(["colspan" => "5"], INPUT(["name" => "tags", "type" => "text", "placeholder" => "tagme", "class" => "autocomplete_tags", "autocomplete" => "off"]))
+                    TD(["colspan" => "6"], INPUT(["name" => "tags", "type" => "text", "placeholder" => "tagme", "class" => "autocomplete_tags"]))
                 ),
                 TR(
                     TD(["width" => "20"], rawHTML("Common Source")),
-                    TD(["colspan" => "5"], INPUT(["name" => "source", "type" => "text"]))
+                    TD(["colspan" => "6"], INPUT(["name" => "source", "type" => "text", "placeholder" => "https://..."]))
                 ),
                 $upload_list,
                 TR(
-                    TD(["colspan" => "6"], INPUT(["id" => "uploadbutton", "type" => "submit", "value" => "Post"]))
+                    TD(["colspan" => "7"], INPUT(["id" => "uploadbutton", "type" => "submit", "value" => "Post"]))
                 ),
             )
         );
@@ -85,7 +85,7 @@ class UploadTheme extends Themelet
         $page->add_block(new NavBlock());
         $page->add_block(new Block("Upload", $html, "main", 20));
         if ($tl_enabled) {
-            $page->add_block(new Block("Bookmarklets", (string)$this->h_bookmarklets(), "left", 20));
+            $page->add_block(new Block("Bookmarklets", $this->build_bookmarklets(), "left", 20));
         }
     }
 
@@ -99,9 +99,10 @@ class UploadTheme extends Themelet
 
         $upload_list->appendChild(
             TR(
-                TD(["colspan" => $tl_enabled ? 2 : 4], "Files"),
+                TD(["colspan" => 2], "Select File"),
-                $tl_enabled ? TD(["colspan" => "2"], "URLs") : emptyHTML(),
+                TD($tl_enabled ? "or URL" : null),
-                TD(["colspan" => "2"], "Post-Specific Tags"),
+                TD("Post-Specific Tags"),
+                TD("Post-Specific Source"),
             )
         );
 
@@ -109,12 +110,42 @@ class UploadTheme extends Themelet
             $upload_list->appendChild(
                 TR(
                     TD(
-                        ["colspan" => $tl_enabled ? 2 : 4],
+                        ["colspan" => 2, "style" => "white-space: nowrap;"],
-                        DIV(["id" => "canceldata{$i}", "style" => "display:inline;margin-right:5px;font-size:15px;visibility:hidden;","onclick" => "document.getElementById('data{$i}').value='';updateTracker();"], "✖"),
+                        DIV([
-                        INPUT(["type" => "file", "id"=>"data{$i}", "name" => "data{$i}[]", "accept" => $accept, "multiple" => true])
+                            "id" => "canceldata{$i}",
+                            "style" => "display:inline;margin-right:5px;font-size:15px;visibility:hidden;",
+                            "onclick" => "document.getElementById('data{$i}').value='';updateTracker();",
+                        ], "✖"),
+                        INPUT([
+                            "type" => "file",
+                            "id" => "data{$i}",
+                            "name" => "data{$i}[]",
+                            "accept" => $accept,
+                            "multiple" => true,
+                        ]),
+                    ),
+                    TD(
+                        $tl_enabled ? INPUT([
+                            "type" => "text",
+                            "name" => "url{$i}",
+                            "value" => ($i == 0) ? @$_GET['url'] : null,
+                        ]) : null
+                    ),
+                    TD(
+                        INPUT([
+                            "type" => "text",
+                            "name" => "tags{$i}",
+                            "class" => "autocomplete_tags",
+                            "value" => ($i == 0) ? @$_GET['tags'] : null,
+                        ])
+                    ),
+                    TD(
+                        INPUT([
+                            "type" => "text",
+                            "name" => "source{$i}",
+                            "value" => ($i == 0) ? @$_GET['source'] : null,
+                        ])
                     ),
-                    $tl_enabled ? TD(["colspan" => "2"], INPUT(["type" => "text", "name" => "url{$i}"])) : emptyHTML(),
-                    TD(["colspan" => "2"], INPUT(["type" => "text", "name" => "tags{$i}", "class" => "autocomplete_tags"])),
                 )
             );
         }
@@ -122,7 +153,7 @@ class UploadTheme extends Themelet
         return $upload_list;
     }
 
-    protected function h_bookmarklets(): HTMLElement
+    protected function build_bookmarklets(): HTMLElement
     {
         global $config;
         $link = make_http(make_link("upload"));
@@ -197,7 +228,7 @@ class UploadTheme extends Themelet
             $upload_list->appendChild(
                 TR(
                     TD("or URL"),
-                    TD(INPUT(["name" => "url", "type" => "text"]))
+                    TD(INPUT(["name" => "url", "type" => "text", "value" => @$_GET['url']]))
                 )
             );
         }
@@ -208,9 +239,8 @@ class UploadTheme extends Themelet
         $image = Image::by_id($image_id);
         $thumbnail = $this->build_thumb_html($image);
 
-        $form = SHM_FORM("upload/replace/".$image_id, "POST", true);
+        $form = SHM_FORM("replace/".$image_id, "POST", true);
         $form->appendChild(emptyHTML(
-            INPUT(["type" => "hidden", "name" => "image_id", "value" => $image_id]),
             TABLE(
                 ["id" => "large_upload_form", "class" => "vert"],
                 $upload_list,

index.php

@@ -82,7 +82,7 @@ try {
     if (PHP_SAPI === 'cli' || PHP_SAPI == 'phpdbg') {
         send_event(new CommandEvent($argv));
     } else {
-        send_event(new PageRequestEvent(_get_query()));
+        send_event(new PageRequestEvent($_SERVER['REQUEST_METHOD'], _get_query()));
         $page->display();
     }