Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APMLP-350 fix crash in crashtracker when agent url is an ipv6 address #4237

Merged
merged 6 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
p-datadog marked this conversation as resolved.
Show resolved Hide resolved
# 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}"
p-datadog marked this conversation as resolved.
Show resolved Hide resolved
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
Loading