diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index 646dcf654baa388fe6d8a3db7f2a2f2cf8cb1baa..825f48e8dcdbfeb1df38b5d669204e2562914ecf 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -1079,7 +1079,6 @@ Gitlab/NamespacedClass: - 'lib/gitlab/batch_worker_context.rb' - 'lib/gitlab/blame.rb' - 'lib/gitlab/branch_push_merge_commit_analyzer.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/build_access.rb' - 'lib/gitlab/changes_list.rb' - 'lib/gitlab/chaos.rb' diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 5f9a6d282c886057598f0f7395bcf85e8440b9f9..2095d2c1ca3e154681778627fe1d09dc4f1839d3 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -2443,7 +2443,6 @@ Layout/LineLength: - 'lib/gitlab/background_migration/migrate_requirements_to_work_items.rb' - 'lib/gitlab/background_migration/populate_resolved_on_default_branch_column.rb' - 'lib/gitlab/background_migration/project_namespaces/backfill_project_namespaces.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/bullet/exclusions.rb' - 'lib/gitlab/cache/helpers.rb' - 'lib/gitlab/changelog/config.rb' @@ -3497,7 +3496,6 @@ Layout/LineLength: - 'spec/lib/gitlab/background_migration/job_coordinator_spec.rb' - 'spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb' - 'spec/lib/gitlab/background_migration/update_jira_tracker_data_deployment_type_based_on_url_spec.rb' - - 'spec/lib/gitlab/buffered_io_spec.rb' - 'spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb' - 'spec/lib/gitlab/chat/output_spec.rb' - 'spec/lib/gitlab/checks/branch_check_spec.rb' diff --git a/.rubocop_todo/lint/assignment_in_condition.yml b/.rubocop_todo/lint/assignment_in_condition.yml index 20a51d0c774a7db1e874f8f775196051de268ecd..44dc885eb8372d31efc24b64fe6485c581d8179e 100644 --- a/.rubocop_todo/lint/assignment_in_condition.yml +++ b/.rubocop_todo/lint/assignment_in_condition.yml @@ -152,7 +152,6 @@ Lint/AssignmentInCondition: - 'lib/gitlab/authorized_keys.rb' - 'lib/gitlab/background_migration/fix_projects_without_project_feature.rb' - 'lib/gitlab/blob_helper.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/changelog/config.rb' - 'lib/gitlab/changelog/generator.rb' - 'lib/gitlab/chat/output.rb' diff --git a/.rubocop_todo/lint/missing_cop_enable_directive.yml b/.rubocop_todo/lint/missing_cop_enable_directive.yml index 7716c274f18b6e993a19d2c8dcce7469beccfa69..5a4e8c8bb654ac3fb37fe8bdc994941c34b2c905 100644 --- a/.rubocop_todo/lint/missing_cop_enable_directive.yml +++ b/.rubocop_todo/lint/missing_cop_enable_directive.yml @@ -137,7 +137,6 @@ Lint/MissingCopEnableDirective: - 'lib/gitlab/background_migration/populate_resolved_on_default_branch_column.rb' - 'lib/gitlab/background_migration/purge_stale_security_scans.rb' - 'lib/gitlab/background_migration/update_jira_tracker_data_deployment_type_based_on_url.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/ci/reports/test_suite_summary.rb' - 'lib/gitlab/data_builder/push.rb' - 'lib/gitlab/database/load_balancing/connection_proxy.rb' diff --git a/.rubocop_todo/lint/redundant_cop_disable_directive.yml b/.rubocop_todo/lint/redundant_cop_disable_directive.yml index f10700b8d81b6b52d5086b1f64078a5120e343b6..5555a3b67434b29c3ae5eedebb036a0c4b8afbc2 100644 --- a/.rubocop_todo/lint/redundant_cop_disable_directive.yml +++ b/.rubocop_todo/lint/redundant_cop_disable_directive.yml @@ -165,7 +165,6 @@ Lint/RedundantCopDisableDirective: - 'lib/gitlab/background_migration/re_expire_o_auth_tokens.rb' - 'lib/gitlab/background_migration/remove_occurrence_pipelines_and_duplicate_vulnerabilities_findings.rb' - 'lib/gitlab/background_migration/update_jira_tracker_data_deployment_type_based_on_url.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/cache/request_cache.rb' - 'lib/gitlab/ci/build/artifacts/metadata/entry.rb' - 'lib/gitlab/ci/pipeline/chain/command.rb' diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index d5b9845951e08a9ea5cd1ce1a4c6b043093897a6..9a22fcee54ba11beb1263509fd6604a20bca1cb6 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -2789,7 +2789,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/blame_spec.rb' - 'spec/lib/gitlab/blob_helper_spec.rb' - 'spec/lib/gitlab/branch_push_merge_commit_analyzer_spec.rb' - - 'spec/lib/gitlab/buffered_io_spec.rb' - 'spec/lib/gitlab/build_access_spec.rb' - 'spec/lib/gitlab/bullet_spec.rb' - 'spec/lib/gitlab/cache/helpers_spec.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index c8b4dfedc41077bf037324dfbe919b92bccfa5e6..9983b16d6b7e4fb36fdbf2c041ad39a289642c1f 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -2325,7 +2325,6 @@ Style/InlineDisableAnnotation: - 'lib/gitlab/bitbucket_import/importers/issue_notes_importer.rb' - 'lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb' - 'lib/gitlab/blob_helper.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/cache/ci/project_pipeline_status.rb' - 'lib/gitlab/cache/import/caching.rb' - 'lib/gitlab/cache/request_cache.rb' diff --git a/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb b/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb index 6a1f1f0f5e3679a09f71ac2487b2ed1c44f31afd..30a413610f635762a3b85e377e0c63e009e04df5 100644 --- a/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb +++ b/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb @@ -14,7 +14,7 @@ # # 2. Header timeouts # When the use_read_total_timeout option is used, that means the receiver -# of the HTTP request cannot be trusted. Gitlab::BufferedIo will be used, +# of the HTTP request cannot be trusted. Gitlab::HTTP_V2::BufferedIo will be used, # to read header data. It is a modified version of Net::BufferedIO that # raises a timeout error if reading header data takes too much time. diff --git a/gems/gitlab-http/lib/net_http/response_patch.rb b/gems/gitlab-http/lib/net_http/response_patch.rb index 303d629b32e4f86c1cebba1104dd1801257ee0b0..107a961dc6dd922e9b47df7a6572ae165253c91e 100644 --- a/gems/gitlab-http/lib/net_http/response_patch.rb +++ b/gems/gitlab-http/lib/net_http/response_patch.rb @@ -22,9 +22,6 @@ def self.each_response_header(sock) while true uses_buffered_io = sock.is_a?(Gitlab::HTTP_V2::BufferedIo) - # TODO: Gitlab::BufferedIo is temporarily used for an easy migration. - uses_buffered_io ||= sock.is_a?(Gitlab::BufferedIo) if defined?(Gitlab::BufferedIo) - line = uses_buffered_io ? sock.readuntil("\n", true, start_time) : sock.readuntil("\n", true) line = line.sub(/\s{0,10}\z/, '') break if line.empty? diff --git a/lib/gitlab/buffered_io.rb b/lib/gitlab/buffered_io.rb deleted file mode 100644 index b76c8191fa2958664b1bb33882f6813f2fcac8f5..0000000000000000000000000000000000000000 --- a/lib/gitlab/buffered_io.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - # Net::BufferedIO is overwritten by webmock but in order to test this class, it needs to inherit from the original BufferedIO. - # https://github.com/bblimke/webmock/blob/867f4b290fd133658aa9530cba4ba8b8c52c0d35/lib/webmock/http_lib_adapters/net_http.rb#L266 - parent_class = if const_defined?('WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetBufferedIO') && Rails.env.test? - WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetBufferedIO - else - Net::BufferedIO - end - - class BufferedIo < parent_class - extend ::Gitlab::Utils::Override - - HEADER_READ_TIMEOUT = 20 - - # rubocop: disable Style/RedundantBegin - # rubocop: disable Style/RedundantReturn - # rubocop: disable Cop/LineBreakAfterGuardClauses - # rubocop: disable Layout/EmptyLineAfterGuardClause - - # Original method: - # https://github.com/ruby/ruby/blob/cdb7d699d0641e8f081d590d06d07887ac09961f/lib/net/protocol.rb#L190-L200 - override :readuntil - def readuntil(terminator, ignore_eof = false, start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)) - begin - until idx = @rbuf.index(terminator) - 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 - - rbuf_fill - end - - return rbuf_consume(idx + terminator.size) - rescue EOFError - raise unless ignore_eof - return rbuf_consume(@rbuf.size) - end - end - # rubocop: disable Style/RedundantBegin - # rubocop: enable Style/RedundantReturn - # rubocop: enable Cop/LineBreakAfterGuardClauses - # rubocop: enable Layout/EmptyLineAfterGuardClause - end -end diff --git a/spec/initializers/net_http_response_patch_spec.rb b/spec/initializers/net_http_response_patch_spec.rb index 8074047d6aa36b980a1c719553fb273800ff8b4a..b46f9514a8945fe0458a4f79ea8b93918c8a63bb 100644 --- a/spec/initializers/net_http_response_patch_spec.rb +++ b/spec/initializers/net_http_response_patch_spec.rb @@ -17,7 +17,7 @@ end before do - stub_const('Gitlab::BufferedIo::HEADER_READ_TIMEOUT', 0.1) + stub_const('Gitlab::HTTP_V2::BufferedIo::HEADER_READ_TIMEOUT', 0.1) end subject(:each_response_header) { Net::HTTPResponse.each_response_header(socket) { |k, v| } } @@ -46,9 +46,9 @@ end end - context 'with Gitlab::BufferedIo' do + context 'with Gitlab::HTTP_V2::BufferedIo' do let(:mock_io) { StringIO.new(server_response) } - let(:socket) { Gitlab::BufferedIo.new(mock_io) } + let(:socket) { Gitlab::HTTP_V2::BufferedIo.new(mock_io) } it 'forwards start time to the socket' do allow(socket).to receive(:readuntil).and_call_original diff --git a/spec/lib/gitlab/buffered_io_spec.rb b/spec/lib/gitlab/buffered_io_spec.rb deleted file mode 100644 index 0ec377550c184833520cb271d01c88066475d681..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/buffered_io_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BufferedIo do - describe '#readuntil' do - let(:mock_io) { StringIO.new('a') } - let(:start_time) { Process.clock_gettime(Process::CLOCK_MONOTONIC) } - - before do - stub_const('Gitlab::BufferedIo::HEADER_READ_TIMEOUT', 0.1) - end - - subject(:readuntil) do - described_class.new(mock_io).readuntil('a', false, start_time) - end - - it 'does not raise a timeout error' do - expect { readuntil }.not_to raise_error - end - - context 'when the response contains infinitely long headers' do - before do - read_counter = 0 - - allow(mock_io).to receive(:read_nonblock) do |buffer_size, *_| - read_counter += 1 - raise 'Test did not raise HeaderReadTimeout' if read_counter > 10 - - sleep 0.01 - 'H' * buffer_size - end - end - - 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 not passing start_time' do - subject(:readuntil) do - described_class.new(mock_io).readuntil('a', false) - end - - 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 - end - end - end -end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 170df80f730de9ff8f061ba13ab50271774a9131..d1f126eb6f9426c2f90ad166b0c75738e668d48e 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -5342,7 +5342,6 @@ - './spec/lib/gitlab/blame_spec.rb' - './spec/lib/gitlab/blob_helper_spec.rb' - './spec/lib/gitlab/branch_push_merge_commit_analyzer_spec.rb' -- './spec/lib/gitlab/buffered_io_spec.rb' - './spec/lib/gitlab/build_access_spec.rb' - './spec/lib/gitlab/bullet/exclusions_spec.rb' - './spec/lib/gitlab/bullet_spec.rb'