From 5108cb62596aaa64e4a7847f6ba5740d473e792c Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Fri, 9 Oct 2015 16:12:01 -0400 Subject: [PATCH 1/2] MONGOID-4157 Don't allow invalid options to be set on a client --- lib/mongo/client.rb | 57 +++++++++++++++++++++++++++++++++++---- lib/mongo/loggable.rb | 2 +- spec/mongo/client_spec.rb | 44 ++++++++++++++++++++++++++++-- 3 files changed, 95 insertions(+), 8 deletions(-) diff --git a/lib/mongo/client.rb b/lib/mongo/client.rb index f388e1484c..ddd8d6ffa3 100644 --- a/lib/mongo/client.rb +++ b/lib/mongo/client.rb @@ -20,6 +20,7 @@ module Mongo # @since 2.0.0 class Client extend Forwardable + include Loggable # The options that do not affect the behaviour of a cluster and its # subcomponents. @@ -27,6 +28,40 @@ class Client # @since 2.1.0 CRUD_OPTIONS = [ :database, :read, :write ].freeze + # Valid client options. + # + # @since 2.1.2 + VALID_OPTIONS = [ + :auth_mech, + :auth_source, + :connect, + :database, + :auth_mech_properties, + :heartbeat_frequency, + :local_threshold, + :server_selection_timeout, + :password, + :max_pool_size, + :min_pool_size, + :wait_queue_timeout, + :connect_timeout, + :read, + :roles, + :replica_set, + :ssl, + :ssl_cert, + :ssl_key, + :ssl_key_pass_phrase, + :ssl_verify, + :ssl_ca_cert, + :socket_timeout, + :user, + :write, + :monitoring, + :logger, + :truncate_logs + ] + # @return [ Mongo::Cluster ] cluster The cluster of servers for the client. attr_reader :cluster @@ -160,9 +195,9 @@ def hash def initialize(addresses_or_uri, options = Options::Redacted.new) @monitoring = Monitoring.new(options) if addresses_or_uri.is_a?(::String) - create_from_uri(addresses_or_uri, options) + create_from_uri(addresses_or_uri, validate_options(options)) else - create_from_addresses(addresses_or_uri, options) + create_from_addresses(addresses_or_uri, validate_options(options)) end yield(self) if block_given? end @@ -221,7 +256,7 @@ def use(name) # @since 2.0.0 def with(new_options = Options::Redacted.new) clone.tap do |client| - opts = Options::Redacted.new(new_options) || Options::Redacted.new + opts = validate_options(new_options) client.options.update(opts) Database.create(client) # We can't use the same cluster if some options that would affect it @@ -296,14 +331,14 @@ def list_databases private def create_from_addresses(addresses, opts = Options::Redacted.new) - @options = Options::Redacted.new(Database::DEFAULT_OPTIONS.merge(opts)).freeze + @options = Database::DEFAULT_OPTIONS.merge(opts).freeze @cluster = Cluster.new(addresses, @monitoring, options) @database = Database.new(self, options[:database], options) end def create_from_uri(connection_string, opts = Options::Redacted.new) uri = URI.new(connection_string, opts) - @options = Options::Redacted.new(Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts))).freeze + @options = Database::DEFAULT_OPTIONS.merge(uri.client_options.merge(opts)).freeze @cluster = Cluster.new(uri.servers, @monitoring, options) @database = Database.new(self, options[:database], options) end @@ -324,5 +359,17 @@ def cluster_modifying?(new_options) options[name] != value end end + + def validate_options(opts = Options::Redacted.new) + return Options::Redacted.new unless opts + Options::Redacted.new(opts.select do |o| + if VALID_OPTIONS.include?(o) + true + else + log_warn("Unsupported client option '#{o}'. It will be ignored.") + false + end + end) + end end end diff --git a/lib/mongo/loggable.rb b/lib/mongo/loggable.rb index 7569434011..0fc698799d 100644 --- a/lib/mongo/loggable.rb +++ b/lib/mongo/loggable.rb @@ -93,7 +93,7 @@ def log_warn(message) # # @since 2.1.0 def logger - (options[:logger] || Logger.logger) + ((options && options[:logger]) || Logger.logger) end private diff --git a/spec/mongo/client_spec.rb b/spec/mongo/client_spec.rb index 05b496f97d..9c37bcbc6d 100644 --- a/spec/mongo/client_spec.rb +++ b/spec/mongo/client_spec.rb @@ -166,7 +166,7 @@ described_class.new( ['127.0.0.1:27017'], :read => { :mode => :primary }, - :local_threshold_ms => 10, + :local_threshold => 10, :server_selection_timeout => 10000, :database => TEST_DB ) @@ -174,7 +174,7 @@ let(:options) do Mongo::Options::Redacted.new(:read => { :mode => :primary }, - :local_threshold_ms => 10, + :local_threshold => 10, :server_selection_timeout => 10000, :database => TEST_DB) end @@ -373,6 +373,26 @@ expect(client.cluster.topology).to be_a(Mongo::Cluster::Topology::ReplicaSet) end end + + context 'when an invalid option is provided' do + + let(:client) do + described_class.new(['127.0.0.1:27017'], :ssl => false, :invalid => :test) + end + + it 'does not set the option' do + expect(client.options.keys).not_to include('invalid') + end + + it 'sets the valid options' do + expect(client.options.keys).to include('ssl') + end + + it 'warns that an invalid option has been specified' do + expect(Mongo::Logger.logger).to receive(:warn) + expect(client.options.keys).not_to include('invalid') + end + end end end @@ -624,6 +644,26 @@ end end end + + context 'when an invalid option is provided' do + + let(:new_client) do + client.with(invalid: :option, ssl: false) + end + + it 'does not set the invalid option' do + expect(new_client.options.keys).not_to include('invalid') + end + + it 'sets the valid options' do + expect(new_client.options.keys).to include('ssl') + end + + it 'warns that an invalid option has been specified' do + expect(Mongo::Logger.logger).to receive(:warn) + expect(new_client.options.keys).not_to include('invalid') + end + end end describe '#write_concern' do From 79dff9f664cf5109e751aa53a24353a5c3435130 Mon Sep 17 00:00:00 2001 From: Emily Stolfo Date: Mon, 12 Oct 2015 12:02:07 -0400 Subject: [PATCH 2/2] MONGOID-4157 Spacing in client constant definition --- lib/mongo/client.rb | 58 ++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/lib/mongo/client.rb b/lib/mongo/client.rb index ddd8d6ffa3..d4c9910354 100644 --- a/lib/mongo/client.rb +++ b/lib/mongo/client.rb @@ -32,35 +32,35 @@ class Client # # @since 2.1.2 VALID_OPTIONS = [ - :auth_mech, - :auth_source, - :connect, - :database, - :auth_mech_properties, - :heartbeat_frequency, - :local_threshold, - :server_selection_timeout, - :password, - :max_pool_size, - :min_pool_size, - :wait_queue_timeout, - :connect_timeout, - :read, - :roles, - :replica_set, - :ssl, - :ssl_cert, - :ssl_key, - :ssl_key_pass_phrase, - :ssl_verify, - :ssl_ca_cert, - :socket_timeout, - :user, - :write, - :monitoring, - :logger, - :truncate_logs - ] + :auth_mech, + :auth_source, + :connect, + :database, + :auth_mech_properties, + :heartbeat_frequency, + :local_threshold, + :server_selection_timeout, + :password, + :max_pool_size, + :min_pool_size, + :wait_queue_timeout, + :connect_timeout, + :read, + :roles, + :replica_set, + :ssl, + :ssl_cert, + :ssl_key, + :ssl_key_pass_phrase, + :ssl_verify, + :ssl_ca_cert, + :socket_timeout, + :user, + :write, + :monitoring, + :logger, + :truncate_logs + ].freeze # @return [ Mongo::Cluster ] cluster The cluster of servers for the client. attr_reader :cluster