From 016c17c9c9bbc51bf5fab2087ad485367c789118 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Wed, 28 Feb 2024 08:41:20 +0100 Subject: [PATCH] libpq resolves the host address while PQreset, but ruby-pg doesn't. This is because we explicit set the `hostaddr` connection parameter when the connection is established the first time. This prevents a newly DNS resolution when running PQresetStart. This patch adds DNS resolution to `conn.reset` Since we can not change the connection parameters after connection start, the underlying PGconn pointer is exchanged in reset_start2. This is done by a PQfinish() + PQconnectStart() sequence. That way the `hostaddr` parameter is updated and a new connection is established with it. There is a `/etc/hosts` and `sudo` based test in the specs. The behavior of libpq is slightly different to that of ruby-pg. It can be verified by the following code: ```ruby require "pg" puts "pg version: #{PG::VERSION}" system "sudo sed -i 's/.* abcd/::1 abcd/g' /etc/hosts" conn = PG.connect host: "abcd", password: "l" conn.exec("select 1") p conn.conninfo_hash.slice(:host, :hostaddr, :port) system "sudo sed -i 's/.* abcd/127.0.0.1 abcd/g' /etc/hosts" conn.reset conn.exec("select 1") p conn.conninfo_hash.slice(:host, :hostaddr, :port) system "sudo sed -i 's/.* abcd/::2 abcd/g' /etc/hosts" conn.reset conn.exec("select 1") p conn.conninfo_hash.slice(:host, :hostaddr, :port) ``` This gives the following output showing, that the IP address is updated: ``` pg version: 1.5.5 {:host=>"abcd", :hostaddr=>"::1", :port=>"5432"} {:host=>"abcd", :hostaddr=>"127.0.0.1", :port=>"5432"} ruby-pg/lib/pg/connection.rb:573:in `reset_start2': connection to server at "::2", port 5432 failed: Network is unreachable (PG::ConnectionBad) Is the server running on that host and accepting TCP/IP connections? ``` Whereas libpq resolves similarly with `async_api=false`, but doesn't raise the error in `conn.reset` but in the subsequent `conn.exec`. ``` pg version: 1.5.5 {:host=>"abcd", :hostaddr=>nil, :port=>"5432"} {:host=>"abcd", :hostaddr=>nil, :port=>"5432"} test-reset-dns.rb:18:in `sync_exec': no connection to the server (PG::UnableToSend) ``` Fixes #558 --- ext/pg_connection.c | 22 ++++++++++++ lib/pg/connection.rb | 73 +++++++++++++++++++++----------------- spec/helpers.rb | 5 +++ spec/pg/connection_spec.rb | 17 +++++++++ 4 files changed, 85 insertions(+), 32 deletions(-) diff --git a/ext/pg_connection.c b/ext/pg_connection.c index 8d7d31013..3d74961ae 100644 --- a/ext/pg_connection.c +++ b/ext/pg_connection.c @@ -563,6 +563,27 @@ pgconn_sync_reset( VALUE self ) return self; } +static VALUE +pgconn_reset_start2( VALUE self, VALUE conninfo ) +{ + t_pg_connection *this = pg_get_connection( self ); + + /* Close old connection */ + pgconn_close_socket_io( self ); + PQfinish( this->pgconn ); + + /* Start new connection */ + this->pgconn = gvl_PQconnectStart( StringValueCStr(conninfo) ); + + if( this->pgconn == NULL ) + rb_raise(rb_ePGerror, "PQconnectStart() unable to allocate PGconn structure"); + + if ( PQstatus(this->pgconn) == CONNECTION_BAD ) + pg_raise_conn_error( rb_eConnectionBad, self, "%s", PQerrorMessage(this->pgconn)); + + return Qnil; +} + /* * call-seq: * conn.reset_start() -> nil @@ -4468,6 +4489,7 @@ init_pg_connection(void) rb_define_method(rb_cPGconn, "finished?", pgconn_finished_p, 0); rb_define_method(rb_cPGconn, "sync_reset", pgconn_sync_reset, 0); rb_define_method(rb_cPGconn, "reset_start", pgconn_reset_start, 0); + rb_define_private_method(rb_cPGconn, "reset_start2", pgconn_reset_start2, 1); rb_define_method(rb_cPGconn, "reset_poll", pgconn_reset_poll, 0); rb_define_alias(rb_cPGconn, "close", "finish"); diff --git a/lib/pg/connection.rb b/lib/pg/connection.rb index a8d07bb37..47647697c 100644 --- a/lib/pg/connection.rb +++ b/lib/pg/connection.rb @@ -565,7 +565,12 @@ def encrypt_password( password, username, algorithm=nil ) # Resets the backend connection. This method closes the # backend connection and tries to re-connect. def reset - reset_start + iopts = conninfo_hash.compact + if iopts[:host] && !iopts[:host].empty? && PG.library_version >= 100000 + iopts = self.class.send(:resolve_hosts, iopts) + end + conninfo = self.class.parse_connect_args( iopts ); + reset_start2(conninfo) async_connect_or_reset(:reset_poll) self end @@ -773,6 +778,40 @@ def new(*args) alias setdb new alias setdblogin new + # Resolve DNS in Ruby to avoid blocking state while connecting. + # Multiple comma-separated values are generated, if the hostname resolves to both IPv4 and IPv6 addresses. + # This requires PostgreSQL-10+, so no DNS resolving is done on earlier versions. + private def resolve_hosts(iopts) + ihosts = iopts[:host].split(",", -1) + iports = iopts[:port].split(",", -1) + iports = [nil] if iports.size == 0 + iports = iports * ihosts.size if iports.size == 1 + raise PG::ConnectionBad, "could not match #{iports.size} port numbers to #{ihosts.size} hosts" if iports.size != ihosts.size + + dests = ihosts.each_with_index.flat_map do |mhost, idx| + unless host_is_named_pipe?(mhost) + if Fiber.respond_to?(:scheduler) && + Fiber.scheduler && + RUBY_VERSION < '3.1.' + + # Use a second thread to avoid blocking of the scheduler. + # `TCPSocket.gethostbyname` isn't fiber aware before ruby-3.1. + hostaddrs = Thread.new{ Addrinfo.getaddrinfo(mhost, nil, nil, :STREAM).map(&:ip_address) rescue [''] }.value + else + hostaddrs = Addrinfo.getaddrinfo(mhost, nil, nil, :STREAM).map(&:ip_address) rescue [''] + end + else + # No hostname to resolve (UnixSocket) + hostaddrs = [nil] + end + hostaddrs.map { |hostaddr| [hostaddr, mhost, iports[idx]] } + end + iopts.merge( + hostaddr: dests.map{|d| d[0] }.join(","), + host: dests.map{|d| d[1] }.join(","), + port: dests.map{|d| d[2] }.join(",")) + end + private def connect_to_hosts(*args) option_string = parse_connect_args(*args) iopts = PG::Connection.conninfo_parse(option_string).each_with_object({}){|h, o| o[h[:keyword].to_sym] = h[:val] if h[:val] } @@ -782,37 +821,7 @@ def new(*args) # hostaddr is provided -> no need to resolve hostnames elsif iopts[:host] && !iopts[:host].empty? && PG.library_version >= 100000 - # Resolve DNS in Ruby to avoid blocking state while connecting. - # Multiple comma-separated values are generated, if the hostname resolves to both IPv4 and IPv6 addresses. - # This requires PostgreSQL-10+, so no DNS resolving is done on earlier versions. - ihosts = iopts[:host].split(",", -1) - iports = iopts[:port].split(",", -1) - iports = [nil] if iports.size == 0 - iports = iports * ihosts.size if iports.size == 1 - raise PG::ConnectionBad, "could not match #{iports.size} port numbers to #{ihosts.size} hosts" if iports.size != ihosts.size - - dests = ihosts.each_with_index.flat_map do |mhost, idx| - unless host_is_named_pipe?(mhost) - if Fiber.respond_to?(:scheduler) && - Fiber.scheduler && - RUBY_VERSION < '3.1.' - - # Use a second thread to avoid blocking of the scheduler. - # `TCPSocket.gethostbyname` isn't fiber aware before ruby-3.1. - hostaddrs = Thread.new{ Addrinfo.getaddrinfo(mhost, nil, nil, :STREAM).map(&:ip_address) rescue [''] }.value - else - hostaddrs = Addrinfo.getaddrinfo(mhost, nil, nil, :STREAM).map(&:ip_address) rescue [''] - end - else - # No hostname to resolve (UnixSocket) - hostaddrs = [nil] - end - hostaddrs.map { |hostaddr| [hostaddr, mhost, iports[idx]] } - end - iopts.merge!( - hostaddr: dests.map{|d| d[0] }.join(","), - host: dests.map{|d| d[1] }.join(","), - port: dests.map{|d| d[2] }.join(",")) + iopts = resolve_hosts(iopts) else # No host given end diff --git a/spec/helpers.rb b/spec/helpers.rb index 8f313ecea..c49baecae 100644 --- a/spec/helpers.rb +++ b/spec/helpers.rb @@ -675,6 +675,11 @@ def with_env_vars(**kwargs) ENV.update(old_values) end 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") + end end diff --git a/spec/pg/connection_spec.rb b/spec/pg/connection_spec.rb index 67ddcedad..caac2e50a 100644 --- a/spec/pg/connection_spec.rb +++ b/spec/pg/connection_spec.rb @@ -1690,6 +1690,23 @@ conn.close end + it "refreshs DNS address while conn.reset", :without_transaction, :ipv6 do + set_etc_hosts "::1" + conn = described_class.connect( "postgres://rubypg_test/test" ) + conn.exec("select 1") + + set_etc_hosts "127.0.0.1" + conn.reset + conn.exec("select 1") + + set_etc_hosts "::2" + expect do + conn.reset + conn.exec("select 1") + end.to raise_error(PG::Error) + end + + it "closes the IO fetched from #socket_io when the connection is closed", :without_transaction do conn = PG.connect( @conninfo ) io = conn.socket_io