From 1884f082ee527d57022dc41e70c2adbe0e1fffec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 12 May 2025 18:07:10 +0400 Subject: [PATCH 1/2] Add note about variants not working when not generated ad-hoc --- app/controllers/avatars_controller.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/controllers/avatars_controller.rb b/app/controllers/avatars_controller.rb index c106384..6519c73 100644 --- a/app/controllers/avatars_controller.rb +++ b/app/controllers/avatars_controller.rb @@ -14,6 +14,9 @@ class AvatarsController < ApplicationController blob = if size == :original user.avatar.blob else + # TODO Variants use the same custom storage key/path, which + # makes blob downloads always fetch the original version instead + # of the variant. Needs to be fixed/added in Rails. user.avatar_variant(size: size)&.blob end From 417e3460746599f2749ed85252c8d8fc0ffe69f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Wed, 14 May 2025 14:42:03 +0400 Subject: [PATCH 2/2] Do not use ActiveStorage variants, process original avatar Variants are currently broken. So we process the original file with the most common avatar dimensions and stripping metadata, then hash and upload only that version. --- app/controllers/avatars_controller.rb | 18 +++++------ app/controllers/settings_controller.rb | 27 ++++++++++++++--- app/models/user.rb | 35 +++++++++++----------- app/services/ldap_manager/update_avatar.rb | 5 ++-- app/views/settings/_profile.html.erb | 3 +- 5 files changed, 53 insertions(+), 35 deletions(-) diff --git a/app/controllers/avatars_controller.rb b/app/controllers/avatars_controller.rb index 6519c73..8a7284e 100644 --- a/app/controllers/avatars_controller.rb +++ b/app/controllers/avatars_controller.rb @@ -5,22 +5,20 @@ class AvatarsController < ApplicationController sha256_hash = params[:hash] format = params[:format]&.to_sym || :png - size = params[:size]&.to_sym || :original + # size = params[:size]&.to_sym || :original unless user.avatar.filename.to_s == "#{sha256_hash}.#{format}" http_status :not_found and return end - blob = if size == :original - user.avatar.blob - else - # TODO Variants use the same custom storage key/path, which - # makes blob downloads always fetch the original version instead - # of the variant. Needs to be fixed/added in Rails. - user.avatar_variant(size: size)&.blob - end + # TODO See note for avatar_variant in user model + # blob = if size == :original + # user.avatar.blob + # else + # user.avatar_variant(size: size)&.blob + # end - data = blob.download + data = user.avatar.blob.download send_data data, type: "image/#{format}", disposition: "inline" else http_status :not_found diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index d9e202f..ef03455 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -191,9 +191,14 @@ class SettingsController < ApplicationController end def store_user_avatar - data = @user.avatar_new.tempfile.read - @user.avatar_new.tempfile.rewind - hash = Digest::SHA256.hexdigest(data) + io = @user.avatar_new.tempfile + img_data = process_avatar(io) + tempfile = Tempfile.create + tempfile.binmode + tempfile.write(img_data) + tempfile.rewind + + hash = Digest::SHA256.hexdigest(img_data) ext = @user.avatar_new.content_type == "image/png" ? "png" : "jpg" filename = "#{hash}.#{ext}" @@ -202,8 +207,22 @@ class SettingsController < ApplicationController false else key = "users/#{@user.cn}/avatars/#{filename}" - @user.avatar.attach io: @user.avatar_new.tempfile, key: key, filename: filename + @user.avatar.attach io: tempfile, key: key, filename: filename @user.save end end + + def process_avatar(io) + processed = ImageProcessing::Vips + .source(io) + .resize_to_fill(400, 400) + .saver(strip: true) + .call + io.rewind + processed.read + rescue Vips::Error => e + Sentry.capture_exception(e) if Setting.sentry_enabled? + Rails.logger.error { "Image processing failed for avatar: #{e.message}" } + nil + end end diff --git a/app/models/user.rb b/app/models/user.rb index 502179d..a0c49b0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -56,6 +56,7 @@ class User < ApplicationRecord validates_length_of :display_name, minimum: 3, maximum: 35, allow_blank: true, if: -> { defined?(@display_name) } + validate :acceptable_avatar validate :acceptable_pgp_key_format, if: -> { defined?(@pgp_pubkey) && @pgp_pubkey.present? } @@ -83,6 +84,10 @@ class User < ApplicationRecord :timeoutable, :rememberable + # + # Methods + # + def ldap_before_save self.email = Devise::LDAP::Adapter.get_ldap_param(self.cn, "mail").first self.ou = dn.split(',') @@ -165,23 +170,19 @@ class User < ApplicationRecord @display_name ||= ldap_entry[:display_name] end - def avatar_base64(size: :medium) - return nil unless avatar.attached? - variant = avatar_variant(size: size) - data = ActiveStorage::Blob.service.download(variant.key) - Base64.strict_encode64(data) - end - - def avatar_variant(size: :medium) - dimensions = case size - when :large then [400, 400] - when :medium then [256, 256] - when :small then [64, 64] - else [256, 256] - end - format = avatar.content_type == "image/png" ? :png : :jpeg - avatar.variant(resize_to_fill: dimensions, format: format) - end + # TODO Variant keys are currently broken for some reason + # (They use the same key as the main blob, when it should be + # "/variants/#{key)" + # def avatar_variant(size: :medium) + # dimensions = case size + # when :large then [400, 400] + # when :medium then [256, 256] + # when :small then [64, 64] + # else [256, 256] + # end + # format = avatar.content_type == "image/png" ? :png : :jpeg + # avatar.variant(resize_to_fill: dimensions, format: format) + # end def nostr_pubkey @nostr_pubkey ||= ldap_entry[:nostr_key] diff --git a/app/services/ldap_manager/update_avatar.rb b/app/services/ldap_manager/update_avatar.rb index 163af3f..793409d 100644 --- a/app/services/ldap_manager/update_avatar.rb +++ b/app/services/ldap_manager/update_avatar.rb @@ -14,15 +14,16 @@ module LdapManager end img_data = @user.avatar.blob.download - jpg_data = process(img_data) + jpg_data = process_avatar + Rails.logger.debug { "Storing new jpegPhoto for user #{@user.cn} in LDAP" } result = replace_attribute(@dn, :jpegPhoto, jpg_data) result == 0 end private - def process(data) + def process_avatar @user.avatar.blob.open do |file| processed = ImageProcessing::Vips .source(file) diff --git a/app/views/settings/_profile.html.erb b/app/views/settings/_profile.html.erb index 7dc0f22..50bdf7e 100644 --- a/app/views/settings/_profile.html.erb +++ b/app/views/settings/_profile.html.erb @@ -38,8 +38,7 @@
<% if @user.avatar.attached? %>

- <%= image_tag @user.avatar_variant(size: :medium), - class: "h-24 w-24 rounded-lg" %> + <%= image_tag image_url_for(@user.avatar), class: "h-24 w-24 rounded-lg" %>

<% end %>