Do not nest cloudflare image results (#21088)

forem · May 24, 2024 · 86801b4 · 86801b4
1 parent fe1ffa8
commit 86801b4
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 8 deletions.
diff --git a/app/services/images/optimizer.rb b/app/services/images/optimizer.rb
@@ -122,8 +122,7 @@ def self.get_imgproxy_endpoint
     end
 
     def self.extract_suffix_url(full_url)
-      prefix = "https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image"
-      return full_url unless full_url&.starts_with?(prefix)
+      return full_url unless full_url&.starts_with?(cloudflare_prefix)
 
       uri = URI.parse(full_url)
       match = uri.path.match(%r{https?.+})
@@ -135,7 +134,12 @@ def self.cloudflare_contextually_preferred?(img_src)
       return false unless cloudflare_enabled?
       return false unless FeatureFlag.enabled?(:cloudflare_preferred_for_hosted_images)
 
-      img_src&.start_with?("https://#{ApplicationConfig['AWS_BUCKET_NAME']}.s3.amazonaws.com")
+      img_src&.start_with?("https://#{ApplicationConfig['AWS_BUCKET_NAME']}.s3.amazonaws.com") ||
+        (img_src&.start_with?(cloudflare_prefix) && !img_src&.end_with?("/"))
+    end
+
+    def self.cloudflare_prefix
+      "https://#{ApplicationConfig['CLOUDFLARE_IMAGES_DOMAIN']}/cdn-cgi/image"
     end
   end
 end
diff --git a/spec/services/images/optimizer_spec.rb b/spec/services/images/optimizer_spec.rb
@@ -269,6 +269,14 @@
       )
       expect(cloudflare_url).to eq("https://#{cloudfare_domain}/cdn-cgi/image/width=821,height=420,fit=scale-down,gravity=auto,format=auto/")
     end
+
+    it "outputs the proper url when a cloudflare domain is passed instead of nesting" do
+      cloudflare_url = described_class.cloudflare(
+        "https://images.example.com/cdn-cgi/image/width=821,height=420,fit=scale-down,gravity=auto,format=auto/#{CGI.escape(image_url)}",
+        width: 821, height: 420,
+      )
+      expect(cloudflare_url).to eq("https://images.example.com/cdn-cgi/image/width=821,height=420,fit=scale-down,gravity=auto,format=auto/#{CGI.escape(image_url)}")
+    end
   end
 
   describe "#cloudinary_enabled?" do
@@ -335,21 +343,22 @@
     let(:aws_bucket_name) { "mybucket" }
     let(:hosted_image_url) { "https://#{aws_bucket_name}.s3.amazonaws.com/path/to/image.jpg" }
     let(:non_hosted_image_url) { "https://example.com/path/to/image.jpg" }
-  
+
     before do
       allow(ApplicationConfig).to receive(:[]).with("AWS_BUCKET_NAME").and_return(aws_bucket_name)
+      allow(ApplicationConfig).to receive(:[]).with("CLOUDFLARE_IMAGES_DOMAIN").and_return("cloudflareimage.com")
     end
-  
+
     context "when cloudflare is enabled and feature flag is on" do
       before do
         allow(described_class).to receive(:cloudflare_enabled?).and_return(true)
         allow(FeatureFlag).to receive(:enabled?).with(:cloudflare_preferred_for_hosted_images).and_return(true)
       end
-  
+
       it "returns true for hosted images" do
         expect(described_class.cloudflare_contextually_preferred?(hosted_image_url)).to be(true)
       end
-  
+
       it "returns false for non-hosted images" do
         expect(described_class.cloudflare_contextually_preferred?(non_hosted_image_url)).to be(false)
       end
@@ -359,7 +368,7 @@
       before do
         allow(described_class).to receive(:cloudflare_enabled?).and_return(false)
       end
-  
+
       it "returns false regardless of image source" do
         expect(described_class.cloudflare_contextually_preferred?(hosted_image_url)).to be(false)
         expect(described_class.cloudflare_contextually_preferred?(non_hosted_image_url)).to be(false)