DEBUG-3499 RC: add diagnostics for invalid values

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...)

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)

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

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
 

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

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) { {} }