Обсуждение: Re: [HACKERS] PostgreSQL 8.0.3 and Ipv6

Поиск
Список
Период
Сортировка

Re: [HACKERS] PostgreSQL 8.0.3 and Ipv6

От
Andrew Dunstan
Дата:

Tom Lane wrote:

>Andrew Dunstan <andrew@dunslane.net> writes:
>
>
>>The one place that very slightly bothers me is the ::1 line in
>>pg_hba.conf.  The fact that it comes last in the default config is its
>>saving grace - it won't ever be reached by a passing connection. I think
>>at least, though, we should put a warning comment line in front of it,
>>
>>
>
>If you like, you can improve initdb to comment that line out if
>getaddrinfo chokes on "::1", rather than believing HAVE_IPV6.
>
>
>
>

Good idea. Here's a patch for that. Rather than commenting it out I used
the slightly newer initdb facility to remove it and the associated
comment line altogether.

Passes "make check" on my linux box.

cheers

andrew
Index: src/bin/initdb/initdb.c
===================================================================
RCS file: /home/cvsmirror/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.94
diff -c -r1.94 initdb.c
*** src/bin/initdb/initdb.c    2 Aug 2005 15:16:27 -0000    1.94
--- src/bin/initdb/initdb.c    21 Aug 2005 23:41:25 -0000
***************
*** 148,156 ****
  static char *xstrdup(const char *s);
  static char **replace_token(char **lines,
                              const char *token, const char *replacement);
- #ifndef HAVE_UNIX_SOCKETS
  static char **filter_lines_with_token(char **lines, const char *token);
- #endif
  static char **readfile(char *path);
  static void writefile(char *path, char **lines);
  static FILE *popen_check(const char *command, const char *mode);
--- 148,154 ----
***************
*** 330,336 ****
   *
   * a sort of poor man's grep -v
   */
- #ifndef HAVE_UNIX_SOCKETS
  static char **
  filter_lines_with_token(char **lines, const char *token)
  {
--- 328,333 ----
***************
*** 351,357 ****

      return result;
  }
