fix: Match attribute references against String and Symbol hash keys

.github/actions/check/action.yml

@@ -23,7 +23,7 @@ runs:
 
     - name: Run RuboCop
       shell: bash
-      run: bundle exec rubocop --parallel
+      run: bundle exec rubocop
 
     - name: Build contract tests
       if: ${{ inputs.flaky != 'true' }}

lib/ldclient-rb/context.rb

@@ -230,9 +230,9 @@ def get_value_for_reference(reference)
         name = reference.component(i)
 
         return nil unless value.is_a?(Hash)
-        return nil unless value.has_key?(name)
 
-        value = value[name]
+        found, value = LaunchDarkly::Impl::Context.fetch_attribute(value, name)
+        return nil unless found
       end
 
       value
@@ -418,7 +418,10 @@ def to_h
       when :anonymous
         @anonymous
       else
-        @attributes&.fetch(name, nil)
+        return nil if @attributes.nil?
+
+        _, value = LaunchDarkly::Impl::Context.fetch_attribute(@attributes, name)
+        value
       end
     end
 

lib/ldclient-rb/impl/context.rb

@@ -78,6 +78,42 @@ def self.validate_anonymous(anonymous, allow_nil)
         ERR_ANONYMOUS_NON_BOOLEAN
       end
 
+      #
+      # Attribute reference components are always symbols, but application data
+      # can use string keys. For example, JSON.parse makes string keys by
+      # default. The two forms name the same JSON property, so this method
+      # compares them as equal.
+      #
+      # @param component [Symbol]
+      # @param key [any]
+      # @return [Boolean]
+      #
+      def self.same_attribute_name?(component, key)
+        component == key || component.to_s == key.to_s
+      end
+
+      #
+      # Read an attribute out of a hash by name. The hash can use symbol or
+      # string keys, and the name can be either form. An exact match wins, then
+      # the other form. A symbol therefore takes precedence when the hash holds
+      # both forms of one name.
+      #
+      # The first element of the returned array is true if the hash has the
+      # attribute. The second element is the value, or nil if there is none.
+      #
+      # @param hash [Hash]
+      # @param name [Symbol, String]
+      # @return [Array(Boolean, any)]
+      #
+      def self.fetch_attribute(hash, name)
+        return true, hash[name] if hash.has_key?(name)
+
+        alternate = name.is_a?(Symbol) ? name.to_s : name.to_sym
+        return true, hash[alternate] if hash.has_key?(alternate)
+
+        [false, nil]
+      end
+
       #
       # @param kind [String]
       # @param key [String]

lib/ldclient-rb/impl/context_filter.rb

@@ -84,7 +84,10 @@ def filter_redact_anonymous(context)
           end
         end
 
-        filtered[:_meta] = {redactedAttributes: redacted} unless redacted.empty?
+        # A hash can hold both the string and the symbol form of one name. Each
+        # form redacts a value, but the reference names one attribute, so report
+        # it once.
+        filtered[:_meta] = {redactedAttributes: redacted.uniq} unless redacted.empty?
 
         filtered
       end
@@ -137,11 +140,11 @@ def filter_redact_anonymous(context)
             next unless private_attribute.depth == (current_path.count + 1)
 
             component = private_attribute.component(current_path.count)
-            next unless component == k
+            next unless LaunchDarkly::Impl::Context.same_attribute_name?(component, k)
 
             match = true
             (0...current_path.count).each do |i|
-              unless private_attribute.component(i) == current_path[i]
+              unless LaunchDarkly::Impl::Context.same_attribute_name?(private_attribute.component(i), current_path[i])
                 match = false
                 break
               end

spec/context_spec.rb

@@ -391,6 +391,38 @@ module LaunchDarkly
             expect(org_context.get_value_for_reference(ref)).to matcher
           end
         end
+
+        it "with complex attributes that have string keys" do
+          address = JSON.parse('{"city":"Oakland","state":"CA","zip":94612}')
+          tags = ["LaunchDarkly", "Feature Flags"]
+          nested = JSON.parse('{"upper":{"middle":{"name":"Middle Level","inner":{"levels":[0,1,2]}},"name":"Upper Level"}}')
+
+          org_context = subject.create({ key: 'ld', kind: 'org', name: 'LaunchDarkly', anonymous: true, address: address, tags: tags, nested: nested })
+
+          [
+            ['/address', eq(address)],
+            ['/address/city', eq('Oakland')],
+
+            ['/tags', eq(tags)],
+
+            ['/nested/upper/name', eq('Upper Level')],
+            ['/nested/upper/middle/name', eq('Middle Level')],
+            ['/nested/upper/middle/inner/levels', eq([0, 1, 2])],
+          ].each do |(reference, matcher)|
+            ref = Reference.create(reference)
+            expect(org_context.get_value_for_reference(ref)).to matcher
+          end
+        end
+
+        it "with a string top level attribute name" do
+          address = { city: "Oakland" }
+
+          org_context = subject.create({ :key => 'ld', :kind => 'org', "address" => address })
+
+          expect(org_context.get_value_for_reference(Reference.create('/address'))).to eq(address)
+          expect(org_context.get_value_for_reference(Reference.create('/address/city'))).to eq('Oakland')
+          expect(org_context.get_value("address")).to eq(address)
+        end
       end
     end
 

spec/impl/context_filter_spec.rb

@@ -52,6 +52,89 @@ module Impl
         expect(filtered["user"][:_meta][:redactedAttributes]).to eq([:email])
         expect(filtered["org"]).to eq({ key: "org-key", email: "email" })
       end
