Merge pull request #2885 from ganmacs/ensure-not-nil

out_forward ensures that password and username are not nil
fluent · Mar 16, 2020 · 84c93e9 · 84c93e9
2 parents 026c0d7 + 52e5169
commit 84c93e9
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 4 deletions.
diff --git a/lib/fluent/plugin/out_forward.rb b/lib/fluent/plugin/out_forward.rb
@@ -508,7 +508,7 @@ def ack_reader
 
     class Node
       extend Forwardable
-      def_delegators :@server, :discovery_id, :host, :port, :name, :weight, :standby, :username, :password, :shared_key
+      def_delegators :@server, :discovery_id, :host, :port, :name, :weight, :standby
 
       # @param connection_manager [Fluent::Plugin::ForwardOutput::ConnectionManager]
       # @param ack_handler [Fluent::Plugin::ForwardOutput::AckHandler]
@@ -540,8 +540,8 @@ def initialize(sender, server, failure:, connection_manager:, ack_handler:)
           log: @log,
           hostname: sender.security && sender.security.self_hostname,
           shared_key: server.shared_key || (sender.security && sender.security.shared_key) || '',
-          password: server.password,
-          username: server.username,
+          password: server.password || '',
+          username: server.username || '',
         )
 
         @unpacker = Fluent::MessagePackFactory.msgpack_unpacker

diff --git a/lib/fluent/plugin/out_forward/handshake_protocol.rb b/lib/fluent/plugin/out_forward/handshake_protocol.rb
@@ -105,6 +105,10 @@ def generate_ping(ri)
           .hexdigest
         ping = ['PING', @hostname, @shared_key_salt, shared_key_hexdigest]
         if !ri.auth.empty?
+          if @username.nil? || @password.nil?
+            raise PingpongError, "username and password are required"
+          end
+
           password_hexdigest = Digest::SHA512.new.update(ri.auth).update(@username).update(@password).hexdigest
           ping.push(@username, password_hexdigest)
         else

diff --git a/test/plugin/out_forward/test_handshake_protocol.rb b/test/plugin/out_forward/test_handshake_protocol.rb
@@ -81,6 +81,15 @@ class HandshakeProtocolTest < Test::Unit::TestCase
       assert_equal(ri.state, :established)
     end
 
+    test 'raises an error when password and username are nil if auth exists' do
+      handshake = Fluent::Plugin::ForwardOutput::HandshakeProtocol.new(log: $log, hostname: 'hostname', shared_key: 'shared_key', password: nil, username: nil)
+      ri = Fluent::Plugin::ForwardOutput::ConnectionManager::RequestInfo.new(:helo)
+
+      assert_raise(Fluent::Plugin::ForwardOutput::PingpongError.new('username and password are required')) do
+        handshake.invoke('', ri, ['HELO', { 'auth' => 'auth' }])
+      end
+    end
+
     data(
       lack_of_elem: ['PONG', true, '', 'client_hostname'],
       wrong_message: ['WRONG_PONG', true, '', 'client_hostname', '40a3c5943cc6256e0c5dcf176e97db3826b0909698c330dc8e53d15af63efb47e030d113130255dd6e7ced5176d2999cc2e02a44852d45152503af317b73b33f'],
@@ -89,7 +98,7 @@ class HandshakeProtocolTest < Test::Unit::TestCase
       wrong_key: ['PONG', true, '', 'hostname', 'wrong_key'],
     )
     test 'raises an error when message is' do |msg|
-      handshake = Fluent::Plugin::ForwardOutput::HandshakeProtocol.new(log: $log, hostname: 'hostname', shared_key: 'shared_key', password: nil, username: nil)
+      handshake = Fluent::Plugin::ForwardOutput::HandshakeProtocol.new(log: $log, hostname: 'hostname', shared_key: 'shared_key', password: '', username: '')
       handshake.instance_variable_set(:@shared_key_salt, 'ce1897b0d3dbd76b90d7fb96010dcac3') # to fix salt
 
       ri = Fluent::Plugin::ForwardOutput::ConnectionManager::RequestInfo.new(:pingpong, '', '')

diff --git a/test/plugin/test_out_forward.rb b/test/plugin/test_out_forward.rb
@@ -284,6 +284,29 @@ def try_write(chunk)
     assert_equal 1235, d.instance.discovery_manager.services[1].port
   end
 
+  test 'pass username and password as empty string to HandshakeProtocol' do
+    config_path = File.join(TMP_DIR, "sd_file.conf")
+    File.open(config_path, 'w') do |file|
+      file.write(%[
+- 'host': 127.0.0.1
+  'port': 1234
+  'weight': 1
+])
+    end
+
+    mock(Fluent::Plugin::ForwardOutput::HandshakeProtocol).new(log: anything, hostname: nil, shared_key: anything, password: '', username: '')
+    @d = d = create_driver(%[
+<service_discovery>
+  @type file
+  path #{config_path}
+</service_discovery>
+    ])
+
+    assert_equal 1, d.instance.discovery_manager.services.size
+    assert_equal '127.0.0.1', d.instance.discovery_manager.services[0].host
+    assert_equal 1234, d.instance.discovery_manager.services[0].port
+  end
+
   test 'compress_default_value' do
     @d = d = create_driver
     assert_equal :text, d.instance.compress