FEATURE: allow for local theme js assets (#16374)
Due to default CSP web workers instantiated from CDN based assets are still treated as "same-origin" meaning that we had no way of safely instansiating a web worker from a theme. This limits the theme system and adds the arbitrary restriction that WASM based components can not be safely used. To resolve this limitation all js assets in about.json are also cached on local domain. { "name": "Header Icons", "assets" : { "worker" : "assets/worker.js" } } This can then be referenced in JS via: settings.theme_uploads_local.worker local_js_assets are unconditionally served from the site directly and bypass the entire CDN, using the pre-existing JavascriptCache Previous to this change this code was completely dormant on sites which used s3 based uploads, this reuses the very well tested and cached asset system on s3 based sites. Note, when creating local_js_assets it is highly recommended to keep the assets lean and keep all the heavy working in CDN based assets. For example wasm files can still live on the CDN but the lean worker that loads it can live on local. This change unlocks wasm in theme components, so wasm is now also allowed in `theme_authorized_extensions` * more usages of upload.content * add a specific test for upload.content * Adjust logic to ensure that after upgrades we still get a cached local js on save
This commit is contained in:
parent
ef2e4f7ee0
commit
cedcdb0057
|
@ -8,11 +8,19 @@ class JavascriptCache < ActiveRecord::Base
|
|||
before_save :update_digest
|
||||
|
||||
def url
|
||||
"#{GlobalSetting.cdn_url}#{Discourse.base_path}/theme-javascripts/#{digest}.js?__ws=#{Discourse.current_hostname}"
|
||||
"#{GlobalSetting.cdn_url}#{Discourse.base_path}#{path}"
|
||||
end
|
||||
|
||||
def local_url
|
||||
"#{Discourse.base_url}#{path}"
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def path
|
||||
"/theme-javascripts/#{digest}.js?__ws=#{Discourse.current_hostname}"
|
||||
end
|
||||
|
||||
def update_digest
|
||||
self.digest = Digest::SHA1.hexdigest(content) if content_changed?
|
||||
end
|
||||
|
|
|
@ -539,12 +539,19 @@ class Theme < ActiveRecord::Base
|
|||
end
|
||||
|
||||
theme_uploads = {}
|
||||
theme_uploads_local = {}
|
||||
|
||||
upload_fields.each do |field|
|
||||
if field.upload&.url
|
||||
theme_uploads[field.name] = Discourse.store.cdn_url(field.upload.url)
|
||||
end
|
||||
if field.javascript_cache
|
||||
theme_uploads_local[field.name] = field.javascript_cache.local_url
|
||||
end
|
||||
end
|
||||
|
||||
hash['theme_uploads'] = theme_uploads if theme_uploads.present?
|
||||
hash['theme_uploads_local'] = theme_uploads_local if theme_uploads_local.present?
|
||||
|
||||
hash
|
||||
end
|
||||
|
@ -652,7 +659,7 @@ class Theme < ActiveRecord::Base
|
|||
end
|
||||
|
||||
settings_hash&.each do |name, value|
|
||||
next if name == "theme_uploads"
|
||||
next if name == "theme_uploads" || name == "theme_uploads_local"
|
||||
contents << to_scss_variable(name, value)
|
||||
end
|
||||
|
||||
|
|
|
@ -184,7 +184,7 @@ class ThemeField < ActiveRecord::Base
|
|||
|
||||
begin
|
||||
content = File.read(path)
|
||||
svg_file = Nokogiri::XML(content) do |config|
|
||||
Nokogiri::XML(content) do |config|
|
||||
config.options = Nokogiri::XML::ParseOptions::NOBLANKS
|
||||
end
|
||||
rescue => e
|
||||
|
@ -568,6 +568,12 @@ class ThemeField < ActiveRecord::Base
|
|||
if (will_save_change_to_value? || will_save_change_to_upload_id?) && !will_save_change_to_value_baked?
|
||||
self.value_baked = nil
|
||||
end
|
||||
if upload && upload.extension == "js"
|
||||
if will_save_change_to_upload_id? || !javascript_cache
|
||||
javascript_cache ||= build_javascript_cache
|
||||
javascript_cache.content = upload.content
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
after_save do
|
||||
|
|
|
@ -171,6 +171,22 @@ class Upload < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def content
|
||||
original_path = Discourse.store.path_for(self)
|
||||
external_copy = nil
|
||||
|
||||
if original_path.blank?
|
||||
external_copy = Discourse.store.download(self)
|
||||
original_path = external_copy.path
|
||||
end
|
||||
|
||||
File.read(original_path)
|
||||
ensure
|
||||
if external_copy
|
||||
File.unlink(external_copy.path)
|
||||
end
|
||||
end
|
||||
|
||||
def fix_image_extension
|
||||
return false if extension == "unknown"
|
||||
|
||||
|
|
|
@ -1274,7 +1274,7 @@ files:
|
|||
default: 50000
|
||||
max: 1024000
|
||||
theme_authorized_extensions:
|
||||
default: "jpg|jpeg|png|woff|woff2|svg|eot|ttf|otf|gif|webp|js"
|
||||
default: "wasm|jpg|jpeg|png|woff|woff2|svg|eot|ttf|otf|gif|webp|js"
|
||||
type: list
|
||||
list_type: compact
|
||||
authorized_extensions:
|
||||
|
|
|
@ -237,24 +237,7 @@ module SvgSprite
|
|||
|
||||
custom_sprite_paths = Dir.glob(plugin_paths)
|
||||
|
||||
if theme_id.present?
|
||||
ThemeField.where(type_id: ThemeField.types[:theme_upload_var], name: THEME_SPRITE_VAR_NAME, theme_id: Theme.transform_ids(theme_id))
|
||||
.pluck(:upload_id).each do |upload_id|
|
||||
|
||||
upload = Upload.find(upload_id) rescue nil
|
||||
|
||||
if Discourse.store.external?
|
||||
external_copy = Discourse.store.download(upload) rescue nil
|
||||
original_path = external_copy.try(:path)
|
||||
else
|
||||
original_path = Discourse.store.path_for(upload)
|
||||
end
|
||||
|
||||
custom_sprite_paths << original_path if original_path.present?
|
||||
end
|
||||
end
|
||||
|
||||
custom_sprite_paths.map do |path|
|
||||
custom_sprites = custom_sprite_paths.map do |path|
|
||||
if File.exist?(path)
|
||||
{
|
||||
filename: "#{File.basename(path, ".svg")}",
|
||||
|
@ -262,6 +245,25 @@ module SvgSprite
|
|||
}
|
||||
end
|
||||
end
|
||||
|
||||
if theme_id.present?
|
||||
ThemeField.where(type_id: ThemeField.types[:theme_upload_var], name: THEME_SPRITE_VAR_NAME, theme_id: Theme.transform_ids(theme_id))
|
||||
.pluck(:upload_id, :theme_id).each do |upload_id, child_theme_id|
|
||||
|
||||
begin
|
||||
upload = Upload.find(upload_id)
|
||||
custom_sprites << {
|
||||
filename: "theme_#{theme_id}_#{upload_id}.svg",
|
||||
sprite: upload.content
|
||||
}
|
||||
rescue => e
|
||||
name = Theme.find(child_theme_id).name rescue nil
|
||||
Discourse.warn_exception(e, "#{name} theme contains a corrupt svg upload")
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
custom_sprites
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -483,7 +485,7 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL
|
|||
|
||||
# Need to load full records for default values
|
||||
Theme.where(id: theme_ids).each do |theme|
|
||||
settings = theme.cached_settings.each do |key, value|
|
||||
_settings = theme.cached_settings.each do |key, value|
|
||||
if key.to_s.include?("_icon") && String === value
|
||||
theme_icon_settings |= value.split('|')
|
||||
end
|
||||
|
|
|
@ -110,7 +110,7 @@ class ThemeStore::GitImporter
|
|||
else
|
||||
Discourse::Utils.execute_command(git_ssh_command, "git", "clone", @url, @temp_folder)
|
||||
end
|
||||
rescue RuntimeError => err
|
||||
rescue RuntimeError
|
||||
raise RemoteTheme::ImportError.new(I18n.t("themes.import_error.git"))
|
||||
end
|
||||
ensure
|
||||
|
|
|
@ -53,8 +53,7 @@ class ThemeStore::ZipExporter
|
|||
raise RuntimeError.new("Theme exporter tried to leave directory") unless path.to_s.starts_with?(destination_folder)
|
||||
|
||||
if ThemeField.types[field.type_id] == :theme_upload_var
|
||||
filename = Discourse.store.path_for(field.upload)
|
||||
content = filename ? File.read(filename) : Discourse.store.download(field.upload).read
|
||||
content = field.upload.content
|
||||
else
|
||||
content = field.value
|
||||
end
|
||||
|
|
|
@ -213,7 +213,7 @@ describe SvgSprite do
|
|||
end
|
||||
|
||||
it "includes Font Awesome icon from groups" do
|
||||
group = Fabricate(:group, flair_icon: "far-building")
|
||||
_group = Fabricate(:group, flair_icon: "far-building")
|
||||
expect(SvgSprite.bundle).to match(/far-building/)
|
||||
end
|
||||
|
||||
|
|
|
@ -174,7 +174,7 @@ HTML
|
|||
|
||||
it "correctly handles extra JS fields" do
|
||||
js_field = theme.set_field(target: :extra_js, name: "discourse/controllers/discovery.js.es6", value: "import 'discourse/lib/ajax'; console.log('hello from .js.es6');")
|
||||
js_2_field = theme.set_field(target: :extra_js, name: "discourse/controllers/discovery-2.js", value: "import 'discourse/lib/ajax'; console.log('hello from .js');")
|
||||
_js_2_field = theme.set_field(target: :extra_js, name: "discourse/controllers/discovery-2.js", value: "import 'discourse/lib/ajax'; console.log('hello from .js');")
|
||||
hbs_field = theme.set_field(target: :extra_js, name: "discourse/templates/discovery.hbs", value: "{{hello-world}}")
|
||||
raw_hbs_field = theme.set_field(target: :extra_js, name: "discourse/templates/discovery.hbr", value: "{{hello-world}}")
|
||||
hbr_field = theme.set_field(target: :extra_js, name: "discourse/templates/other_discovery.hbr", value: "{{hello-world}}")
|
||||
|
@ -429,4 +429,90 @@ HTML
|
|||
end
|
||||
end
|
||||
|
||||
context 'local js assets' do
|
||||
|
||||
let :js_content do
|
||||
"// not transpiled; console.log('hello world');"
|
||||
end
|
||||
|
||||
let :upload_file do
|
||||
tmp = Tempfile.new(["jsfile", ".js"])
|
||||
File.write(tmp.path, js_content)
|
||||
tmp
|
||||
end
|
||||
|
||||
after do
|
||||
upload_file.unlink
|
||||
end
|
||||
|
||||
it "correctly handles local JS asset caching" do
|
||||
|
||||
upload = UploadCreator.new(upload_file, "test.js", for_theme: true)
|
||||
.create_for(Discourse::SYSTEM_USER_ID)
|
||||
|
||||
js_field = theme.set_field(
|
||||
target: :common,
|
||||
type_id: ThemeField.types[:theme_upload_var],
|
||||
name: 'test_js',
|
||||
upload_id: upload.id
|
||||
)
|
||||
|
||||
common_field = theme.set_field(
|
||||
target: :common,
|
||||
name: "head_tag",
|
||||
value: "<script>let c = 'd';</script>",
|
||||
type: :html
|
||||
)
|
||||
|
||||
theme.set_field(
|
||||
target: :settings,
|
||||
type: :yaml,
|
||||
name: "yaml",
|
||||
value: "hello: world"
|
||||
)
|
||||
|
||||
theme.set_field(
|
||||
target: :extra_js,
|
||||
name: "discourse/controllers/discovery.js.es6",
|
||||
value: "import 'discourse/lib/ajax'; console.log('hello from .js.es6');"
|
||||
)
|
||||
|
||||
theme.save!
|
||||
|
||||
# a bit fragile, but at least we test it properly
|
||||
[theme.reload.javascript_cache.content, common_field.reload.javascript_cache.content].each do |js|
|
||||
js_to_eval = <<~JS
|
||||
var settings;
|
||||
var window = {};
|
||||
var require = function(name) {
|
||||
if(name == "discourse/lib/theme-settings-store") {
|
||||
return({
|
||||
registerSettings: function(id, s) {
|
||||
settings = s;
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
window.require = require;
|
||||
#{js}
|
||||
settings
|
||||
JS
|
||||
|
||||
ctx = MiniRacer::Context.new
|
||||
val = ctx.eval(js_to_eval)
|
||||
ctx.dispose
|
||||
|
||||
expect(val["theme_uploads"]["test_js"]).to eq(js_field.upload.url)
|
||||
expect(val["theme_uploads_local"]["test_js"]).to eq(js_field.javascript_cache.local_url)
|
||||
|
||||
end
|
||||
|
||||
# this is important, we do not want local_js_urls to leak into scss
|
||||
expect(theme.scss_variables).to include("$hello: unquote(\"world\");")
|
||||
expect(theme.scss_variables).to include("$test_js: unquote(\"#{upload.url}\");")
|
||||
|
||||
expect(theme.scss_variables).not_to include("theme_uploads")
|
||||
end
|
||||
end
|
||||
|
||||
end
|
||||
|
|
|
@ -204,6 +204,13 @@ describe Upload do
|
|||
setup_s3
|
||||
end
|
||||
|
||||
it "can download an s3 upload" do
|
||||
stub_request(:get, upload.url)
|
||||
.to_return(status: 200, body: "hello", headers: {})
|
||||
|
||||
expect(upload.content).to eq("hello")
|
||||
end
|
||||
|
||||
it "should return the right upload when using base url (not CDN) for s3" do
|
||||
upload
|
||||
expect(Upload.get_from_url(upload.url)).to eq(upload)
|
||||
|
|
Loading…
Reference in New Issue