+
+      describe "with string keyed attribute data" do
+        it "redacts a nested private attribute" do
+          filter = ContextFilter.new(false, ["/profile/email"])
+          context = LDContext.create({ kind: "user", key: "user-key", profile: JSON.parse('{"email":"private@example.test","plan":"pro"}') })
+
+          filtered = filter.filter(context)
+
+          expect(filtered[:profile]).to eq({ "plan" => "pro" })
+          expect(filtered[:_meta][:redactedAttributes]).to eq([:"/profile/email"])
+        end
+
+        it "redacts a nested per-context private attribute" do
+          filter = ContextFilter.new(false, [])
+          context = LDContext.create({
+            kind: "user",
+            key: "user-key",
+            account: JSON.parse('{"apiToken":"private-token","region":"test-region"}'),
+            _meta: { privateAttributes: ["/account/apiToken"] },
+          })
+
+          filtered = filter.filter(context)
+
+          expect(filtered[:account]).to eq({ "region" => "test-region" })
+          expect(filtered[:_meta][:redactedAttributes]).to eq([:"/account/apiToken"])
+        end
+
+        it "produces the same event payload as symbol keyed data" do
+          filter = ContextFilter.new(false, ["/profile/email"])
+          json = '{"email":"private@example.test","plan":"pro"}'
+          string_keyed = LDContext.create({ kind: "user", key: "user-key", profile: JSON.parse(json) })
+          symbol_keyed = LDContext.create({ kind: "user", key: "user-key", profile: JSON.parse(json, symbolize_names: true) })
+
+          # The two hashes keep the key type they were given, so compare the
+          # payloads the way the event sender sees them. A string key and a
+          # symbol key serialize to the same JSON property name.
+          expect(JSON.generate(filter.filter(string_keyed))).to eq(JSON.generate(filter.filter(symbol_keyed)))
+        end
+
+        it "redacts a private attribute nested three levels deep" do
+          filter = ContextFilter.new(false, ["/a/b/c"])
+          context = LDContext.create({ kind: "user", key: "user-key", a: JSON.parse('{"b":{"c":"private","d":"keep"}}') })
+
+          filtered = filter.filter(context)
+
+          expect(filtered[:a]).to eq({ "b" => { "d" => "keep" } })
+          expect(filtered[:_meta][:redactedAttributes]).to eq([:"/a/b/c"])
+        end
+
+        it "redacts both the string and the symbol form of one name, and reports the reference once" do
+          filter = ContextFilter.new(false, ["/profile/email"])
+          context = LDContext.create({
+            kind: "user",
+            key: "user-key",
+            profile: { :email => "symbol-private", "email" => "string-private", "other" => "keep" },
+          })
+
+          filtered = filter.filter(context)
+
+          expect(filtered[:profile]).to eq({ "other" => "keep" })
+          expect(filtered[:_meta][:redactedAttributes]).to eq([:"/profile/email"])
+        end
+
+        it "redacts a nested private attribute under a string top level key" do
+          filter = ContextFilter.new(false, ["/profile/email"])
+          context = LDContext.create({ :kind => "user", :key => "user-key", "profile" => JSON.parse('{"email":"private@example.test","plan":"pro"}') })
+
+          filtered = filter.filter(context)
+
+          expect(filtered["profile"]).to eq({ "plan" => "pro" })
+          expect(filtered[:_meta][:redactedAttributes]).to eq([:"/profile/email"])
+        end
+
+        it "redacts an escaped nested private attribute name" do
+          filter = ContextFilter.new(false, ["/profile/a~1b"])
+          context = LDContext.create({ kind: "user", key: "user-key", profile: JSON.parse('{"a/b":"private","keep":"keep"}') })
+
+          filtered = filter.filter(context)
+
+          expect(filtered[:profile]).to eq({ "keep" => "keep" })
+          expect(filtered[:_meta][:redactedAttributes]).to eq([:"/profile/a~1b"])
+        end
+      end
     end
   end
 end

spec/impl/context_spec.rb

@@ -30,6 +30,44 @@ module Impl
           expect(subject.validate_key(input)).to eq(expected)
         end
       end
+
+      it "compares attribute names in either direction" do
+        [
+          [:email, "email"],
+          ["email", :email],
+          [:email, :email],
+          ["email", "email"],
+        ].each do |(component, key)|
+          expect(subject.same_attribute_name?(component, key)).to be true
+        end
+
+        expect(subject.same_attribute_name?(:email, "other")).to be false
+      end
+
+      it "fetches an attribute for either key type by either name type" do
+        [
+          [{ email: "value" }, :email],
+          [{ email: "value" }, "email"],
+          [{ "email" => "value" }, :email],
+          [{ "email" => "value" }, "email"],
+        ].each do |(hash, name)|
+          expect(subject.fetch_attribute(hash, name)).to eq([true, "value"])
+        end
+
+        expect(subject.fetch_attribute({ email: "value" }, :other)).to eq([false, nil])
+      end
+
+      it "prefers an exact match when a hash holds both forms of a name" do
+        hash = { :email => "symbol", "email" => "string" }
+
+        expect(subject.fetch_attribute(hash, :email)).to eq([true, "symbol"])
+        expect(subject.fetch_attribute(hash, "email")).to eq([true, "string"])
+      end
+
+      it "reports a missing attribute as absent rather than nil valued" do
+        expect(subject.fetch_attribute({ email: nil }, :email)).to eq([true, nil])
+        expect(subject.fetch_attribute({}, :email)).to eq([false, nil])
+      end
     end
   end
 end