From d5a46533b662c38a779d3ba402f81a16be2dae85 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 19 Dec 2024 08:33:43 -0500 Subject: [PATCH 1/6] APMLP-350 fix crash in crashtracker when agent url is an ipv6 address --- ext/libdatadog_api/crashtracker.c | 3 +++ .../core/crashtracking/component_spec.rb | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/ext/libdatadog_api/crashtracker.c b/ext/libdatadog_api/crashtracker.c index cffee2768ec..7633d1af40b 100644 --- a/ext/libdatadog_api/crashtracker.c +++ b/ext/libdatadog_api/crashtracker.c @@ -54,6 +54,9 @@ static VALUE _native_start_or_update_on_fork(int argc, VALUE *argv, DDTRACE_UNUS // Tags and endpoint are heap-allocated, so after here we can't raise exceptions otherwise we'll leak this memory // Start of exception-free zone to prevent leaks {{ ddog_Endpoint *endpoint = ddog_endpoint_from_url(char_slice_from_ruby_string(agent_base_url)); + if (endpoint == NULL) { + rb_raise(rb_eRuntimeError, "Failed to create endpoint from agent_base_url: %"PRIsVALUE, agent_base_url); + } ddog_Vec_Tag tags = convert_tags(tags_as_array); ddog_crasht_Config config = { diff --git a/spec/datadog/core/crashtracking/component_spec.rb b/spec/datadog/core/crashtracking/component_spec.rb index f05d803c1d8..034bd5f441e 100644 --- a/spec/datadog/core/crashtracking/component_spec.rb +++ b/spec/datadog/core/crashtracking/component_spec.rb @@ -95,6 +95,27 @@ expect(described_class.build(settings, agent_settings, logger: logger)).to be_nil end end + + context 'when agent_base_url is invalid (e.g. hostname is an IPv6 address)' do + let(:agent_base_url) { 'http://1234:1234::1/' } + + it 'returns an instance of Component that failed to start' do + expect(Datadog::Core::Crashtracking::TagBuilder).to receive(:call).with(settings) + .and_return(tags) + expect(Datadog::Core::Crashtracking::AgentBaseUrl).to receive(:resolve).with(agent_settings) + .and_return(agent_base_url) + expect(::Libdatadog).to receive(:ld_library_path) + .and_return(ld_library_path) + expect(::Libdatadog).to receive(:path_to_crashtracking_receiver_binary) + .and_return(path_to_crashtracking_receiver_binary) + + # Diagnostics is only provided via the error report to logger, + # there is no indication in the object state that it failed to start. + expect(logger).to receive(:error).with(/Failed to start crash tracking/) + + expect(described_class.build(settings, agent_settings, logger: logger)).to be_a(described_class) + end + end end context 'instance methods' do From da04ea2053b0ecf5d2dfb7b15f00984fd2de5d49 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 19 Dec 2024 08:48:06 -0500 Subject: [PATCH 2/6] APMLP-350 correctly handle IPv6 addresses in crashtracker agent settings resolver --- .../core/crashtracking/agent_base_url.rb | 11 ++++++- .../core/crashtracking/agent_base_url_spec.rb | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/lib/datadog/core/crashtracking/agent_base_url.rb b/lib/datadog/core/crashtracking/agent_base_url.rb index 0a1a95ac605..fcfa7f66b25 100644 --- a/lib/datadog/core/crashtracking/agent_base_url.rb +++ b/lib/datadog/core/crashtracking/agent_base_url.rb @@ -7,10 +7,19 @@ module Core module Crashtracking # This module provides a method to resolve the base URL of the agent module AgentBaseUrl + # IPv6 regular expression from + # https://stackoverflow.com/questions/53497/regular-expression-that-matches-valid-ipv6-addresses + # Does not match IPv4 addresses. + IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/ + def self.resolve(agent_settings) case agent_settings.adapter when Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER - "#{agent_settings.ssl ? 'https' : 'http'}://#{agent_settings.hostname}:#{agent_settings.port}/" + hostname = agent_settings.hostname + if hostname =~ IPV6_REGEXP + hostname = "[#{hostname}]" + end + "#{agent_settings.ssl ? 'https' : 'http'}://#{hostname}:#{agent_settings.port}/" when Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER "unix://#{agent_settings.uds_path}" end diff --git a/spec/datadog/core/crashtracking/agent_base_url_spec.rb b/spec/datadog/core/crashtracking/agent_base_url_spec.rb index 86163c035fd..407b74daf26 100644 --- a/spec/datadog/core/crashtracking/agent_base_url_spec.rb +++ b/spec/datadog/core/crashtracking/agent_base_url_spec.rb @@ -37,6 +37,38 @@ expect(described_class.resolve(agent_settings)).to eq('http://example.com:8080/') end end + + context 'when hostname is an IPv4 address' do + let(:agent_settings) do + instance_double( + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, + adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, + ssl: false, + hostname: '1.2.3.4', + port: 8080 + ) + end + + it 'returns the correct base URL' do + expect(described_class.resolve(agent_settings)).to eq('http://1.2.3.4:8080/') + end + end + + context 'when hostname is an IPv6 address' do + let(:agent_settings) do + instance_double( + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings, + adapter: Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER, + ssl: false, + hostname: '1234:1234::1', + port: 8080 + ) + end + + it 'returns the correct base URL' do + expect(described_class.resolve(agent_settings)).to eq('http://[1234:1234::1]:8080/') + end + end end context 'when using UnixSocket adapter' do From ca22d34ecde2cc78c69b0a62d453be457a7a7cb0 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 19 Dec 2024 09:03:59 -0500 Subject: [PATCH 3/6] rubocop --- lib/datadog/core/crashtracking/agent_base_url.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/datadog/core/crashtracking/agent_base_url.rb b/lib/datadog/core/crashtracking/agent_base_url.rb index fcfa7f66b25..4de622d67ae 100644 --- a/lib/datadog/core/crashtracking/agent_base_url.rb +++ b/lib/datadog/core/crashtracking/agent_base_url.rb @@ -10,15 +10,13 @@ module AgentBaseUrl # IPv6 regular expression from # https://stackoverflow.com/questions/53497/regular-expression-that-matches-valid-ipv6-addresses # Does not match IPv4 addresses. - IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/ + IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/.freeze def self.resolve(agent_settings) case agent_settings.adapter when Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER hostname = agent_settings.hostname - if hostname =~ IPV6_REGEXP - hostname = "[#{hostname}]" - end + hostname = "[#{hostname}]" if hostname =~ IPV6_REGEXP "#{agent_settings.ssl ? 'https' : 'http'}://#{hostname}:#{agent_settings.port}/" when Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER "unix://#{agent_settings.uds_path}" From 135c638e88e16f939f951ecd195862f586869393 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 19 Dec 2024 09:15:16 -0500 Subject: [PATCH 4/6] rubocop --- lib/datadog/core/crashtracking/agent_base_url.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/datadog/core/crashtracking/agent_base_url.rb b/lib/datadog/core/crashtracking/agent_base_url.rb index 4de622d67ae..a1d1f7f5c32 100644 --- a/lib/datadog/core/crashtracking/agent_base_url.rb +++ b/lib/datadog/core/crashtracking/agent_base_url.rb @@ -10,13 +10,15 @@ module AgentBaseUrl # IPv6 regular expression from # https://stackoverflow.com/questions/53497/regular-expression-that-matches-valid-ipv6-addresses # Does not match IPv4 addresses. - IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/.freeze + IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/ # rubocop:disable Layout/LineLength def self.resolve(agent_settings) case agent_settings.adapter when Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER hostname = agent_settings.hostname - hostname = "[#{hostname}]" if hostname =~ IPV6_REGEXP + if hostname =~ IPV6_REGEXP + hostname = "[#{hostname}]" + end "#{agent_settings.ssl ? 'https' : 'http'}://#{hostname}:#{agent_settings.port}/" when Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER "unix://#{agent_settings.uds_path}" From 390fe1258847cd155aac708c685fee2931c44871 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 19 Dec 2024 09:36:46 -0500 Subject: [PATCH 5/6] rubocop --- lib/datadog/core/crashtracking/agent_base_url.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/datadog/core/crashtracking/agent_base_url.rb b/lib/datadog/core/crashtracking/agent_base_url.rb index a1d1f7f5c32..74b59a1cda5 100644 --- a/lib/datadog/core/crashtracking/agent_base_url.rb +++ b/lib/datadog/core/crashtracking/agent_base_url.rb @@ -10,15 +10,13 @@ module AgentBaseUrl # IPv6 regular expression from # https://stackoverflow.com/questions/53497/regular-expression-that-matches-valid-ipv6-addresses # Does not match IPv4 addresses. - IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/ # rubocop:disable Layout/LineLength + IPV6_REGEXP = /\A(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\z)/.freeze # rubocop:disable Layout/LineLength def self.resolve(agent_settings) case agent_settings.adapter when Datadog::Core::Configuration::Ext::Agent::HTTP::ADAPTER hostname = agent_settings.hostname - if hostname =~ IPV6_REGEXP - hostname = "[#{hostname}]" - end + hostname = "[#{hostname}]" if hostname =~ IPV6_REGEXP "#{agent_settings.ssl ? 'https' : 'http'}://#{hostname}:#{agent_settings.port}/" when Datadog::Core::Configuration::Ext::Agent::UnixSocket::ADAPTER "unix://#{agent_settings.uds_path}" From 20aac5573cc414df5ed19ed1d6dcfb2773f450c7 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 19 Dec 2024 09:37:17 -0500 Subject: [PATCH 6/6] types --- sig/datadog/core/crashtracking/agent_base_url.rbs | 1 + 1 file changed, 1 insertion(+) diff --git a/sig/datadog/core/crashtracking/agent_base_url.rbs b/sig/datadog/core/crashtracking/agent_base_url.rbs index 2bdf7f07259..3a2c77f8e85 100644 --- a/sig/datadog/core/crashtracking/agent_base_url.rbs +++ b/sig/datadog/core/crashtracking/agent_base_url.rbs @@ -2,6 +2,7 @@ module Datadog module Core module Crashtracking module AgentBaseUrl + IPV6_REGEXP: Regexp def self.resolve: (Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings) -> ::String? end end