Remove dead code in ECPGconnect(), and improve documentation.

The stanza in ECPGconnect() that intended to allow specification of a
Unix socket directory path in place of a port has never executed since
it was committed, nearly two decades ago; the preceding strrchr()
already found the last colon so there cannot be another one.  The lack
of complaints about that is doubtless related to the fact that no
user-facing documentation suggested it was possible.

Rather than try to fix that up, let's just remove the unreachable
code, and instead document the way that does work to write a socket
directory path, namely specifying it as a "host" option.

In support of that, make another pass at clarifying the syntax
documentation for ECPG connection targets, particularly documenting
which things are parsed as identifiers and where to use double quotes.
Rearrange some things that seemed poorly ordered, and fix a couple of
minor doc errors.

Kyotaro Horiguchi, per gripe from Shenhao Wang
(docs changes mostly by me)

Discussion: https://postgr.es/m/ae52a416bbbf459c96bab30b3038e06c@G08CNEXMBPEKD06.g08.fujitsu.local

diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 0fef9bfcbe5d91b17e5b214f3736ea74119d3ce8..9310a7116628f21dda372fb0677468190631308d 100644 (file)
--- 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>
 
@@ -143,17 +143,26 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
     </listitem>
    </itemizedlist>
 
-   If you specify the connection target literally (that is, not
-   through a variable reference) and you don't quote the value, then
-   the case-insensitivity rules of normal SQL are applied.  In that
-   case you can also double-quote the individual parameters separately
-   as needed.  In practice, it is probably less error-prone to use a
-   (single-quoted) string literal or a variable reference.  The
-   connection target <literal>DEFAULT</literal> initiates a connection
+   The connection target <literal>DEFAULT</literal> initiates a connection
    to the default database under the default user name.  No separate
    user name or connection name can be specified in that case.
   </para>
 
+  <para>
+   If you specify the connection target directly (that is, not as a string
+   literal or variable reference), then the components of the target are
+   passed through normal SQL parsing; this means that, for example,
+   the <replaceable>hostname</replaceable> must look like one or more SQL
+   identifiers separated by dots, and those identifiers will be
+   case-folded unless double-quoted.  Values of
+   any <replaceable>options</replaceable> must be SQL identifiers,
+   integers, or variable references.  Of course, you can put nearly
+   anything into an SQL identifier by double-quoting it.
+   In practice, it is probably less error-prone to use a (single-quoted)
+   string literal or a variable reference than to write the connection
+   target directly.
+  </para>
+
   <para>
    There are also different ways to specify the user name:
 
@@ -201,6 +210,15 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
    write <literal>&amp;</literal> within a <replaceable>value</replaceable>.
   </para>
 
+  <para>
+   Notice that when specifying a socket connection
+   (with the <literal>unix:</literal> prefix), the host name must be
+   exactly <literal>localhost</literal>.  To select a non-default
+   socket directory, write the directory's pathname as the value of
+   a <varname>host</varname> option in
+   the <replaceable>options</replaceable> part of the target.
+  </para>
+
   <para>
    The <replaceable>connection-name</replaceable> is used to handle
    multiple connections in one program.  It can be omitted if a
@@ -210,24 +228,12 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
    chapter).
   </para>
 
-  <para>
-   If untrusted users have access to a database that has not adopted a
-   <link linkend="ddl-schemas-patterns">secure schema usage pattern</link>,
-   begin each session by removing publicly-writable schemas
-   from <varname>search_path</varname>.  For example,
-   add <literal>options=-c search_path=</literal>
-   to <literal><replaceable>options</replaceable></literal>, or
-   issue <literal>EXEC SQL SELECT pg_catalog.set_config('search_path', '',
-   false);</literal> after connecting.  This consideration is not specific to
-   ECPG; it applies to every interface for executing arbitrary SQL commands.
-  </para>
-
   <para>
    Here are some examples of <command>CONNECT</command> statements:
 <programlisting>
 EXEC SQL CONNECT TO mydb@sql.mydomain.com;
 
-EXEC SQL CONNECT TO unix:postgresql://sql.mydomain.com/mydb AS myconnection USER john;
+EXEC SQL CONNECT TO tcp:postgresql://sql.mydomain.com/mydb AS myconnection USER john;
 
 EXEC SQL BEGIN DECLARE SECTION;
 const char *target = "mydb@sql.mydomain.com";
@@ -238,8 +244,8 @@ EXEC SQL END DECLARE SECTION;
 EXEC SQL CONNECT TO :target USER :user USING :passwd;
 /* or EXEC SQL CONNECT TO :target USER :user/:passwd; */
 </programlisting>
-   The last form makes use of the variant referred to above as
-   character variable reference.  You will see in later sections how C
+   The last example makes use of the feature referred to above as
+   character variable references.  You will see in later sections how C
    variables can be used in SQL statements when you prefix them with a
    colon.
   </para>
@@ -251,6 +257,18 @@ EXEC SQL CONNECT TO :target USER :user USING :passwd;
    example above to encapsulate the connection target string
    somewhere.
   </para>
+
+  <para>
+   If untrusted users have access to a database that has not adopted a
+   <link linkend="ddl-schemas-patterns">secure schema usage pattern</link>,
+   begin each session by removing publicly-writable schemas
+   from <varname>search_path</varname>.  For example,
+   add <literal>options=-c search_path=</literal>
+   to <literal><replaceable>options</replaceable></literal>, or
+   issue <literal>EXEC SQL SELECT pg_catalog.set_config('search_path', '',
+   false);</literal> after connecting.  This consideration is not specific to
+   ECPG; it applies to every interface for executing arbitrary SQL commands.
+  </para>
   </sect2>
 
   <sect2 id="ecpg-set-connection">

diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 6b0a3067e6c124d1f99016e375c0f2480550cde2..60564b176c92f8c5d3b1103f0d1f2315b3123b3b 100644 (file)
--- 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,46 +384,22 @@ 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)
                {
-                   if (strcmp(dbname + offset, "localhost") != 0 && strcmp(dbname + offset, "127.0.0.1") != 0)
+                   /*
+                    * The alternative of using "127.0.0.1" here is deprecated
+                    * and undocumented; we'll keep it for backward
+                    * compatibility's sake, but not extend it to allow IPv6.
+                    */
+                   if (strcmp(dbname + offset, "localhost") != 0 &&
+                       strcmp(dbname + offset, "127.0.0.1") != 0)
                    {
                        ecpg_log("ECPGconnect: non-localhost access via sockets on line %d\n", lineno);
                        ecpg_raise(lineno, ECPG_CONNECT, ECPG_SQLSTATE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION, realname ? realname : ecpg_gettext("<DEFAULT>"));
@@ -450,7 +425,6 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
                        connect_params++;
                    }
                }
-
            }
        }
        else