From a0bae61ad1a8e382f0a173c40ab6336e5683bf57 Mon Sep 17 00:00:00 2001 From: Samir Jha Date: Tue, 18 Nov 2025 16:00:27 +0000 Subject: [PATCH] Fixes #38862 - Handle nil host UUIDs properly --- .../container_gateway_api.rb | 5 +- .../container_gateway_main.rb | 6 ++ test/container_gateway_api_test.rb | 67 +++++++++++++++++++ test/container_gateway_backend_test.rb | 44 ++++++++++++ 4 files changed, 121 insertions(+), 1 deletion(-) diff --git a/lib/smart_proxy_container_gateway/container_gateway_api.rb b/lib/smart_proxy_container_gateway/container_gateway_api.rb index 7ae0b98..1911719 100644 --- a/lib/smart_proxy_container_gateway/container_gateway_api.rb +++ b/lib/smart_proxy_container_gateway/container_gateway_api.rb @@ -262,7 +262,8 @@ class Api < ::Sinatra::Base database.connection.transaction(isolation: :serializable, retry_on: [Sequel::SerializationFailure]) do hosts_table = database.connection[:hosts] hosts_table.delete - hosts_table.import(%i[uuid], hosts.map { |host| [host['uuid']] }) + valid_hosts = hosts.filter_map { |host| [host['uuid']] if host['uuid'].present? } + hosts_table.import(%i[uuid], valid_hosts) unless valid_hosts.empty? end {} end @@ -277,6 +278,8 @@ class Api < ::Sinatra::Base do_authorize_any params['hosts'].flat_map do |host_map| host_map.filter_map do |host_uuid, repos| + next if host_uuid.nil? || host_uuid.to_s.strip.empty? + if repos.nil? || repos.empty? repo_names = [] else diff --git a/lib/smart_proxy_container_gateway/container_gateway_main.rb b/lib/smart_proxy_container_gateway/container_gateway_main.rb index 6806a1e..0b8b922 100644 --- a/lib/smart_proxy_container_gateway/container_gateway_main.rb +++ b/lib/smart_proxy_container_gateway/container_gateway_main.rb @@ -214,6 +214,8 @@ def build_host_repository_mapping(host_repo_maps) end def build_host_entries(hosts, repositories, host_uuid, repos) + return if host_uuid.nil? || host_uuid.to_s.strip.empty? + host = hosts[{ uuid: host_uuid }] return unless host return if repos.nil? || repos.empty? @@ -232,7 +234,11 @@ def extract_auth_required_repo_names(repos) end def update_host_repositories(uuid, repositories) + return if uuid.nil? || uuid.to_s.strip.empty? + host = find_or_create_host(uuid) + return unless host + hosts_repositories = database.connection[:hosts_repositories] database.connection.transaction(isolation: :serializable, retry_on: [Sequel::SerializationFailure], diff --git a/test/container_gateway_api_test.rb b/test/container_gateway_api_test.rb index 57a30d4..a067aaa 100644 --- a/test/container_gateway_api_test.rb +++ b/test/container_gateway_api_test.rb @@ -427,4 +427,71 @@ def test_flatpak_static_index_server_error assert_equal 500, last_response.status assert_equal '{"error": "internal server error"}', last_response.body end + + def test_update_hosts_filters_nil_uuids + hosts = [ + { 'uuid' => 'host-uuid-1' }, + { 'uuid' => nil }, + { 'uuid' => 'host-uuid-2' } + ] + + put '/update_hosts', { 'hosts' => hosts } + assert last_response.ok? + + # Verify only valid hosts were inserted (old behavior would have failed or inserted nil) + assert @database.connection[:hosts].count >= 2 + assert_not_nil @database.connection[:hosts].first(uuid: 'host-uuid-1') + assert_not_nil @database.connection[:hosts].first(uuid: 'host-uuid-2') + end + + def test_update_hosts_filters_empty_string_uuids + hosts = [ + { 'uuid' => 'host-uuid-1' }, + { 'uuid' => '' }, + { 'uuid' => 'host-uuid-2' } + ] + + put '/update_hosts', { 'hosts' => hosts } + assert last_response.ok? + + # Verify only valid hosts were inserted + assert @database.connection[:hosts].count >= 2 + assert_not_nil @database.connection[:hosts].first(uuid: 'host-uuid-1') + assert_not_nil @database.connection[:hosts].first(uuid: 'host-uuid-2') + end + + def test_update_hosts_handles_missing_uuid_key + hosts = [ + { 'uuid' => 'host-uuid-1' }, + { 'name' => 'host-without-uuid' }, + { 'uuid' => 'host-uuid-2' } + ] + + put '/update_hosts', { 'hosts' => hosts } + assert last_response.ok? + + # Verify only valid hosts were inserted + assert @database.connection[:hosts].count >= 2 + assert_not_nil @database.connection[:hosts].first(uuid: 'host-uuid-1') + assert_not_nil @database.connection[:hosts].first(uuid: 'host-uuid-2') + end + + def test_update_hosts_with_empty_array + put '/update_hosts', { 'hosts' => [] } + assert last_response.ok? + end + + def test_update_hosts_with_all_invalid_uuids + hosts = [ + { 'uuid' => nil }, + { 'uuid' => '' }, + { 'name' => 'no-uuid' } + ] + + put '/update_hosts', { 'hosts' => hosts } + assert last_response.ok? + + # Verify that no hosts were added to the DB + assert_equal 0, @database.connection[:hosts].count + end end diff --git a/test/container_gateway_backend_test.rb b/test/container_gateway_backend_test.rb index 619b4d3..1f72390 100644 --- a/test/container_gateway_backend_test.rb +++ b/test/container_gateway_backend_test.rb @@ -327,5 +327,49 @@ def test_build_host_repository_mapping_handles_mixed_scenarios assert_equal [[repo[:id], host[:id]]], result end + + def test_update_host_repositories_with_nil_uuid + @container_gateway_main.update_repository_list([{ 'repository' => 'test_repo1', 'auth_required' => true }]) + + # Should not raise an error and should not create any hosts + @container_gateway_main.update_host_repositories(nil, ['test_repo1']) + + assert_equal 0, @database.connection[:hosts].count + assert_equal 0, @database.connection[:hosts_repositories].count + end + + def test_update_host_repositories_with_empty_string_uuid + @container_gateway_main.update_repository_list([{ 'repository' => 'test_repo1', 'auth_required' => true }]) + + # Should not raise an error and should not create any hosts + @container_gateway_main.update_host_repositories('', ['test_repo1']) + + assert_equal 0, @database.connection[:hosts].count + assert_equal 0, @database.connection[:hosts_repositories].count + end + + def test_build_host_entries_with_nil_uuid + @container_gateway_main.update_repository_list([{ 'repository' => 'test_repo1', 'auth_required' => true }]) + + hosts = @database.connection[:hosts] + repositories = @database.connection[:repositories] + repos = [{ 'repository' => 'test_repo1', 'auth_required' => true }] + + result = @container_gateway_main.build_host_entries(hosts, repositories, nil, repos) + + assert_nil result + end + + def test_build_host_entries_with_empty_string_uuid + @container_gateway_main.update_repository_list([{ 'repository' => 'test_repo1', 'auth_required' => true }]) + + hosts = @database.connection[:hosts] + repositories = @database.connection[:repositories] + repos = [{ 'repository' => 'test_repo1', 'auth_required' => true }] + + result = @container_gateway_main.build_host_entries(hosts, repositories, '', repos) + + assert_nil result + end end # rubocop:enable Metrics/AbcSize