Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image attachment content type validation fix for ActiveStorage #4021

Merged
merged 2 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions core/app/models/spree/image/active_storage_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ module Spree::Image::ActiveStorageAttachment
delegate :width, :height, to: :attachment, prefix: true

included do
validates :attachment, presence: true
validate :attachment_is_an_image
validate :supported_content_type

has_attachment :attachment,
styles: {
mini: '48x48>',
Expand All @@ -15,7 +19,11 @@ module Spree::Image::ActiveStorageAttachment
large: '1200x1200>'
},
default_style: :product
validates :attachment, presence: true
validate :attachment_is_an_image

def supported_content_type
unless attachment.content_type.in?(Spree::Config.allowed_image_mime_types)
errors.add(:attachment, :content_type_not_supported)
end
end
end
end
2 changes: 1 addition & 1 deletion core/app/models/spree/image/paperclip_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module Spree::Image::PaperclipAttachment
convert_options: { all: '-strip -auto-orient -colorspace sRGB' }
validates_attachment :attachment,
presence: true,
content_type: { content_type: %w[image/jpeg image/jpg image/png image/gif] }
content_type: { content_type: Spree::Config.allowed_image_mime_types }

# save the w,h of the original image (from which others can be calculated)
# we need to look at the write-queue for images which have not been saved yet
Expand Down
8 changes: 8 additions & 0 deletions core/lib/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,14 @@ def payment_canceller
# Enumerable of images adhering to the present_image_class interface
class_name_attribute :image_attachment_module, default: 'Spree::Image::ActiveStorageAttachment'

# @!attribute [rw] allowed_image_mime_types
#
# Defines which MIME types are allowed for images
# `%w(image/jpeg image/jpg image/png image/gif).freeze` is the default.
#
# @return [Array]
class_name_attribute :allowed_image_mime_types, default: %w(image/jpeg image/jpg image/png image/gif).freeze
kennyadsl marked this conversation as resolved.
Show resolved Hide resolved

# Allows switching attachment library for Taxon
#
# `Spree::Taxon::ActiveStorageAttachment`
Expand Down
1 change: 1 addition & 0 deletions core/lib/spree/testing_support/fixtures/file.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This is a text file
6 changes: 4 additions & 2 deletions core/spec/lib/search/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require 'spree/core/product_filters'

RSpec.describe Spree::Core::Search::Base do
include ImageSpecHelper
before do
include Spree::Core::ProductFilters
@taxon = create(:taxon, name: "Ruby on Rails")
Expand All @@ -22,11 +23,12 @@

context "when include_images is included in the initialization params" do
let(:params) { { include_images: true, keyword: @product1.name, taxon: @taxon.id } }
let(:image_file) { open_image('blank.jpg') }
subject { described_class.new(params).retrieve_products }

before do
@product1.master.images.create(attachment_file_name: "Test", position: 2)
@product1.master.images.create(attachment_file_name: "Test1", position: 1)
@product1.master.images.create(attachment_file_name: 'test', attachment: image_file, position: 2)
@product1.master.images.create(attachment_file_name: 'test1', attachment: image_file, position: 1)
@product1.reload
end

Expand Down
13 changes: 12 additions & 1 deletion core/spec/models/spree/image_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,25 @@
require 'rails_helper'

RSpec.describe Spree::Image, type: :model do
include ImageSpecHelper
it_behaves_like 'an attachment' do
subject { create(:image) }
let(:attachment_name) { :attachment }
let(:default_style) { :product }
end

it 'is valid when attachment has allowed content type' do
image = build(:image, attachment: open_image('blank.jpg'))
expect(image).to be_valid
end

it 'is not valid when attachment has restricted content type' do
image = build(:image, attachment: open_image('file.txt'))
expect(image).to_not be_valid
end

describe 'attachment details' do
let(:image_file) { File.open(File.join('lib', 'spree', 'testing_support', 'fixtures', 'blank.jpg')) }
let(:image_file) { open_image('blank.jpg') }
subject { create(:image, attachment: image_file) }

it 'returns if attachment is present' do
Expand Down
7 changes: 7 additions & 0 deletions core/spec/support/image_spec_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

module ImageSpecHelper
def open_image(image)
File.open(File.join('lib', 'spree', 'testing_support', 'fixtures', image))
end
end
3 changes: 2 additions & 1 deletion core/spec/support/shared_examples/attachment.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# frozen_string_literal: true

RSpec.shared_examples 'an attachment' do
include ImageSpecHelper
context 'valid attachment' do
before do
subject.send(
:"#{attachment_name}=",
File.open(File.join('lib', 'spree', 'testing_support', 'fixtures', 'blank.jpg'))
open_image('blank.jpg')
)
end

Expand Down