Skip to content

Commit

Permalink
APMLP-350 fix crash in crashtracker when agent url is an ipv6 address (
Browse files Browse the repository at this point in the history
  • Loading branch information
p-datadog authored Dec 19, 2024
1 parent ba0aaf3 commit c116d75
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 1 deletion.
3 changes: 3 additions & 0 deletions ext/libdatadog_api/crashtracker.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
9 changes: 8 additions & 1 deletion lib/datadog/core/crashtracking/agent_base_url.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@ 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)/.freeze # rubocop:disable Layout/LineLength

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
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}"
end
Expand Down
1 change: 1 addition & 0 deletions sig/datadog/core/crashtracking/agent_base_url.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions spec/datadog/core/crashtracking/agent_base_url_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions spec/datadog/core/crashtracking/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c116d75

Please sign in to comment.