FIX: Set sane default for Net::HTTP when processing a request (#28141)

This commit patches `Net::HTTP` to reduce the default timeouts of 60
seconds when we are processing a request. There are certain routes in
Discourse which makes external requests and if the proper timeouts are
not set, we risk having the Unicorn master process force restarting the
Unicorn workers once the `30` seconds timeout is reached. This can
potentially become a vector for DoS attacks and this commit is aimed at
reducing the risk here.

app/controllers/test_requests_controller.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+# This controller's actions are only available in the test environment to help with more complex testing requirements
+class TestRequestsController < ApplicationController
+  if Rails.env.test?
+    def test_net_http_timeouts
+      net_http = Net::HTTP.new("example.com")
+
+      render json: {
+               open_timeout: net_http.open_timeout,
+               read_timeout: net_http.read_timeout,
+               write_timeout: net_http.write_timeout,
+               max_retries: net_http.max_retries,
+             }
+    end
+  end
+end

config/initializers/200-first_middlewares.rb

@@ -7,6 +7,8 @@
 # We aren't manipulating the middleware stack directly because of
 # https://github.com/rails/rails/pull/27936
 
+require "middleware/processing_request"
+Rails.configuration.middleware.unshift(Middleware::ProcessingRequest)
 Rails.configuration.middleware.unshift(MessageBus::Rack::Middleware)
 
 # no reason to track this in development, that is 300+ redis calls saved per

config/routes.rb

@@ -1649,5 +1649,10 @@ Discourse::Application.routes.draw do
 
     get "/form-templates/:id" => "form_templates#show"
     get "/form-templates" => "form_templates#index"
+
+    if Rails.env.test?
+      # Routes that are only used for testing
+      get "/test_net_http_timeouts" => "test_requests#test_net_http_timeouts"
+    end
   end
 end

lib/freedom_patches/net_http.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module NetHTTPPatch
+  OPEN_TIMEOUT = 5
+  READ_TIMEOUT = 10
+  WRITE_TIMEOUT = 5
+
+  # By default Net::HTTP will retry 1 time on idempotent requests but we can't afford that while processing a request
+  # so setting it to 0
+  MAX_RETIRES = 0
+
+  def initialize(*args, &block)
+    super(*args, &block)
+
+    ## START PATCH
+    if Thread.current[Middleware::ProcessingRequest::PROCESSING_REQUEST_THREAD_KEY]
+      self.open_timeout = OPEN_TIMEOUT
+      self.read_timeout = READ_TIMEOUT
+      self.write_timeout = WRITE_TIMEOUT
+      self.max_retries = 0
+    end
+    ## END PATCH
+  end
+end
+
+Net::HTTP.prepend(NetHTTPPatch)

lib/middleware/processing_request.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class Middleware::ProcessingRequest
+  PROCESSING_REQUEST_THREAD_KEY = "discourse.processing_request"
+
+  def initialize(app)
+    @app = app
+  end
+
+  def call(env)
+    Thread.current[PROCESSING_REQUEST_THREAD_KEY] = true
+    @app.call(env)
+  ensure
+    Thread.current[PROCESSING_REQUEST_THREAD_KEY] = nil
+  end
+end

spec/integrity/middleware_order_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe "Middleware order" do
       ActionDispatch::RemoteIp,
       Middleware::RequestTracker,
       MessageBus::Rack::Middleware,
+      Middleware::ProcessingRequest,
       Rack::Sendfile,
       ActionDispatch::Static,
       ActionDispatch::Executor,

spec/requests/net_http_timeout_spec.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+RSpec.describe "Net::HTTP timeouts when processing a request" do
+  it "should set the right timeouts for any `Net::HTTP` instances intialized while processing a request" do
+    stub_const(NetHTTPPatch, :OPEN_TIMEOUT, 0.001) do
+      stub_const(NetHTTPPatch, :READ_TIMEOUT, 0.002) do
+        stub_const(NetHTTPPatch, :WRITE_TIMEOUT, 0.003) do
+          get "/test_net_http_timeouts.json"
+
+          parsed = response.parsed_body
+
+          expect(parsed["open_timeout"]).to eq(0.001)
+          expect(parsed["read_timeout"]).to eq(0.002)
+          expect(parsed["write_timeout"]).to eq(0.003)
+          expect(parsed["max_retries"]).to eq(0)
+        end
+      end
+    end
+  end
+end