diff --git a/app/models/message/attachment.rb b/app/models/message/attachment.rb index bc727ba..4b895ea 100644 --- a/app/models/message/attachment.rb +++ b/app/models/message/attachment.rb @@ -30,6 +30,7 @@ module Message::Attachment attachment&.analyze end + # A file that ffmpeg or libvips can't decode is still the message: post it without a preview. def process_attachment_thumbnail case when attachment.video? @@ -37,5 +38,7 @@ module Message::Attachment when attachment.representable? attachment.representation(:thumb).processed end + rescue ActiveStorage::PreviewError, Vips::Error => error + Rails.logger.warn "Posted #{attachment.filename} without a preview: #{error.class}: #{error.message.lines.first&.chomp}" end end diff --git a/test/controllers/messages_controller_test.rb b/test/controllers/messages_controller_test.rb index 8088694..e5bfc14 100644 --- a/test/controllers/messages_controller_test.rb +++ b/test/controllers/messages_controller_test.rb @@ -57,6 +57,18 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest end end + test "creating a message with an image that can't be decoded broadcasts the message to the room" do + webp = Vips::Image.new_from_file(file_fixture("moon.jpg").to_s).webpsave_buffer + broken = Rack::Test::UploadedFile.new(StringIO.new(webp.byteslice(0, webp.bytesize / 2)), "image/webp", original_filename: "broken.webp") + + post room_messages_url(@room, format: :turbo_stream), params: { message: { attachment: broken, client_message_id: 999 } } + + assert_response :success + assert_rendered_turbo_stream_broadcast @room, :messages, action: "append", target: [ @room, :messages ] do + assert_select ".message__body a[href*='broken.webp']" + end + end + test "creating a message broadcasts unread room to each member" do @room.users.each do |member| assert_broadcasts UnreadRoomsChannel.stream_name_for(member.id), 1 do diff --git a/test/models/message/attachment_test.rb b/test/models/message/attachment_test.rb index 47d71c1..6925eb0 100644 --- a/test/models/message/attachment_test.rb +++ b/test/models/message/attachment_test.rb @@ -19,6 +19,20 @@ class Message::AttachmentTest < ActiveSupport::TestCase assert_equal message.plain_text_body, "moon.jpg" end + test "creating a message keeps an image that can't be decoded" do + webp = Vips::Image.new_from_file(file_fixture("moon.jpg").to_s).webpsave_buffer + message = create_unreadable_attachment_message(webp.byteslice(0, webp.bytesize / 2), "broken.webp") + + assert_equal "broken.webp", message.reload.attachment.filename.to_s + assert_nil message.attachment.representation(:thumb).image + end + + test "creating a message keeps a video that can't be decoded" do + message = create_unreadable_attachment_message(file_fixture("alpha-centuri.mov").binread(64), "broken.mov") + + assert_equal "broken.mov", message.reload.attachment.filename.to_s + assert_not message.attachment.preview(format: :webp).image.attached? + end private def create_attachment_message(file, content_type) @@ -27,4 +41,11 @@ class Message::AttachmentTest < ActiveSupport::TestCase client_message_id: "message", attachment: fixture_file_upload(file, content_type) end + + def create_unreadable_attachment_message(content, filename) + rooms(:hq).messages.create_with_attachment! \ + creator: users(:david), + client_message_id: "message", + attachment: { io: StringIO.new(content), filename: filename } + end end