From 6475ad34259522811dba9a613b58deaf97b30082 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Thu, 4 Dec 2025 12:54:21 +0000 Subject: [PATCH 1/2] Don't try to resize non-variable avatars --- app/controllers/users/avatars_controller.rb | 2 +- app/models/user.rb | 6 +----- app/models/user/avatar.rb | 13 +++++++++++++ .../users/avatars_controller_test.rb | 4 ++-- test/fixtures/files/avatar.svg | 3 +++ test/models/user/avatar_test.rb | 17 +++++++++++++++++ 6 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 app/models/user/avatar.rb create mode 100644 test/fixtures/files/avatar.svg create mode 100644 test/models/user/avatar_test.rb diff --git a/app/controllers/users/avatars_controller.rb b/app/controllers/users/avatars_controller.rb index 61ce75f9f..d85e5f869 100644 --- a/app/controllers/users/avatars_controller.rb +++ b/app/controllers/users/avatars_controller.rb @@ -10,7 +10,7 @@ class Users::AvatarsController < ApplicationController if @user.system? redirect_to view_context.image_path("system_user.png") elsif @user.avatar.attached? - redirect_to rails_blob_url(@user.avatar.variant(:thumb), disposition: "inline") + redirect_to rails_blob_url(@user.avatar_thumbnail, disposition: "inline") elsif stale? @user, cache_control: cache_control render_initials end diff --git a/app/models/user.rb b/app/models/user.rb index d70a260ea..8c0ac42a5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,12 +1,8 @@ class User < ApplicationRecord - include Accessor, Assignee, Attachable, Configurable, EmailAddressChangeable, + include Accessor, Assignee, Attachable, Avatar, Configurable, EmailAddressChangeable, Mentionable, Named, Notifiable, Role, Searcher, Watcher include Timelined # Depends on Accessor - has_one_attached :avatar do |attachable| - attachable.variant :thumb, resize_to_fill: [ 256, 256 ] - end - belongs_to :account belongs_to :identity, optional: true diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb new file mode 100644 index 000000000..e59bdfe6c --- /dev/null +++ b/app/models/user/avatar.rb @@ -0,0 +1,13 @@ +module User::Avatar + extend ActiveSupport::Concern + + included do + has_one_attached :avatar do |attachable| + attachable.variant :thumb, resize_to_fill: [ 256, 256 ] + end + end + + def avatar_thumbnail + avatar.variable? ? avatar.variant(:thumb) : avatar + end +end diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb index 737c28267..3ded884b8 100644 --- a/test/controllers/users/avatars_controller_test.rb +++ b/test/controllers/users/avatars_controller_test.rb @@ -32,7 +32,7 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest get user_avatar_path(users(:david)) - assert_redirected_to rails_blob_url(users(:david).avatar.variant(:thumb), disposition: "inline") + assert_redirected_to rails_blob_url(users(:david).avatar_thumbnail, disposition: "inline") end test "show other image redirects to the blob url" do @@ -41,7 +41,7 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest get user_avatar_path(users(:kevin)) - assert_redirected_to rails_blob_url(users(:kevin).avatar.variant(:thumb), disposition: "inline") + assert_redirected_to rails_blob_url(users(:kevin).avatar_thumbnail, disposition: "inline") end test "delete self" do diff --git a/test/fixtures/files/avatar.svg b/test/fixtures/files/avatar.svg new file mode 100644 index 000000000..1f0c5b682 --- /dev/null +++ b/test/fixtures/files/avatar.svg @@ -0,0 +1,3 @@ + + + diff --git a/test/models/user/avatar_test.rb b/test/models/user/avatar_test.rb new file mode 100644 index 000000000..2c2da0a56 --- /dev/null +++ b/test/models/user/avatar_test.rb @@ -0,0 +1,17 @@ +require "test_helper" + +class User::AvatarTest < ActiveSupport::TestCase + test "avatar_thumbnail returns variant for variable images" do + users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg") + + assert users(:david).avatar.variable? + assert_equal users(:david).avatar.variant(:thumb).blob, users(:david).avatar_thumbnail.blob + end + + test "avatar_thumbnail returns original blob for non-variable images" do + users(:david).avatar.attach(io: File.open(file_fixture("avatar.svg")), filename: "avatar.svg", content_type: "image/svg+xml") + + assert_not users(:david).avatar.variable? + assert_equal users(:david).avatar.blob, users(:david).avatar_thumbnail.blob + end +end From 2e47749739896c6fbae0a0b5562de5611c8ebce4 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Thu, 4 Dec 2025 14:24:39 +0000 Subject: [PATCH 2/2] Don't allow SVG avatar uploads in the first place --- app/models/user/avatar.rb | 11 +++++++++++ app/views/users/edit.html.erb | 2 +- test/models/user/avatar_test.rb | 13 +++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb index e59bdfe6c..a4b2ae631 100644 --- a/app/models/user/avatar.rb +++ b/app/models/user/avatar.rb @@ -1,13 +1,24 @@ module User::Avatar extend ActiveSupport::Concern + ALLOWED_AVATAR_CONTENT_TYPES = %w[ image/jpeg image/png image/gif image/webp ].freeze + included do has_one_attached :avatar do |attachable| attachable.variant :thumb, resize_to_fill: [ 256, 256 ] end + + validate :avatar_content_type_allowed end def avatar_thumbnail avatar.variable? ? avatar.variant(:thumb) : avatar end + + private + def avatar_content_type_allowed + if avatar.attached? && !ALLOWED_AVATAR_CONTENT_TYPES.include?(avatar.content_type) + errors.add(:avatar, "must be a JPEG, PNG, GIF, or WebP image") + end + end end diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index ab189c6ab..a3b5845e8 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -14,7 +14,7 @@ diff --git a/test/models/user/avatar_test.rb b/test/models/user/avatar_test.rb index 2c2da0a56..96ea5bac7 100644 --- a/test/models/user/avatar_test.rb +++ b/test/models/user/avatar_test.rb @@ -14,4 +14,17 @@ class User::AvatarTest < ActiveSupport::TestCase assert_not users(:david).avatar.variable? assert_equal users(:david).avatar.blob, users(:david).avatar_thumbnail.blob end + + test "allows valid image content types" do + users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg") + + assert users(:david).valid? + end + + test "rejects SVG uploads" do + users(:david).avatar.attach(io: File.open(file_fixture("avatar.svg")), filename: "avatar.svg") + + assert_not users(:david).valid? + assert_includes users(:david).errors[:avatar], "must be a JPEG, PNG, GIF, or WebP image" + end end