- #endif

  /*
   * get the lines from a text file
--- 348,353 ----
***************
*** 1157,1162 ****
--- 1153,1170 ----
      char      **conflines;
      char        repltok[100];
      char        path[MAXPGPATH];
+
+     struct addrinfo *gai_result;
+     struct addrinfo hints;
+
+     hints.ai_flags = AI_NUMERICHOST;
+     hints.ai_family = PF_UNSPEC;
+     hints.ai_socktype = 0;
+     hints.ai_protocol = 0;
+     hints.ai_addrlen = 0;
+     hints.ai_canonname = NULL;
+     hints.ai_addr = NULL;
+     hints.ai_next = NULL;

      fputs(_("creating configuration files ... "), stdout);
      fflush(stdout);
***************
*** 1210,1220 ****
      conflines = replace_token(conflines,"@remove-line-for-nolocal@","");
  #endif

! #ifndef HAVE_IPV6
!     conflines = replace_token(conflines,
!                               "host    all         all         ::1",
!                               "#host    all         all         ::1");
! #endif

      /* Replace default authentication methods */
      conflines = replace_token(conflines,
--- 1218,1234 ----
      conflines = replace_token(conflines,"@remove-line-for-nolocal@","");
  #endif

!     /*
!      * runtime test for IPv6 support (previously compile time test).
!      * this lets us build on hosts with IPv6 support and run
!      * on hosts without, especially on Windows.
!      *
!      */
!     if (getaddrinfo("::1", NULL, &hints, &gai_result) != 0)
!         conflines = filter_lines_with_token(conflines,
!                                             "@remove-line-for-no-ip6@");
!     else
!         conflines = replace_token(conflines,"@remove-line-for-no-ip6@","");

      /* Replace default authentication methods */
      conflines = replace_token(conflines,
Index: src/backend/libpq/pg_hba.conf.sample
===================================================================
RCS file: /home/cvsmirror/pgsql/src/backend/libpq/pg_hba.conf.sample,v
retrieving revision 1.59
diff -c -r1.59 pg_hba.conf.sample
*** src/backend/libpq/pg_hba.conf.sample    15 Aug 2005 02:40:25 -0000    1.59
--- src/backend/libpq/pg_hba.conf.sample    21 Aug 2005 23:33:30 -0000
***************
*** 67,71 ****
  @remove-line-for-nolocal@local   all         all                               @authmethod@
  # IPv4 local connections:
  host    all         all         127.0.0.1/32          @authmethod@
! # IPv6 local connections:
! host    all         all         ::1/128               @authmethod@
--- 67,71 ----
  @remove-line-for-nolocal@local   all         all                               @authmethod@
  # IPv4 local connections:
  host    all         all         127.0.0.1/32          @authmethod@
! @remove-line-for-no-ip6@# IPv6 local connections:
! @remove-line-for-no-ip6@host    all         all         ::1/128               @authmethod@


Re: [HACKERS] PostgreSQL 8.0.3 and Ipv6

От
Tom Lane
Дата:
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> If you like, you can improve initdb to comment that line out if
>> getaddrinfo chokes on "::1", rather than believing HAVE_IPV6.

> Good idea. Here's a patch for that. Rather than commenting it out I used
> the slightly newer initdb facility to remove it and the associated
> comment line altogether.

Hm, is that really better than just commenting it out?  Particularly
on Windows, where we could imagine someone installing a newer version
of the relevant DLL and then wanting to use IPv6.  Seems to me that
leaving the line present but commented out is the right thing, because
it documents how things should look for IPv6.

            regards, tom lane

Re: [HACKERS] PostgreSQL 8.0.3 and Ipv6

От
Andrew Dunstan
Дата:

Tom Lane wrote:

>Andrew Dunstan <andrew@dunslane.net> writes:
>
>
>>Tom Lane wrote:
>>
>>
>>>If you like, you can improve initdb to comment that line out if
>>>getaddrinfo chokes on "::1", rather than believing HAVE_IPV6.
>>>
>>>
>
>
>
>>Good idea. Here's a patch for that. Rather than commenting it out I used
>>the slightly newer initdb facility to remove it and the associated
>>comment line altogether.
>>
>>
>
>Hm, is that really better than just commenting it out?  Particularly
>on Windows, where we could imagine someone installing a newer version
>of the relevant DLL and then wanting to use IPv6.  Seems to me that
>leaving the line present but commented out is the right thing, because
>it documents how things should look for IPv6.
>
>
>
>

Seemed to me slightly less potentially confusing, but I don't feel
strongly. Try this instead if you prefer.

cheers

andrew
Index: src/bin/initdb/initdb.c
===================================================================
RCS file: /home/cvsmirror/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.94
diff -c -r1.94 initdb.c
*** src/bin/initdb/initdb.c    2 Aug 2005 15:16:27 -0000    1.94
--- src/bin/initdb/initdb.c    22 Aug 2005 13:35:22 -0000
***************
*** 1157,1162 ****
--- 1157,1174 ----
      char      **conflines;
      char        repltok[100];
      char        path[MAXPGPATH];
+
+     struct addrinfo *gai_result;
+     struct addrinfo hints;
+
+     hints.ai_flags = AI_NUMERICHOST;
+     hints.ai_family = PF_UNSPEC;
+     hints.ai_socktype = 0;
+     hints.ai_protocol = 0;
+     hints.ai_addrlen = 0;
+     hints.ai_canonname = NULL;
+     hints.ai_addr = NULL;
+     hints.ai_next = NULL;

      fputs(_("creating configuration files ... "), stdout);
      fflush(stdout);
***************
*** 1210,1220 ****
      conflines = replace_token(conflines,"@remove-line-for-nolocal@","");
  #endif

! #ifndef HAVE_IPV6
!     conflines = replace_token(conflines,
!                               "host    all         all         ::1",
!                               "#host    all         all         ::1");
! #endif

      /* Replace default authentication methods */
      conflines = replace_token(conflines,
--- 1222,1239 ----
      conflines = replace_token(conflines,"@remove-line-for-nolocal@","");
  #endif

!
!     /*
!      * runtime test for IPv6 support (previously compile time test).
!      * this lets us build on hosts with IPv6 support and run
!      * on hosts without, especially on Windows.
!      *
!      */
!     if (getaddrinfo("::1", NULL, &hints, &gai_result) != 0)
!         conflines = replace_token(conflines,
!                                   "host    all         all         ::1",
!                                   "#host    all         all         ::1");
!

      /* Replace default authentication methods */
      conflines = replace_token(conflines,

Re: [HACKERS] PostgreSQL 8.0.3 and Ipv6

От
Tom Lane
Дата:
Andrew Dunstan <andrew@dunslane.net> writes:
> Try this instead if you prefer.

I thought this was a little bit too trusting about there being a
getaddrinfo to probe, so I tightened it up as per the attached
applied patch.

Where are we at this point on the Windows/IPv6 issue --- are there
more fixes to come, or is it done?

            regards, tom lane


*** src/bin/initdb/initdb.c.orig    Tue Aug  2 11:16:27 2005
--- src/bin/initdb/initdb.c    Mon Aug 22 12:24:42 2005
***************
*** 54,66 ****
  #include <unistd.h>
  #include <locale.h>
  #include <signal.h>
- #include <errno.h>
  #ifdef HAVE_LANGINFO_H
  #include <langinfo.h>
  #endif

  #include "libpq/pqsignal.h"
  #include "mb/pg_wchar.h"
  #include "getopt_long.h"

  #ifndef HAVE_INT_OPTRESET
--- 54,66 ----
  #include <unistd.h>
  #include <locale.h>
  #include <signal.h>
  #ifdef HAVE_LANGINFO_H
  #include <langinfo.h>
  #endif

  #include "libpq/pqsignal.h"
  #include "mb/pg_wchar.h"
+ #include "getaddrinfo.h"
  #include "getopt_long.h"

  #ifndef HAVE_INT_OPTRESET
***************
*** 1210,1220 ****
      conflines = replace_token(conflines,"@remove-line-for-nolocal@","");
  #endif

! #ifndef HAVE_IPV6
      conflines = replace_token(conflines,
                                "host    all         all         ::1",
                                "#host    all         all         ::1");
! #endif

      /* Replace default authentication methods */
      conflines = replace_token(conflines,
--- 1210,1251 ----
      conflines = replace_token(conflines,"@remove-line-for-nolocal@","");
  #endif

! #if defined(HAVE_IPV6) && defined(HAVE_STRUCT_ADDRINFO) && defined(HAVE_GETADDRINFO)
!     /*
!      * Probe to see if there is really any platform support for IPv6, and
!      * comment out the relevant pg_hba line if not.  This avoids runtime
!      * warnings if getaddrinfo doesn't actually cope with IPv6.  Particularly
!      * useful on Windows, where executables built on a machine with IPv6
!      * may have to run on a machine without.
!      *
!      * We don't bother with testing if we aren't using the system version
!      * of getaddrinfo, since we know our own version doesn't do IPv6.
!      */
!     {
!         struct addrinfo *gai_result;
!         struct addrinfo hints;
!
!         /* for best results, this code should match parse_hba() */
!         hints.ai_flags = AI_NUMERICHOST;
!         hints.ai_family = PF_UNSPEC;
!         hints.ai_socktype = 0;
!         hints.ai_protocol = 0;
!         hints.ai_addrlen = 0;
!         hints.ai_canonname = NULL;
!         hints.ai_addr = NULL;
!         hints.ai_next = NULL;
!
!         if (getaddrinfo("::1", NULL, &hints, &gai_result) != 0)
!             conflines = replace_token(conflines,
!                                       "host    all         all         ::1",
!                                       "#host    all         all         ::1");
!     }
! #else /* !HAVE_IPV6 etc */
!     /* If we didn't compile IPV6 support at all, always comment it out */
      conflines = replace_token(conflines,
                                "host    all         all         ::1",
                                "#host    all         all         ::1");
! #endif /* HAVE_IPV6 etc */

      /* Replace default authentication methods */
      conflines = replace_token(conflines,

Re: [HACKERS] PostgreSQL 8.0.3 and Ipv6

От
Andrew Dunstan
Дата:

Tom Lane wrote:

>Andrew Dunstan <andrew@dunslane.net> writes:
>
>
>>Try this instead if you prefer.
>>
>>
>
>I thought this was a little bit too trusting about there being a
>getaddrinfo to probe, so I tightened it up as per the attached
>applied patch.
>
>Where are we at this point on the Windows/IPv6 issue --- are there
>more fixes to come, or is it done?
>
>

Not done yet. One thing left.

The idea was that we would put dynamic testing of properly working
getaddrinfo and friends on Windows into our getaddrinfo.c.  That would
be the "local tweak" you mentioned previously ;-)

In consequence of that plan, I think we would need to remove "&&
defined(HAVE_GETADDRINFO)" from your applied patch and let it fall
through to our homegrown getaddrinfo if there isn't one. On most such
platforms it would fail, consuming a few more millisecs, but with
Windows with the expected patch it could pass.

(I know it's a muddle - I can't think how we came not to do IPv6 for
Windows in 8.0, or at the very least put it on the TODO list.)

cheers

andrew