replace subfolder on cdn url conversion between general cdn and s3 (#7764)

replace subfolder on cdn url conversion between general cdn and s3 (#7764)

When both a cdn URL and an s3 cdn URL defined, subfolder paths were leaking through to the s3 cdn URL. If we are replacing the cdn url with the s3_cdn url, we also need to make sure that the subpath is removed as well, as it appears in the original cdn url.

The test should give a fairly good gist of the situations - in subfolder situations where s3_cdn and a cdn is defined: asset_path returns the asset with a subfolder, in the form {cdn_url}/{subfolder}/{asset_path}

Currently this is being replaced to {s3_cdn_url}/{subfolder}/{asset_path} I am proposing we change this to: {s3_cdn_url}/{asset_path} as it seems like for s3_cdn urls we should not be carrying around app subfolder pathing anywhere we are looking up s3 paths.

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index cb1e0a1..dfb1d5e 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
 # frozen_string_literal: true
 require 'current_user'
 require 'canonical_url'
@@ -62,7 +63,8 @@ module ApplicationHelper
     if GlobalSetting.use_s3? && GlobalSetting.s3_cdn_url
       if GlobalSetting.cdn_url
-        path = path.gsub(GlobalSetting.cdn_url, GlobalSetting.s3_cdn_url)
+        folder = ActionController::Base.config.relative_url_root || "/"
+        path = path.gsub(File.join(GlobalSetting.cdn_url, folder, "/"), File.join(GlobalSetting.s3_cdn_url, "/"))
         # we must remove the subfolder path here, assets are uploaded to s3
         # without it getting involved
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index d55023f..25e7543 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
 # frozen_string_literal: true
 require 'rails_helper'
@@ -32,6 +33,13 @@ describe ApplicationHelper do
         expect(helper.preload_script("application")).to include('')
+      it "replaces cdn URLs with s3 cdn subfolder paths" do
+        global_setting :s3_cdn_url, ''
+        set_cdn_url ""
+        ActionController::Base.config.relative_url_root = "/community"
+        expect(helper.preload_script("application")).to include('')
+      end
       it "returns magic brotli mangling for brotli requests" do
         helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br'

GitHub sha: 893b5003

1 Like