Validate image uploads and map errors to HTTP status
Invalid uploads raised ArgumentError and surfaced as 500s. Validate the
content type, extension, and size, then route rejections through Roda's
error_handler plugin so the client gets a 422. Tests pass storage_path
as a Pathname, which file_path requires.

Assisted-by: Claude Opus 4.8 via Claude Code
change szxsvkyxslkpkkpupvrvqpnvwyupmlro
commit 2e0c02d4b10963640ef03e5653c69bd6a2cdb8e6
author Alpha Chen <alpha@kejadlen.dev>
date
parent vrwlstqo
diff --git a/lib/web.rb b/lib/web.rb
index 39fc37b..2e5b587 100644
--- a/lib/web.rb
+++ b/lib/web.rb
@@ -6,9 +6,29 @@ require_relative "views/layout"
 require_relative "views/capture"
 
 module Domus
+  # Raised when a request can't be processed because of client input. The
+  # error_handler plugin renders it with the HTTP status it carries.
+  class ClientError < StandardError
+    attr_reader :status
+
+    def initialize(message, status: 422)
+      super(message)
+      @status = status
+    end
+  end
+
   class Web < Roda
     plugin :public
     plugin :all_verbs
+    plugin :error_handler do |e|
+      raise e unless e.is_a?(ClientError)
+
+      response.status = e.status
+      e.message
+    end
+
+    IMAGE_EXTENSIONS = %w[.jpg .jpeg .png .gif .webp .heic .heif].freeze
+    MAX_UPLOAD_BYTES = 25 * 1024 * 1024
 
     route do |r|
       r.public
@@ -17,11 +37,6 @@ module Domus
         r.get do
           Views::Capture.new.call
         end
-
-        r.post do
-          save_file(r.params)
-          r.redirect "/"
-        end
       end
 
       r.on "files" do
@@ -37,16 +52,20 @@ module Domus
     def app = opts.fetch(:app)
     def db = app.db
 
+    # Persists an uploaded image, raising ClientError when the upload is
+    # rejected so the error_handler plugin can render the right status.
     def save_file(params)
       upload = params["file"]
-      raise ArgumentError, "missing file upload" unless upload.is_a?(Hash) && upload[:tempfile]
-      raise ArgumentError, "only images are accepted" unless upload[:type].to_s.start_with?("image/")
-
-      ext = ::File.extname(upload[:filename].to_s)
-      id = db[:files].insert(
-        extension: ext,
-        created_at: Time.now
-      )
+      raise ClientError, "Choose a file to upload." unless upload.is_a?(Hash) && upload[:tempfile]
+      raise ClientError, "Only image files are accepted." unless upload[:type].to_s.start_with?("image/")
+
+      # The browser-supplied type is spoofable, so also require a known
+      # image extension before we trust and store the file.
+      ext = ::File.extname(upload[:filename].to_s).downcase
+      raise ClientError, "That image format isn't supported." unless IMAGE_EXTENSIONS.include?(ext)
+      raise ClientError, "That image is too large (25 MB max)." if upload[:tempfile].size > MAX_UPLOAD_BYTES
+
+      id = db[:files].insert(extension: ext, created_at: Time.now)
 
       dest = app.file_path(id: id, extension: ext)
       FileUtils.mkdir_p(::File.dirname(dest))
diff --git a/sig/generated/config.rbs b/sig/generated/config.rbs
index 0c3c558..3fc012d 100644
--- a/sig/generated/config.rbs
+++ b/sig/generated/config.rbs
@@ -4,10 +4,10 @@ module Domus
   class Config < Data
     attr_reader database_url(): String
 
-    attr_reader storage_path(): String
+    attr_reader storage_path(): Pathname
 
-    def self.new: (String database_url, String storage_path) -> instance
-                | (database_url: String, storage_path: String) -> instance
+    def self.new: (String database_url, Pathname storage_path) -> instance
+                | (database_url: String, storage_path: Pathname) -> instance
 
     def self.members: () -> [ :database_url, :storage_path ]
 
diff --git a/test/test_app.rb b/test/test_app.rb
index cfa1e84..deabe64 100644
--- a/test/test_app.rb
+++ b/test/test_app.rb
@@ -2,6 +2,7 @@
 
 require_relative "test_helper"
 require "rack/test"
+require "tempfile"
 
 class TestApp < Minitest::Test
   include Rack::Test::Methods
@@ -10,10 +11,65 @@ class TestApp < Minitest::Test
     Domus::Web
   end
 
+  def domus = Domus::Web.opts.fetch(:app)
+
+  def setup
+    domus.db[:files].delete
+  end
+
   def test_root
     get "/"
     assert_equal 200, last_response.status
     assert_includes last_response.body, "Domus"
     assert_includes last_response.body, "Add an image"
   end
+
+  def test_upload_image_saves_file_and_redirects
+    post "/files", "file" => upload("photo.png", "image/png", "fake-png-bytes")
+
+    assert_equal 302, last_response.status
+    rows = domus.db[:files].all
+    assert_equal 1, rows.size
+    assert_equal ".png", rows.first[:extension]
+    assert_equal "fake-png-bytes", File.read(domus.file_path(rows.first))
+  end
+
+  def test_upload_without_file_is_rejected
+    post "/files", {}
+
+    assert_equal 422, last_response.status
+    assert_equal 0, domus.db[:files].count
+  end
+
+  def test_upload_rejects_non_image
+    post "/files", "file" => upload("notes.txt", "text/plain", "hello")
+
+    assert_equal 422, last_response.status
+    assert_equal 0, domus.db[:files].count
+  end
+
+  def test_upload_rejects_unsupported_extension
+    post "/files", "file" => upload("sketch.svg", "image/svg+xml", "<svg/>")
+
+    assert_equal 422, last_response.status
+    assert_equal 0, domus.db[:files].count
+  end
+
+  def test_upload_rejects_oversized_file
+    oversized = "x" * (Domus::Web::MAX_UPLOAD_BYTES + 1)
+    post "/files", "file" => upload("huge.png", "image/png", oversized)
+
+    assert_equal 422, last_response.status
+    assert_equal 0, domus.db[:files].count
+  end
+
+  private
+
+  def upload(filename, type, contents)
+    file = Tempfile.new(filename)
+    file.binmode
+    file.write(contents)
+    file.rewind
+    Rack::Test::UploadedFile.new(file.path, type, original_filename: filename)
+  end
 end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index a328a0e..26bacd9 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -4,6 +4,7 @@ require "minitest/autorun"
 require "sequel"
 require "sequel/extensions/migration"
 require "fileutils"
+require "pathname"
 
 require_relative "../lib/app"
 require_relative "../lib/web"
@@ -11,7 +12,7 @@ require_relative "../lib/web"
 storage = Dir.mktmpdir("domus-test")
 at_exit { FileUtils.rm_rf(storage) }
 
-app = Domus::App.new(Domus::Config.new(database_url: ":memory:", storage_path: storage))
+app = Domus::App.new(Domus::Config.new(database_url: ":memory:", storage_path: Pathname(storage)))
 migrate_dir = File.expand_path("../db/migrate", __dir__)
 Sequel::Migrator.run(app.db, migrate_dir) unless Dir.empty?(migrate_dir)
 Domus::Web.opts[:app] = app