From 8615fc6cbbd1085b37b5ec251e4acd39b16cb839 Mon Sep 17 00:00:00 2001 From: Gary Pendergast Date: Mon, 3 Mar 2025 16:32:25 +1100 Subject: [PATCH] DEV: Add a user agent to all HTTP requests that Discourse makes. (#31555) This change standardises the `User-Agent` header that Discourse will send when talking to other sites. `Discourse.user_agent` is now the authority on what the user agent value should be. For Onebox requests, this changes the user agent from their existing value to match the new value (unless overridden). For all other requests, `Net::HTTPHeader` is monkey-patched to add a default `User-Agent` header when one hasn't been provided. --- app/controllers/test_requests_controller.rb | 6 ++++++ app/jobs/regular/emit_web_hook_event.rb | 2 +- config/initializers/100-onebox_options.rb | 7 +------ config/routes.rb | 1 + lib/discourse.rb | 8 ++++++++ lib/freedom_patches/net_http_header.rb | 15 +++++++++++++++ lib/onebox/engine/twitter_status_onebox.rb | 4 ---- lib/onebox/helpers.rb | 11 ++++++++--- spec/lib/discourse_spec.rb | 10 ++++++++++ spec/lib/onebox/helpers_spec.rb | 2 +- spec/lib/oneboxer_spec.rb | 4 +--- spec/requests/admin/web_hooks_controller_spec.rb | 6 +++++- spec/requests/net_http_header_spec.rb | 14 ++++++++++++++ 13 files changed, 71 insertions(+), 19 deletions(-) create mode 100644 lib/freedom_patches/net_http_header.rb create mode 100644 spec/requests/net_http_header_spec.rb diff --git a/app/controllers/test_requests_controller.rb b/app/controllers/test_requests_controller.rb index b8d8d755c90..6729ad4feb9 100644 --- a/app/controllers/test_requests_controller.rb +++ b/app/controllers/test_requests_controller.rb @@ -13,5 +13,11 @@ class TestRequestsController < ApplicationController max_retries: net_http.max_retries, } end + + def test_net_http_headers + net_http_get = Net::HTTP::Get.new("example.com") + + render json: net_http_get + end end end diff --git a/app/jobs/regular/emit_web_hook_event.rb b/app/jobs/regular/emit_web_hook_event.rb index bb26403fb09..a9ef0da4249 100644 --- a/app/jobs/regular/emit_web_hook_event.rb +++ b/app/jobs/regular/emit_web_hook_event.rb @@ -133,7 +133,7 @@ module Jobs "Content-Length" => web_hook_body.bytesize.to_s, "Content-Type" => content_type, "Host" => uri.host, - "User-Agent" => "Discourse/#{Discourse::VERSION::STRING}", + "User-Agent" => Discourse.user_agent, "X-Discourse-Instance" => Discourse.base_url, "X-Discourse-Event-Id" => web_hook_event.id.to_s, "X-Discourse-Event-Type" => @arguments[:event_type], diff --git a/config/initializers/100-onebox_options.rb b/config/initializers/100-onebox_options.rb index c9ecef1e783..78613144a27 100644 --- a/config/initializers/100-onebox_options.rb +++ b/config/initializers/100-onebox_options.rb @@ -5,14 +5,9 @@ Rails.application.config.to_prepare do Onebox.options = { twitter_client: TwitterApi, redirect_limit: 3, - 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", - } + Onebox.options = { twitter_client: TwitterApi, redirect_limit: 3 } end end diff --git a/config/routes.rb b/config/routes.rb index 3deb56c6abe..373ff7200cc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1729,6 +1729,7 @@ Discourse::Application.routes.draw do if Rails.env.test? # Routes that are only used for testing get "/test_net_http_timeouts" => "test_requests#test_net_http_timeouts" + get "/test_net_http_headers" => "test_requests#test_net_http_headers" end end end diff --git a/lib/discourse.rb b/lib/discourse.rb index e70685ccd20..870523d4f2d 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -850,6 +850,14 @@ module Discourse GitUtils.try_git(git_cmd, default_value) end + def self.user_agent + if git_version.present? + @user_agent ||= "Discourse/#{VERSION::STRING}-#{git_version}; +https://www.discourse.org/" + else + @user_agent ||= "Discourse/#{VERSION::STRING}; +https://www.discourse.org/" + end + end + # Either returns the site_contact_username user or the first admin. def self.site_contact_user user = diff --git a/lib/freedom_patches/net_http_header.rb b/lib/freedom_patches/net_http_header.rb new file mode 100644 index 00000000000..b7f743864d4 --- /dev/null +++ b/lib/freedom_patches/net_http_header.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module NetHTTPHeaderPatch + def initialize_http_header(initheader) + # If no user-agent is set, set it to the default + initheader ||= {} + user_agent_key = + initheader.keys.find { |key| key.to_s.downcase == "user-agent" } || "User-Agent".to_sym + initheader[user_agent_key] ||= Discourse.user_agent + + super initheader + end +end + +Net::HTTPHeader.prepend(NetHTTPHeaderPatch) diff --git a/lib/onebox/engine/twitter_status_onebox.rb b/lib/onebox/engine/twitter_status_onebox.rb index 9b950b8069a..77de26e6db6 100644 --- a/lib/onebox/engine/twitter_status_onebox.rb +++ b/lib/onebox/engine/twitter_status_onebox.rb @@ -15,10 +15,6 @@ module Onebox path.match?(%r{^/.+?/status(es)?/\d+(/(video|photo)/\d?)?(/?\?.*)?/?$}) end - def http_params - { "User-Agent" => "DiscourseBot/1.0" } - end - def to_html raw.present? ? super : "" end diff --git a/lib/onebox/helpers.rb b/lib/onebox/helpers.rb index c7b5d272989..e1520ed564f 100644 --- a/lib/onebox/helpers.rb +++ b/lib/onebox/helpers.rb @@ -232,9 +232,14 @@ module Onebox 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 + if SiteSetting.onebox_user_agent.present? + return "#{SiteSetting.onebox_user_agent} v#{Discourse::VERSION::STRING}" + end + + if Onebox.options.user_agent.present? + return "#{Onebox.options.user_agent} v#{Discourse::VERSION::STRING}" + end + Discourse.user_agent end # Percent-encodes a URI string per RFC3986 - https://tools.ietf.org/html/rfc3986 diff --git a/spec/lib/discourse_spec.rb b/spec/lib/discourse_spec.rb index 2e99924e1d5..dfefba6bf21 100644 --- a/spec/lib/discourse_spec.rb +++ b/spec/lib/discourse_spec.rb @@ -194,6 +194,16 @@ RSpec.describe Discourse do end end + describe "#user_agent" do + it "returns a user agent string" do + stub_const(Discourse::VERSION, :STRING, "1.2.3") do + Discourse.stubs(:git_version).returns("123456") + + expect(Discourse.user_agent).to eq("Discourse/1.2.3-123456; +https://www.discourse.org/") + end + end + end + describe "#site_contact_user" do fab!(:admin) fab!(:another_admin) { Fabricate(:admin) } diff --git a/spec/lib/onebox/helpers_spec.rb b/spec/lib/onebox/helpers_spec.rb index 5969bd925d9..cdb5c0b3190 100644 --- a/spec/lib/onebox/helpers_spec.rb +++ b/spec/lib/onebox/helpers_spec.rb @@ -219,7 +219,7 @@ RSpec.describe Onebox::Helpers do it "has the default Discourse user agent" do stub_request(:get, "http://example.com/some-resource").with( headers: { - "user-agent" => /Discourse Forum Onebox/, + "user-agent" => Discourse.user_agent, }, ).to_return(status: 200, body: "test") diff --git a/spec/lib/oneboxer_spec.rb b/spec/lib/oneboxer_spec.rb index 4b5b496a385..2f3b87566fb 100644 --- a/spec/lib/oneboxer_spec.rb +++ b/spec/lib/oneboxer_spec.rb @@ -626,9 +626,7 @@ RSpec.describe Oneboxer do end describe "onebox custom user agent" do - let!(:default_onebox_user_agent) do - "#{Onebox.options.user_agent} v#{Discourse::VERSION::STRING}" - end + let!(:default_onebox_user_agent) { Discourse.user_agent } it "uses the site setting value" do SiteSetting.force_custom_user_agent_hosts = "http://codepen.io|https://video.discourse.org/" diff --git a/spec/requests/admin/web_hooks_controller_spec.rb b/spec/requests/admin/web_hooks_controller_spec.rb index b27ced7cc9a..2302355c03d 100644 --- a/spec/requests/admin/web_hooks_controller_spec.rb +++ b/spec/requests/admin/web_hooks_controller_spec.rb @@ -311,7 +311,11 @@ RSpec.describe Admin::WebHooksController do expect(parsed_event["payload"]).to eq("abc") expect(JSON.parse(parsed_event["response_headers"])).to eq( - { "content-type" => "application/json", "yoo" => "man" }, + { + "content-type" => "application/json", + "user-agent" => Discourse.user_agent, + "yoo" => "man", + }, ) expect(parsed_event["response_body"]).to eq("efg") end diff --git a/spec/requests/net_http_header_spec.rb b/spec/requests/net_http_header_spec.rb new file mode 100644 index 00000000000..ce96a88cdaf --- /dev/null +++ b/spec/requests/net_http_header_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# We can use the redeliver event to test the user-agent header +RSpec.describe "Net::HTTPHeader sets a default user-agent" do + it "should set a user-agent when none has been set" do + get "/test_net_http_headers.json" + + expect(response).to have_http_status(:success) + + parsed_body = JSON.parse(response.body) + expect(parsed_body).to have_key("user-agent") + expect(parsed_body["user-agent"].first).to eq(Discourse.user_agent) + end +end