From d681decf015adc9d66adb5bf5944e30e63169595 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 24 Jul 2024 04:45:30 +0530 Subject: [PATCH] FEATURE: use new site setting for onebox custom user agent. (#28045) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, we couldn't change the user agent name dynamically for onebox requests. In this commit, a new hidden site setting `onebox_user_agent` is created to override the default user agent value specified in the [initializer](https://github.com/discourse/discourse/blob/c333e9d6e6a6a0407133eb2d5087605897048f95/config/initializers/100-onebox_options.rb#L15). Co-authored-by: Régis Hanol --- config/initializers/100-onebox_options.rb | 4 +- config/site_settings.yml | 3 + lib/final_destination.rb | 2 +- lib/onebox/helpers.rb | 10 +++- spec/lib/onebox/helpers_spec.rb | 4 +- spec/lib/oneboxer_spec.rb | 69 +++++++++++++++++------ 6 files changed, 66 insertions(+), 26 deletions(-) diff --git a/config/initializers/100-onebox_options.rb b/config/initializers/100-onebox_options.rb index 70f0a468e8f..c9ecef1e783 100644 --- a/config/initializers/100-onebox_options.rb +++ b/config/initializers/100-onebox_options.rb @@ -5,14 +5,14 @@ Rails.application.config.to_prepare do Onebox.options = { twitter_client: TwitterApi, redirect_limit: 3, - user_agent: "Discourse Forum Onebox v#{Discourse::VERSION::STRING}", + user_agent: "Discourse Forum Onebox", allowed_ports: [80, 443, SiteSetting.port.to_i], } else Onebox.options = { twitter_client: TwitterApi, redirect_limit: 3, - user_agent: "Discourse Forum Onebox v#{Discourse::VERSION::STRING}", + user_agent: "Discourse Forum Onebox", } end end diff --git a/config/site_settings.yml b/config/site_settings.yml index 98feece9ecc..78112ce97d2 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2106,6 +2106,9 @@ onebox: list_type: compact enable_inline_onebox_on_all_domains: default: true + onebox_user_agent: + default: "" + hidden: true force_custom_user_agent_hosts: default: "http://codepen.io" type: list diff --git a/lib/final_destination.rb b/lib/final_destination.rb index ee39aa266df..22ccec4305d 100644 --- a/lib/final_destination.rb +++ b/lib/final_destination.rb @@ -81,7 +81,7 @@ class FinalDestination @user_agent = ( if @force_custom_user_agent_hosts.any? { |host| hostname_matches?(host) } - Onebox.options.user_agent + Onebox::Helpers.user_agent else @default_user_agent end diff --git a/lib/onebox/helpers.rb b/lib/onebox/helpers.rb index c4bb7809525..8817781f0a5 100644 --- a/lib/onebox/helpers.rb +++ b/lib/onebox/helpers.rb @@ -104,9 +104,7 @@ module Onebox headers ||= {} - if Onebox.options.user_agent && !headers["User-Agent"] - headers["User-Agent"] = Onebox.options.user_agent - end + headers["User-Agent"] ||= user_agent if user_agent request = Net::HTTP::Get.new(uri.request_uri, headers) start_time = Time.now @@ -232,6 +230,12 @@ module Onebox end end + def self.user_agent + user_agent = SiteSetting.onebox_user_agent.presence || Onebox.options.user_agent + user_agent = "#{user_agent} v#{Discourse::VERSION::STRING}" + user_agent + end + # Percent-encodes a URI string per RFC3986 - https://tools.ietf.org/html/rfc3986 def self.uri_encode(url) return "" unless url diff --git a/spec/lib/onebox/helpers_spec.rb b/spec/lib/onebox/helpers_spec.rb index 139d422aec4..24ca7ece862 100644 --- a/spec/lib/onebox/helpers_spec.rb +++ b/spec/lib/onebox/helpers_spec.rb @@ -211,7 +211,7 @@ RSpec.describe Onebox::Helpers do context "with custom option" do around do |example| previous_options = Onebox.options.to_h - Onebox.options = { user_agent: "EvilTroutBot v0.1" } + Onebox.options = { user_agent: "EvilTroutBot" } example.run @@ -221,7 +221,7 @@ RSpec.describe Onebox::Helpers do it "has the custom user agent" do stub_request(:get, "http://example.com/some-resource").with( headers: { - "user-agent" => "EvilTroutBot v0.1", + "user-agent" => "EvilTroutBot v#{Discourse::VERSION::STRING}", }, ).to_return(status: 200, body: "test") diff --git a/spec/lib/oneboxer_spec.rb b/spec/lib/oneboxer_spec.rb index 8912b91adf2..cbf4ba55e4e 100644 --- a/spec/lib/oneboxer_spec.rb +++ b/spec/lib/oneboxer_spec.rb @@ -570,26 +570,59 @@ RSpec.describe Oneboxer do end end - it "uses the Onebox custom user agent on specified hosts" do - SiteSetting.force_custom_user_agent_hosts = "http://codepen.io|https://video.discourse.org/" - url = "https://video.discourse.org/presentation.mp4" + describe "onebox custom user agent" do + let!(:default_onebox_user_agent) do + "#{Onebox.options.user_agent} v#{Discourse::VERSION::STRING}" + end - stub_request(:head, url).to_return(status: 403, body: "", headers: {}) - stub_request(:get, url).to_return(status: 403, body: "", headers: {}) - stub_request(:head, url).with(headers: { "User-Agent" => Onebox.options.user_agent }).to_return( - status: 200, - body: "", - headers: { - }, - ) - stub_request(:get, url).with(headers: { "User-Agent" => Onebox.options.user_agent }).to_return( - status: 200, - body: "", - headers: { - }, - ) + it "uses the site setting value" do + SiteSetting.force_custom_user_agent_hosts = "http://codepen.io|https://video.discourse.org/" + url = "https://video.discourse.org/presentation.mp4" + custom_user_agent = "Custom User Agent" - expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to be_present + %i[head get].each do |method| + stub_request(method, url).with( + headers: { + "User-Agent" => default_onebox_user_agent, + }, + ).to_return(status: 403, body: "", headers: {}) + stub_request(method, url).with( + headers: { + "User-Agent" => "#{custom_user_agent} v#{Discourse::VERSION::STRING}", + }, + ).to_return(status: 200, body: "", headers: {}) + end + + expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to include("onebox-warning-message") + + SiteSetting.onebox_user_agent = custom_user_agent + + expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to include( + "onebox-placeholder-container", + ) + end + + it "forcing on specified hosts" do + SiteSetting.force_custom_user_agent_hosts = "http://codepen.io|https://video.discourse.org/" + url = "https://video.discourse.org/presentation.mp4" + + stub_request(:head, url).to_return(status: 403, body: "", headers: {}) + stub_request(:get, url).to_return(status: 403, body: "", headers: {}) + stub_request(:head, url).with( + headers: { + "User-Agent" => default_onebox_user_agent, + }, + ).to_return(status: 200, body: "", headers: {}) + stub_request(:get, url).with( + headers: { + "User-Agent" => default_onebox_user_agent, + }, + ).to_return(status: 200, body: "", headers: {}) + + expect(Oneboxer.preview(url, invalidate_oneboxes: true)).to include( + "onebox-placeholder-container", + ) + end end context "with youtube stub" do