From bf7d57f28b061e52b4bd7a1bdb9d8c22453c8241 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Thu, 5 Sep 2024 21:24:18 +0200 Subject: [PATCH] Save connection options for conn.reset Commit 016c17c9c9bbc51bf5fab2087ad485367c789118 introduced a regression, that leads to a host list duplication every time conn.reset is used. Since the hosts are duplicated when resolve_hosts is called, the conn.coninfo_hash can not be used. So the fix is to store the original hash of connection options for later use in conn.reset . Fixes #586 --- ext/pg_connection.c | 1 + lib/pg/connection.rb | 7 ++++++- spec/helpers.rb | 4 ++-- spec/pg/connection_async_spec.rb | 21 +++++++++++++++++++++ spec/pg/connection_spec.rb | 8 ++++---- 5 files changed, 34 insertions(+), 7 deletions(-) diff --git a/ext/pg_connection.c b/ext/pg_connection.c index 4e9d0e7d7..a36d09786 100644 --- a/ext/pg_connection.c +++ b/ext/pg_connection.c @@ -264,6 +264,7 @@ pgconn_s_allocate( VALUE klass ) RB_OBJ_WRITE(self, &this->decoder_for_get_copy_data, Qnil); RB_OBJ_WRITE(self, &this->trace_stream, Qnil); rb_ivar_set(self, rb_intern("@calls_to_put_copy_data"), INT2FIX(0)); + rb_ivar_set(self, rb_intern("@iopts_for_reset"), Qnil); return self; } diff --git a/lib/pg/connection.rb b/lib/pg/connection.rb index 0beb85d04..196a19d51 100644 --- a/lib/pg/connection.rb +++ b/lib/pg/connection.rb @@ -573,7 +573,9 @@ def encrypt_password( password, username, algorithm=nil ) # Resets the backend connection. This method closes the # backend connection and tries to re-connect. def reset - iopts = conninfo_hash.compact + # Use connection options from PG::Connection.new to reconnect with the same options but with renewed DNS resolution. + # Use conninfo_hash as a fallback when connect_start was used to create the connection object. + iopts = @iopts_for_reset || conninfo_hash.compact if iopts[:host] && !iopts[:host].empty? && PG.library_version >= 100000 iopts = self.class.send(:resolve_hosts, iopts) end @@ -825,6 +827,7 @@ def new(*args) iopts = PG::Connection.conninfo_parse(option_string).each_with_object({}){|h, o| o[h[:keyword].to_sym] = h[:val] if h[:val] } iopts = PG::Connection.conndefaults.each_with_object({}){|h, o| o[h[:keyword].to_sym] = h[:val] if h[:val] }.merge(iopts) + iopts_for_reset = iopts if iopts[:hostaddr] # hostaddr is provided -> no need to resolve hostnames @@ -838,6 +841,8 @@ def new(*args) raise PG::ConnectionBad, conn.error_message if conn.status == PG::CONNECTION_BAD + # save the connection options for conn.reset + conn.instance_variable_set(:@iopts_for_reset, iopts_for_reset) conn.send(:async_connect_or_reset, :connect_poll) conn end diff --git a/spec/helpers.rb b/spec/helpers.rb index 606c9d54a..6c66562a4 100644 --- a/spec/helpers.rb +++ b/spec/helpers.rb @@ -678,8 +678,8 @@ def with_env_vars(**kwargs) end # Append or change 'rubypg_test' host entry in /etc/hosts to a given IP address - def set_etc_hosts(hostaddr) - system "sudo --non-interactive sed -i '/.* rubypg_test$/{h;s/.*/#{hostaddr} rubypg_test/};${x;/^$/{s//#{hostaddr} rubypg_test/;H};x}' /etc/hosts" or skip("unable to change /etc/hosts file") + def set_etc_hosts(hostaddr, hostname) + system "sudo --non-interactive sed -i '/.* #{hostname}$/{h;s/.*/#{hostaddr} #{hostname}/};${x;/^$/{s//#{hostaddr} #{hostname}/;H};x}' /etc/hosts" or skip("unable to change /etc/hosts file") end end diff --git a/spec/pg/connection_async_spec.rb b/spec/pg/connection_async_spec.rb index 36e6be7e3..efbe01b50 100644 --- a/spec/pg/connection_async_spec.rb +++ b/spec/pg/connection_async_spec.rb @@ -135,4 +135,25 @@ def interrupt_thread(exc=nil) end end + it "doesn't duplicate hosts in conn.reset", :without_transaction, :ipv6, :postgresql_10 do + set_etc_hosts "::1", "rubypg_test2 rubypg_test_ipv6" + set_etc_hosts "127.0.0.1", "rubypg_test2 rubypg_test_ipv4" + conn = described_class.connect( "postgres://rubypg_test2/test" ) + conn.exec("select 1") + expect( conn.conninfo_hash[:host] ).to eq( "rubypg_test2,rubypg_test2" ) + expect( conn.conninfo_hash[:hostaddr] ).to eq( "::1,127.0.0.1" ) + expect( conn.conninfo_hash[:port] ).to eq( "#{@port},#{@port}" ) + expect( conn.host ).to eq( "rubypg_test2" ) + expect( conn.hostaddr ).to eq( "::1" ) + expect( conn.port ).to eq( @port ) + + conn.reset + conn.exec("select 2") + expect( conn.conninfo_hash[:host] ).to eq( "rubypg_test2,rubypg_test2" ) + expect( conn.conninfo_hash[:hostaddr] ).to eq( "::1,127.0.0.1" ) + expect( conn.conninfo_hash[:port] ).to eq( "#{@port},#{@port}" ) + expect( conn.host ).to eq( "rubypg_test2" ) + expect( conn.hostaddr ).to eq( "::1" ) + expect( conn.port ).to eq( @port ) + end end diff --git a/spec/pg/connection_spec.rb b/spec/pg/connection_spec.rb index 47f4399e9..df5438ab3 100644 --- a/spec/pg/connection_spec.rb +++ b/spec/pg/connection_spec.rb @@ -1728,15 +1728,15 @@ end it "refreshes DNS address while conn.reset", :without_transaction, :ipv6 do - set_etc_hosts "::1" - conn = described_class.connect( "postgres://rubypg_test/test" ) + set_etc_hosts "::1", "rubypg_test1" + conn = described_class.connect( "postgres://rubypg_test1/test" ) conn.exec("select 1") - set_etc_hosts "127.0.0.1" + set_etc_hosts "127.0.0.1", "rubypg_test1" conn.reset conn.exec("select 1") - set_etc_hosts "::2" + set_etc_hosts "::2", "rubypg_test1" expect do conn.reset conn.exec("select 1")