Thread: Re: [HACKERS] PostgreSQL 8.0.3 and Ipv6
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@
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
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,
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,
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