From 94cfe98ee4e67d8523da865ee062d8262823adf0 Mon Sep 17 00:00:00 2001
From: Rafael dos Santos Silva <xfalcox@gmail.com>
Date: Thu, 5 May 2022 15:13:17 -0300
Subject: [PATCH] FEATURE: Validate setting combination between exif strip and
 img opt (#16662)

Admins won't be able to disable strip_image_metadata if they don't
disable composer_media_optimization_image_enabled first since the later
will strip the same metadata on client during upload, making disabling
the former have no effect.

Bug report at https://meta.discourse.org/t/-/223350
---
 config/locales/server.en.yml               |  1 +
 lib/site_settings/validations.rb           |  6 ++++
 spec/lib/site_settings/validations_spec.rb | 34 ++++++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 4793883c6a1..ee56eba9b74 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -219,6 +219,7 @@ en:
       cors_origins_should_not_have_trailing_slash: "You should not add the trailing slash (/) to CORS origins."
       slow_down_crawler_user_agent_must_be_at_least_3_characters: "User agents must be at least 3 characters long to avoid incorrectly rate limiting human users."
       slow_down_crawler_user_agent_cannot_be_popular_browsers: "You cannot add any of the following values to the setting: %{values}."
+      strip_image_metadata_cannot_be_disabled_if_composer_media_optimization_image_enabled: "You cannot disable strip image metadata if 'composer media optimization image enabled' is enabled. Disable 'composer media optimization image enabled' before disabling strip image metadata."
     conflicting_google_user_id: 'The Google Account ID for this account has changed; staff intervention is required for security reasons. Please contact staff and point them to <br><a href="https://meta.discourse.org/t/76575">https://meta.discourse.org/t/76575</a>'
     onebox:
       invalid_address: "Sorry, we were unable to generate a preview for this web page, because the server '%{hostname}' could not be found. Instead of a preview, only a link will appear in your post. :cry:"
diff --git a/lib/site_settings/validations.rb b/lib/site_settings/validations.rb
index 2c23f0b0e24..cd2490628b3 100644
--- a/lib/site_settings/validations.rb
+++ b/lib/site_settings/validations.rb
@@ -239,6 +239,12 @@ module SiteSettings::Validations
     end
   end
 
+  def validate_strip_image_metadata(new_val)
+    return if new_val == "t"
+    return if SiteSetting.composer_media_optimization_image_enabled == false
+    validate_error :strip_image_metadata_cannot_be_disabled_if_composer_media_optimization_image_enabled
+  end
+
   private
 
   def validate_bucket_setting(setting_name, upload_bucket, backup_bucket)
diff --git a/spec/lib/site_settings/validations_spec.rb b/spec/lib/site_settings/validations_spec.rb
index 70f3559fa65..a0a86913a88 100644
--- a/spec/lib/site_settings/validations_spec.rb
+++ b/spec/lib/site_settings/validations_spec.rb
@@ -368,4 +368,38 @@ describe SiteSettings::Validations do
       }.to raise_error(Discourse::InvalidParameters, popular_browser_message)
     end
   end
+
+  describe "strip image metadata and composer media optimization interplay" do
+    describe "#validate_strip_image_metadata" do
+      let(:error_message) { I18n.t("errors.site_settings.strip_image_metadata_cannot_be_disabled_if_composer_media_optimization_image_enabled") }
+
+      context "when the new value is false" do
+        context "when composer_media_optimization_image_enabled is enabled" do
+          before do
+            SiteSetting.composer_media_optimization_image_enabled = true
+          end
+
+          it "should raise an error" do
+            expect { subject.validate_strip_image_metadata("f") }.to raise_error(Discourse::InvalidParameters, error_message)
+          end
+        end
+
+        context "when composer_media_optimization_image_enabled is disabled" do
+          before do
+            SiteSetting.composer_media_optimization_image_enabled = false
+          end
+
+          it "should be ok" do
+            expect { subject.validate_strip_image_metadata("f") }.not_to raise_error
+          end
+        end
+      end
+
+      context "when the new value is true" do
+        it "should be ok" do
+          expect { subject.validate_strip_image_metadata("t") }.not_to raise_error
+        end
+      end
+    end
+  end
 end