Re: parse mistake in ecpg connect string

At Tue, 9 Feb 2021 02:12:37 +0000, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com> wrote in 
> Dear Wang, Tom
> 
> > I think add the bison rule is a little difficult because in PG13 windows can also support unix-socket, 
> > In your patch:
> > > dir_name: '/' dir_name{ $$ = make2_str(mm_strdup("/"), $2); }
> > > | ecpg_ident{ $$ = $1; }
> > >;
> > Windows will remains wrong(I'm not sure ecpg on windows can use unix socket connection).
> > 
> > And if we add the rules in bison files, both ecpg and ecpglib will both parse the host in different ways.
> > Ecpg parse the host by bison rules, and ecpglib parse the host by splitting the connect string use char '@' or char
'='.
> > I think it's not a good action.
> > 
> > But If we add some description on docs, these problem can be solved in an easy way.
> > Therefore, I prefer to add some description on docs.
> 
> I didn't care about the windows environment.
> Somewhat WIN32 directive can be used for switching code, but I agree your claims.

This thread looks like discussing about unix-domain socket on
Windows. (I'll look into it.)

> > I think we can add some description on docs, but I don't have ability to write description in English,
> > Can someone help me write a description?
> 
> I'm also not a native English speaker, but I put a draft.
> Please review it and combine them if it's OK.
> 
> > Should we allow "::1" here as well?  On the other hand, colons are
> > already overloaded in this syntax, so maybe allowing them in the
> > host part is a bad idea.

Yeah, that made me smile for the same reason:p

> I have no idea how to fix it now, so I added notice that IPv6 should not be used
> in the host part...

Anyway the host part for the unix: method is just for
spelling. Although I think we can further remove ipv4 address, we
don't even need to bother that. (However, I don't object to add "::1"
either.)

I think replacing "hostname" in the unix: method to "localhost" works.

> dbname[@hostname][:port]
> tcp:postgresql://hostname[:port][/dbname][?options]
- unix:postgresql://<italic>hostname</>[:<i>port</>]..
+ unix:postgresql://<nonitalic>localhost</>[:<i>port</>]..

@@ -199,6 +199,13 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
    any <replaceable>keyword</replaceable> or <replaceable>value</replaceable>,
    though not within or after one.  Note that there is no way to
    write <literal>&</literal> within a <replaceable>value</replaceable>.
+
+   Also note that if you want to specify the socket directory
+   for Unix-domain communications, an option <replaceable>host=</replaceable>
+   and single-quoted string must be used.
+   The notation rule is almost the same as libpq's one,
+   but the IPv6 address cannot be used here.
+

<replaceable> is not the tag to use for "host". <varname> is that.

If we change the "hostname" for the unix: method to be fixed, no need
to mention IP address. (Even if someone tries using IP addresses
instead of localhost, that case is not our business:p)

How about the attached?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 0fef9bfcbe..cd34f6a69e 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -120,7 +120,7 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
 
     <listitem>
      <simpara>
-
<literal>unix:postgresql://<replaceable>hostname</replaceable><optional>:<replaceable>port</replaceable></optional><optional>/<replaceable>dbname</replaceable></optional><optional>?<replaceable>options</replaceable></optional></literal>
+
<literal>unix:postgresql://localhost<optional>:<replaceable>port</replaceable></optional><optional>/<replaceable>dbname</replaceable></optional><optional>?<replaceable>options</replaceable></optional></literal>
      </simpara>
     </listitem>
 
@@ -199,6 +199,10 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
    any <replaceable>keyword</replaceable> or <replaceable>value</replaceable>,
    though not within or after one.  Note that there is no way to
    write <literal>&</literal> within a <replaceable>value</replaceable>.
+   The host part of the unix: method URI is fixed, use
+   the <varname>host</varname> option in case of using a non-default socket
+   directory.  Since a directory path contains slashes, the whole target
+   should be an SQL string literal.
   </para>
 
   <para>
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 6b0a3067e6..f45892304d 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -360,8 +360,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 
                 /*------
                  * new style:
-                 *    <tcp|unix>:postgresql://server[:port|:/unixsocket/path:]
-                 *    [/db-name][?options]
+                 *    <tcp|unix>:postgresql://server[:port][/db-name][?options]
                  *------
                  */
                 offset += strlen("postgresql://");
@@ -385,41 +384,11 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
                 }
 
                 tmp = strrchr(dbname + offset, ':');
-                if (tmp != NULL)    /* port number or Unix socket path given */
+                if (tmp != NULL)    /* port number given */
                 {
-                    char       *tmp2;
-
                     *tmp = '\0';
-                    if ((tmp2 = strchr(tmp + 1, ':')) != NULL)
-                    {
-                        *tmp2 = '\0';
-                        host = ecpg_strdup(tmp + 1, lineno);
-                        connect_params++;
-                        if (strncmp(dbname, "unix:", 5) != 0)
-                        {
-                            ecpg_log("ECPGconnect: socketname %s given for TCP connection on line %d\n", host,
lineno);
-                            ecpg_raise(lineno, ECPG_CONNECT,
ECPG_SQLSTATE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION,realname ? realname : ecpg_gettext("<DEFAULT>"));
 
-                            if (host)
-                                ecpg_free(host);
-
-                            /*
-                             * port not set yet if (port) ecpg_free(port);
-                             */
-                            if (options)
-                                ecpg_free(options);
-                            if (realname)
-                                ecpg_free(realname);
-                            if (dbname)
-                                ecpg_free(dbname);
-                            free(this);
-                            return false;
-                        }
-                    }
-                    else
-                    {
-                        port = ecpg_strdup(tmp + 1, lineno);
-                        connect_params++;
-                    }
+                    port = ecpg_strdup(tmp + 1, lineno);
+                    connect_params++;
                 }
 
                 if (strncmp(dbname, "unix:", 5) == 0)