diff --git a/lib/datadog/core/remote/client.rb b/lib/datadog/core/remote/client.rb index c49608a044a..6683ecf33f9 100644 --- a/lib/datadog/core/remote/client.rb +++ b/lib/datadog/core/remote/client.rb @@ -98,7 +98,9 @@ def apply_config(paths, targets, contents) content = contents.find_content(path, target) # abort entirely if matching content not found - raise SyncError, "no valid content for target at path '#{path}'" if content.nil? + if content.nil? + raise SyncError, "no valid content for target at path '#{path}'" + end # to be added or updated << config # TODO: metadata (hash, version, etc...) diff --git a/lib/datadog/core/remote/configuration/content.rb b/lib/datadog/core/remote/configuration/content.rb index 8266c898ccd..92bcc46efb6 100644 --- a/lib/datadog/core/remote/configuration/content.rb +++ b/lib/datadog/core/remote/configuration/content.rb @@ -22,6 +22,18 @@ def parse(hash) attr_accessor :version def initialize(path:, data:) + if data.nil? + # +data+ is passed to Digest calculation and also is + # unconditionally taken length of by +length+ method. + # As such, the class is not written to expect +data+ to be nil. + # Detect bad incoming values here to provide earlier diagnostics + # when developing tests, for example. + raise ArgumentError, 'data must not be nil' + end + unless String === data + raise ArgumentError, "Invalid type for data: #{data.class}: expected String" + end + @path = path @data = data @apply_state = ApplyState::UNACKNOWLEDGED @@ -72,8 +84,9 @@ def compute_and_store_hash(type) private_class_method :new end - # ContentList stores a list of Conetnt instances - # It provides convinient methods for finding content base on Configuration::Path and Configuration::Target + # ContentList stores a list of Content instances. + # It provides convenient methods for finding content based on + # Configuration::Path and Configuration::Target. class ContentList < Array class << self def parse(array) diff --git a/lib/datadog/core/remote/configuration/digest.rb b/lib/datadog/core/remote/configuration/digest.rb index c1e941a6bd6..9ca2c5e7e85 100644 --- a/lib/datadog/core/remote/configuration/digest.rb +++ b/lib/datadog/core/remote/configuration/digest.rb @@ -26,6 +26,19 @@ class InvalidHashTypeError < StandardError; end class << self def hexdigest(type, data) + unless String === data + # This class (Digest) passes +data+ to the Ruby standard + # library Digest routines without validating its type. + # The stdlib Digest requires a String, and the previous + # implementation of this class that used StringIO + # unconditionally read from +data+ without validating the + # type. Meaning, passing +nil+ as +data+ has never worked. + # It still doesn't work in the present implementation. + # Flag the nil data now to get earlier diagnostics when + # developing tests for example. + raise ArgumentError, "Invalid type for data: #{data.class}: expected String" + end + d = case type when :sha256 ::Digest::SHA256.new diff --git a/lib/datadog/core/remote/configuration/repository.rb b/lib/datadog/core/remote/configuration/repository.rb index 5e53b0cad73..6a8f947cf25 100644 --- a/lib/datadog/core/remote/configuration/repository.rb +++ b/lib/datadog/core/remote/configuration/repository.rb @@ -185,7 +185,7 @@ def apply(repository) end end - # Update existimng repository's contents + # Update existing repository's contents class Update attr_reader :path, :target, :content diff --git a/lib/datadog/core/remote/configuration/target.rb b/lib/datadog/core/remote/configuration/target.rb index 6b1ba6e5130..a18f3a7ca33 100644 --- a/lib/datadog/core/remote/configuration/target.rb +++ b/lib/datadog/core/remote/configuration/target.rb @@ -11,8 +11,15 @@ class Configuration class TargetMap < Hash class << self def parse(hash) - opaque_backend_state = hash['signed']['custom']['opaque_backend_state'] - version = hash['signed']['version'] + signed = hash.fetch('signed') + # Note that the +dig+ call permits +hash['signed']+ to be + # missing the +custom+ subtree entirely. + # Previously the subtree was required but +opaque_backend_state+ + # could still be missing (and obtained here as nil). + opaque_backend_state = signed.dig('custom', 'opaque_backend_state') + # The version appears to be optional to the rest of this class, + # and we have tests that do not provide it. + version = signed['version'] map = new @@ -21,7 +28,7 @@ def parse(hash) @version = version end - hash['signed']['targets'].each_with_object(map) do |(p, t), m| + signed.fetch('targets').each_with_object(map) do |(p, t), m| path = Configuration::Path.parse(p) target = Configuration::Target.parse(t) @@ -46,9 +53,9 @@ def initialize class Target class << self def parse(hash) - length = Integer(hash['length']) - digests = Configuration::DigestList.parse(hash['hashes']) - version = Integer(hash['custom']['v']) + length = Integer(hash.fetch('length')) + digests = Configuration::DigestList.parse(hash.fetch('hashes')) + version = Integer(hash.dig('custom', 'v')) new(digests: digests, length: length, version: version) end diff --git a/spec/datadog/tracing/remote_spec.rb b/spec/datadog/tracing/remote_spec.rb index 82af94805bb..6ab9658fe9d 100644 --- a/spec/datadog/tracing/remote_spec.rb +++ b/spec/datadog/tracing/remote_spec.rb @@ -27,7 +27,7 @@ describe '#process_config' do subject(:process_config) { remote.process_config(config, content) } let(:config) { nil } - let(:content) { Datadog::Core::Remote::Configuration::Content.parse({path: path, content: nil}) } + let(:content) { Datadog::Core::Remote::Configuration::Content.parse({path: path, content: ''}) } context 'with an empty content' do let(:config) { {} }