diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 61fe6f6dbc2..0fe7bd1585e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -53,7 +53,7 @@ class ApplicationController < ActionController::Base after_action :add_noindex_header_to_non_canonical, if: :spa_boot_request? after_action :set_cross_origin_opener_policy_header, if: :spa_boot_request? after_action :clean_xml, if: :is_feed_response? - around_action :add_link_header, if: -> { spa_boot_request? } + around_action :add_early_hint_header, if: -> { spa_boot_request? } HONEYPOT_KEY ||= "HONEYPOT_KEY" CHALLENGE_KEY ||= "CHALLENGE_KEY" @@ -1096,29 +1096,29 @@ class ApplicationController < ActionController::Base result end - def add_link_header - @links_to_preload = [] if GlobalSetting.preload_link_header + # We don't actually send 103 Early Hint responses from Discourse. However, upstream proxies can be configured + # to cache a response header from the app and use that to send an Early Hint response to future clients. + # See 'early_hint_header_mode' and 'early_hint_header_name' Global Setting descriptions for more info. + def add_early_hint_header + return yield if GlobalSetting.early_hint_header_mode.nil? + + @asset_preload_links = [] yield links = [] - if SiteSetting.experimental_preconnect_link_header + if GlobalSetting.early_hint_header_mode == "preconnect" [GlobalSetting.cdn_url, SiteSetting.s3_cdn_url].each do |url| next if url.blank? base_url = URI.join(url, "/").to_s.chomp("/") - links.push("<#{base_url}>; rel=preconnect") - # Not all browsers support the preconnect resource hint so we are adding dns-prefetch as the fallback - links.push("<#{base_url}>; rel=dns-prefetch") end + elsif GlobalSetting.early_hint_header_mode == "preload" + links.push(*@asset_preload_links) end - if GlobalSetting.preload_link_header && !@links_to_preload.empty? - links = links.concat(@links_to_preload) - end - - response.headers["Link"] = links.join(", ") if links.present? + response.headers[GlobalSetting.early_hint_header_name] = links.join(", ") if links.present? end def spa_boot_request? diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5af27c985bd..45288529127 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -158,8 +158,8 @@ module ApplicationHelper end def add_resource_preload_list(resource_url, type) - if !@links_to_preload.nil? - @links_to_preload << %Q(<#{resource_url}>; rel="preload"; as="#{type}") + if !@asset_preload_links.nil? + @asset_preload_links << %Q(<#{resource_url}>; rel="preload"; as="#{type}") end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index de9a6b711c9..7051b794911 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -7,7 +7,7 @@ - <%- if GlobalSetting.preload_link_header %> + <%- if GlobalSetting.early_hint_header_mode == "prefetch" %> <%= render partial: "common/discourse_preload_stylesheet" %> <%- end %> <%= render partial: "layouts/head" %> @@ -23,13 +23,11 @@ <%= build_plugin_html 'server:before-script-load' %> - <%- if GlobalSetting.preload_link_header %> - <% add_resource_preload_list(script_asset_path("start-discourse"), "script") %> - <% add_resource_preload_list(script_asset_path("browser-update"), "script") %> - <%- else %> - " as="script" nonce="<%= csp_nonce_placeholder %>"> - " as="script" nonce="<%= csp_nonce_placeholder %>"> - <%- end %> + <% add_resource_preload_list(script_asset_path("start-discourse"), "script") %> + <% add_resource_preload_list(script_asset_path("browser-update"), "script") %> + " as="script" nonce="<%= csp_nonce_placeholder %>"> + " as="script" nonce="<%= csp_nonce_placeholder %>"> + <%= preload_script 'browser-detect' %> <%= preload_script "vendor" %> diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf index 04db7f940dc..9da80716163 100644 --- a/config/discourse_defaults.conf +++ b/config/discourse_defaults.conf @@ -366,8 +366,12 @@ enable_long_polling = # Length of time to hold open a long polling connection in milliseconds long_polling_interval = -# Moves asset preloading from tags in the response document head to response headers -preload_link_header = false +# Specify the mode for the early hint header. Can be nil (disabled), "preconnect" (lists just CDN domains) or "preload" (lists all assets). +# The 'preload' mode currently serves inconsistent headers for different pages/users, and is not recommended for production use. +early_hint_header_mode = + +# Specify which header name to use for the early hint. Defaults to "Link", but can be changed to support different proxy mechanisms. +early_hint_header_name = "Link" # When using an external upload store, redirect `user_avatar` requests instead of proxying redirect_avatar_requests = false diff --git a/config/site_settings.yml b/config/site_settings.yml index f9b4a73d525..70c05c6c0c5 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2368,9 +2368,6 @@ developer: experimental_objects_type_for_theme_settings: default: false hidden: true - experimental_preconnect_link_header: - default: false - hidden: true navigation: navigation_menu: diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 6526ba884d4..93718f2b01b 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -133,42 +133,42 @@ RSpec.describe ApplicationHelper do describe "add_resource_preload_list" do it "adds resources to the preload list when it's available" do - @links_to_preload = [] + @asset_preload_links = [] add_resource_preload_list("/assets/start-discourse.js", "script") add_resource_preload_list("/assets/discourse.css", "style") - expect(@links_to_preload.size).to eq(2) + expect(@asset_preload_links.size).to eq(2) end it "doesn't add resources to the preload list when it's not available" do - @links_to_preload = nil + @asset_preload_links = nil add_resource_preload_list("/assets/start-discourse.js", "script") add_resource_preload_list("/assets/discourse.css", "style") - expect(@links_to_preload).to eq(nil) + expect(@asset_preload_links).to eq(nil) end it "adds resources to the preload list when preload_script is called" do - @links_to_preload = [] + @asset_preload_links = [] helper.preload_script("start-discourse") - expect(@links_to_preload.size).to eq(1) + expect(@asset_preload_links.size).to eq(1) end it "adds resources to the preload list when discourse_stylesheet_link_tag is called" do - @links_to_preload = [] + @asset_preload_links = [] helper.discourse_stylesheet_link_tag(:desktop) - expect(@links_to_preload.size).to eq(1) + expect(@asset_preload_links.size).to eq(1) end it "adds resources as the correct type" do - @links_to_preload = [] + @asset_preload_links = [] helper.discourse_stylesheet_link_tag(:desktop) helper.preload_script("start-discourse") - expect(@links_to_preload[0]).to match(/as="style"/) - expect(@links_to_preload[1]).to match(/as="script"/) + expect(@asset_preload_links[0]).to match(/as="style"/) + expect(@asset_preload_links[1]).to match(/as="script"/) end end diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index be4e3d2ccd9..86d34d8803c 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -1188,60 +1188,50 @@ RSpec.describe ApplicationController do end end - describe "Link header" do - describe "when `experimental_preconnect_link_header` site setting is enabled" do - before { SiteSetting.experimental_preconnect_link_header = true } + describe "Early hint header" do + before { global_setting :cdn_url, "https://cdn.example.com/something" } - it "should include the `preconnect` and `dns-prefetch` resource hints in the Link header when `GlobalSetting.cdn_url is configured`" do - global_setting :cdn_url, "https://cdn.example.com/something" + it "is not included by default" do + get "/latest" + expect(response.status).to eq(200) + expect(response.headers["Link"]).to eq(nil) + end + context "when in preconnect mode" do + before { global_setting :early_hint_header_mode, "preconnect" } + + it "includes the preconnect hint" do get "/latest" - expect(response.status).to eq(200) - - expect(response.headers["Link"]).to include( - "; rel=preconnect, ; rel=dns-prefetch", - ) + expect(response.headers["Link"]).to include("; rel=preconnect") + expect(response.headers["Link"]).not_to include("rel=preload") end - it "should include the `preconnect` and `dns-prefetch` resource hints in the Link header when `SiteSetting.s3_cdn_url is configured`" do - SiteSetting.s3_cdn_url = "https://s3.some-cdn.com/something" - + it "can use a different header" do + global_setting :early_hint_header_name, "X-Discourse-Early-Hint" get "/latest" - expect(response.status).to eq(200) - - expect(response.headers["Link"]).to include( - "; rel=preconnect, ; rel=dns-prefetch", + expect(response.headers["X-Discourse-Early-Hint"]).to include( + "; rel=preconnect", ) + expect(response.headers["Link"]).to eq(nil) + end + + it "is skipped for non-app URLs" do + get "/latest.json" + expect(response.status).to eq(200) + expect(response.headers["Link"]).to eq(nil) end end - context "when `GlobalSetting.preload_link_header` is enabled" do - before { global_setting :preload_link_header, true } + context "when in preload mode" do + before { global_setting :early_hint_header_mode, "preload" } - it "should have the Link header with assets on full page requests" do - get("/latest") - expect(response.headers).to include("Link") - end - - it "shouldn't have the Link header on xhr api requests" do - get("/latest.json") - expect(response.headers).not_to include("Link") - end - end - - context "when `GlobalSetting.preload_link_header` is disabled" do - before { global_setting :preload_link_header, false } - - it "shouldn't have the Link header with assets on full page requests" do - get("/latest") - expect(response.headers).not_to include("Link") - end - - it "shouldn't have the Link header on xhr api requests" do - get("/latest.json") - expect(response.headers).not_to include("Link") + it "includes the preload hint" do + get "/latest" + expect(response.status).to eq(200) + expect(response.headers["Link"]).to include('.js>; rel="preload"') + expect(response.headers["Link"]).to include('.css?__ws=test.localhost>; rel="preload"') end end end