Merge pull request #10201 from saunmanoj888/fix-invalid-image-upload

Fix corrupt and invalid image upload issue

app/models/enterprise.rb

@@ -85,8 +85,12 @@ class Enterprise < ApplicationRecord
   has_one_attached :promo_image
   has_one_attached :terms_and_conditions
 
-  validates :logo, content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
-  validates :promo_image, content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+  validates :logo,
+            processable_image: true,
+            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+  validates :promo_image,
+            processable_image: true,
+            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
   validates :terms_and_conditions, content_type: {
     in: "application/pdf",
     message: I18n.t(:enterprise_terms_and_conditions_type_error),

app/models/enterprise_group.rb

@@ -28,8 +28,12 @@ class EnterpriseGroup < ApplicationRecord
   has_one_attached :logo
   has_one_attached :promo_image
 
-  validates :logo, content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
-  validates :promo_image, content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+  validates :logo,
+            processable_image: true,
+            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+  validates :promo_image,
+            processable_image: true,
+            content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
 
   scope :by_position, -> { order('position ASC') }
   scope :on_front_page, -> { where(on_front_page: true) }

app/models/spree/image.rb

@@ -11,8 +11,10 @@ module Spree
 
     has_one_attached :attachment
 
-    validates :attachment, attached: true, 
-      content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
+    validates :attachment,
+              attached: true,
+              processable_image: true,
+              content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
     validate :no_attachment_errors
 
     def variant(name)

app/models/spree/product.rb

@@ -114,6 +114,7 @@ module Spree
               presence: { if: ->(p) { %w(weight volume).include? p.variant_unit } }
     validates :variant_unit_name,
               presence: { if: ->(p) { p.variant_unit == 'items' } }
+    validate :validate_image_for_master
 
     attr_accessor :option_values_hash
 
@@ -474,5 +475,11 @@ module Spree
       requested = permalink.presence || permalink_was.presence || name.presence || 'product'
       self.permalink = create_unique_permalink(requested.parameterize)
     end
+
+    def validate_image_for_master
+      return if master.images.all?(&:valid?)
+
+      errors.add(:base, I18n.t('spree.admin.products.image_not_processable'))
+    end
   end
 end

config/locales/en.yml

@@ -125,6 +125,7 @@ en:
       aspect_ratio_not_landscape: "must be a landscape image"
       aspect_ratio_is_not: "must have an aspect ratio of %{aspect_ratio}"
       aspect_ratio_unknown: "has an unknown aspect ratio"
+      image_not_processable: "is not a valid image"
 
   stripe:
     error_code:
@@ -4139,6 +4140,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
             no_payment_via_admin_backend: Paypal payments cannot be captured in the Backoffice
       products:
         image_upload_error: "Please upload the image in JPG, PNG, GIF, SVG or WEBP format."
+        image_not_processable: "Image attachment is not a valid image."
         new:
           title: "New Product"
           new_product: "New Product"

spec/controllers/api/v0/product_images_controller_spec.rb

@@ -25,7 +25,7 @@ describe Api::V0::ProductImagesController, type: :controller do
       }
 
       expect(response.status).to eq 201
-      expect(product_without_image.images.first.id).to eq json_response['id']
+      expect(product_without_image.reload.images.first.id).to eq json_response['id']
     end
 
     it "updates an existing product image" do
@@ -34,7 +34,7 @@ describe Api::V0::ProductImagesController, type: :controller do
       }
 
       expect(response.status).to eq 200
-      expect(product_with_image.images.first.id).to eq json_response['id']
+      expect(product_with_image.reload.images.first.id).to eq json_response['id']
     end
 
     it "reports errors when saving fails" do

spec/models/spree/product_spec.rb

@@ -460,6 +460,25 @@ module Spree
           expect(product).not_to be_valid
         end
       end
+
+      describe "#validate_image_for_master" do
+        let(:product) { build_stubbed(:simple_product) }
+
+        context "when the image attached to the master variant is invalid" do
+          before { product.master.images.new.errors.add(:image_not_processable, "invalid") }
+
+          it "adds an error message to the base object" do
+            expect(product).not_to be_valid
+            expect(product.errors[:base]).to include('Image attachment is not a valid image.')
+          end
+        end
+
+        context "when master variant is valid" do
+          it "returns true" do
+            expect(product).to be_valid
+          end
+        end
+      end
     end
 
     describe "callbacks" do

spec/services/image_importer_spec.rb

@@ -15,7 +15,7 @@ describe ImageImporter do
       }.by(1)
 
       expect(product.images.count).to eq 1
-      expect(product.images.first.attachment_blob.byte_size).to eq 6274
+      expect(product.reload.images.first.attachment_blob.byte_size).to eq 6274
     end
   end
 end

spec/system/admin/products_spec.rb

@@ -618,7 +618,7 @@ describe '
       click_button "Create"
 
       expect(page).to have_text "Attachment has an invalid content type"
-      expect(page).to have_text "Please upload the image in JPG, PNG, GIF, SVG or WEBP format."
+      expect(page).to have_text "Attachment is not a valid image"
     end
 
     it "deleting product images" do