fix: Add additional payload filter key validation (#295)

Previously, we were allowing any non-empty string value to be provided as a
payload filter key. However, customers can only create a filter key with a
subset of characters within the LaunchDarkly UI.

In an effort to warn customers earlier about potentially invalid
configurations, we are adding basic key validation as part of the start up
sequence.

contract-tests/service.rb

@@ -31,6 +31,7 @@
       'all-flags-client-side-only',
       'all-flags-details-only-for-tracked-flags',
       'filtering',
+      'filtering-strict',
       'secure-mode-hash',
       'tags',
       'migrations',

launchdarkly-server-sdk.gemspec

@@ -22,6 +22,7 @@ Gem::Specification.new do |spec|
   spec.required_ruby_version = ">= 3.0.0"
 
   spec.add_development_dependency "aws-sdk-dynamodb", "~> 1.57"
+  spec.add_development_dependency "rexml", "~> 3.3", ">= 3.3.7"
   spec.add_development_dependency "bundler", "2.2.33"
   spec.add_development_dependency "simplecov", "~> 0.21"
   spec.add_development_dependency "rspec", "~> 3.10"

lib/ldclient-rb/config.rb

@@ -77,7 +77,7 @@ def initialize(opts = {})
       @socket_factory = opts[:socket_factory]
       @big_segments = opts[:big_segments] || BigSegmentsConfig.new(store: nil)
       @application = LaunchDarkly::Impl::Util.validate_application_info(opts[:application] || {}, @logger)
-      @payload_filter_key = opts[:payload_filter_key]
+      @payload_filter_key = LaunchDarkly::Impl::Util.validate_payload_filter_key(opts[:payload_filter_key] , @logger)
       @hooks = (opts[:hooks] || []).keep_if { |hook| hook.is_a? Interfaces::Hooks::Hook }
       @omit_anonymous_contexts = opts.has_key?(:omit_anonymous_contexts) && opts[:omit_anonymous_contexts]
       @data_source_update_sink = nil

lib/ldclient-rb/impl/util.rb

@@ -75,6 +75,21 @@ def self.validate_application_info(app, logger)
           version: validate_application_value(app[:version], :version, logger),
         }
       end
+
+      #
+      # @param value [String, nil]
+      # @param logger [Logger]
+      # @return [String, nil]
+      #
+      def self.validate_payload_filter_key(value, logger)
+        return nil if value.nil?
+        return value if value.is_a?(String) && /^[a-zA-Z0-9][._\-a-zA-Z0-9]*$/.match?(value)
+
+        logger.warn {
+          "Invalid payload filter configured, full environment will be fetched. Ensure the filter key is not empty and was copied correctly from LaunchDarkly settings."
+        }
+        nil
+      end
     end
   end
 end

lib/ldclient-rb/util.rb

@@ -77,11 +77,6 @@ module Util
     def self.add_payload_filter_key(uri, config)
       return uri if config.payload_filter_key.nil?
 
-      unless config.payload_filter_key.is_a?(String) && !config.payload_filter_key.empty?
-        config.logger.warn { "[LDClient] Filter key must be a non-empty string. No filtering will be applied." }
-        return uri
-      end
-
       begin
         parsed = URI.parse(uri)
         new_query_params = URI.decode_www_form(String(parsed.query)) << ["filter", config.payload_filter_key]

spec/impl/util_spec.rb

@@ -0,0 +1,52 @@
+require "spec_helper"
+
+module LaunchDarkly
+  module Impl
+    describe "payload filter key validation" do
+      let(:logger) { double }
+
+      it "silently discards nil" do
+        expect(logger).not_to receive(:warn)
+        expect(Util.validate_payload_filter_key(nil, logger)).to be_nil
+      end
+
+      [true, 1, 1.0, [], {}].each do |value|
+        it "returns nil for invalid type #{value.class}" do
+          expect(logger).to receive(:warn)
+          expect(Util.validate_payload_filter_key(value, logger)).to be_nil
+        end
+      end
+
+      [
+        "",
+        "-cannot-start-with-dash",
+        "_cannot-start-with-underscore",
+        "-cannot-start-with-period",
+        "no spaces for you",
+        "org@special/characters",
+      ].each do |value|
+        it "returns nil for invalid value #{value}" do
+          expect(logger).to receive(:warn)
+          expect(Util.validate_payload_filter_key(value, logger)).to be_nil
+        end
+      end
+
+      [
+        "camelCase",
+        "snake_case",
+        "kebab-case",
+        "with.dots",
+        "with_underscores",
+        "with-hyphens",
+        "with1234numbers",
+        "with.many_1234-mixtures",
+        "1start-with-number",
+      ].each do |value|
+        it "passes for value #{value}" do
+          expect(logger).not_to receive(:warn)
+          expect(Util.validate_payload_filter_key(value, logger)).to eq(value)
+        end
+      end
+    end
+  end
+end