Move header_read_timeout_buffered_io feature flag

This replaces the use_read_total_timeout option with the
header_read_timeout_buffered_io feature flag for header timeout
Reasons for this are:

* use_read_total_timeout was intended avoid timeouts for streaming requests
* Header reading is never expected to take longer than the timeout
* With the feature flag being invoked earlier, we can roll out more carefully

lib/gitlab/buffered_io.rb

@@ -23,11 +23,10 @@ module Gitlab
     override :readuntil
     def readuntil(terminator, ignore_eof = false)
       start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
-      check_timeout = Feature.enabled?(:header_read_timeout_buffered_io)
 
       begin
         until idx = @rbuf.index(terminator)
-          if check_timeout && (elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time) > HEADER_READ_TIMEOUT
+          if (elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time) > HEADER_READ_TIMEOUT
             raise Gitlab::HTTP::HeaderReadTimeout, "Request timed out after reading headers for #{elapsed} seconds"
           end
 

lib/gitlab/http_connection_adapter.rb

@@ -29,7 +29,7 @@ module Gitlab
       http = super
       http.hostname_override = hostname if hostname
 
-      if options[:use_read_total_timeout]
+      if Feature.enabled?(:header_read_timeout_buffered_io)
         gitlab_http = Gitlab::NetHttpAdapter.new(http.address, http.port)
 
         http.instance_variables.each do |variable|

spec/lib/gitlab/buffered_io_spec.rb

@@ -49,16 +49,6 @@ RSpec.describe Gitlab::BufferedIo do
     it 'raises a timeout error' do
       expect { readuntil }.to raise_error(Gitlab::HTTP::HeaderReadTimeout, /Request timed out after reading headers for 0\.[0-9]+ seconds/)
     end
-
-    context 'when the header_read_timeout feature is disabled' do
-      before do
-        stub_feature_flags(header_read_timeout_buffered_io: false)
-      end
-
-      it 'does not raise a timeout error' do
-        expect { readuntil }.to raise_error(RuntimeError, 'Test did not raise HeaderReadTimeout')
-      end
-    end
   end
 end
 # rubocop:enable Style/FrozenStringLiteralComment

spec/lib/gitlab/http_connection_adapter_spec.rb

@@ -27,11 +27,21 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do
       end
     end
 
+    context 'with header_read_timeout_buffered_io feature disabled' do
+      before do
+        stub_feature_flags(header_read_timeout_buffered_io: false)
+      end
+
+      it 'uses the regular Net::HTTP class' do
+        expect(connection).to be_a(Net::HTTP)
+      end
+    end
+
     context 'when local requests are allowed' do
       let(:options) { { allow_local_requests: true } }
 
       it 'sets up the connection' do
-        expect(connection).to be_a(Net::HTTP)
+        expect(connection).to be_a(Gitlab::NetHttpAdapter)
         expect(connection.address).to eq('93.184.216.34')
         expect(connection.hostname_override).to eq('example.org')
         expect(connection.addr_port).to eq('example.org')
@@ -43,7 +53,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do
       let(:options) { { allow_local_requests: false } }
 
       it 'sets up the connection' do
-        expect(connection).to be_a(Net::HTTP)
+        expect(connection).to be_a(Gitlab::NetHttpAdapter)
         expect(connection.address).to eq('93.184.216.34')
         expect(connection.hostname_override).to eq('example.org')
         expect(connection.addr_port).to eq('example.org')
@@ -64,7 +74,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do
           let(:options) { { allow_local_requests: true } }
 
           it 'sets up the connection' do
-            expect(connection).to be_a(Net::HTTP)
+            expect(connection).to be_a(Gitlab::NetHttpAdapter)
             expect(connection.address).to eq('172.16.0.0')
             expect(connection.hostname_override).to be(nil)
             expect(connection.addr_port).to eq('172.16.0.0')
@@ -87,7 +97,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do
           let(:options) { { allow_local_requests: true } }
 
           it 'sets up the connection' do
-            expect(connection).to be_a(Net::HTTP)
+            expect(connection).to be_a(Gitlab::NetHttpAdapter)
             expect(connection.address).to eq('127.0.0.1')
             expect(connection.hostname_override).to be(nil)
             expect(connection.addr_port).to eq('127.0.0.1')
@@ -100,7 +110,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do
         let(:uri) { URI('https://example.org:8080') }
 
         it 'sets up the addr_port accordingly' do
-          expect(connection).to be_a(Net::HTTP)
+          expect(connection).to be_a(Gitlab::NetHttpAdapter)
           expect(connection.address).to eq('93.184.216.34')
           expect(connection.hostname_override).to eq('example.org')
           expect(connection.addr_port).to eq('example.org:8080')
@@ -115,7 +125,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do
       end
 
       it 'sets up the connection' do
-        expect(connection).to be_a(Net::HTTP)
+        expect(connection).to be_a(Gitlab::NetHttpAdapter)
         expect(connection.address).to eq('example.org')
         expect(connection.hostname_override).to eq(nil)
         expect(connection.addr_port).to eq('example.org')
@@ -129,7 +139,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do
       end
 
       it 'sets up the connection' do
-        expect(connection).to be_a(Net::HTTP)
+        expect(connection).to be_a(Gitlab::NetHttpAdapter)
         expect(connection.address).to eq('example.org')
         expect(connection.hostname_override).to eq(nil)
         expect(connection.addr_port).to eq('example.org')