[bug fix] connection service file doesn't take effect with ECPG apps

Hello,

I've found a small bug of ECPG and attached a patch. I tested the fix with
9.4. I'd like the fix to be back-ported.

[Problem]
The ECPG app runs the statement:

EXEC SQL CONNECT TO 'tcp:postgresql://?service=my_service';

I want this app to connect to any database based on the connection service
file.
For example, I wrote the following connection service file pg_service.conf,
placed it in the current directory, set PGSERVICEFILE environment variable
to point to it:

[my_service]
dbname = mydb
host = myhost
port = 5555

myhost is a different host than the one where the app runs.

Unfortunately, the app could not connect to the intended database. It tried
to connect to the (non-existent) database server on the local machine and
failed.

[Cause]
ECPGconnect() parses the URI and produces an empty host name. It passes an
empty string as the value for "host" connection parameter to
PQconnectdbParams().

Given an empty host name, PQconnectdbParams() ignores the host parameter in
pg_service.conf. When host is "", PQconnectdbParams() try to connect via
local UNIX domain socket.

[Fix]
It doesn't make sense for ECPGconnect() to pass an empty host name to
PQconnectdbParams(), so prevent it from passing host parameter for the
service setting to take effect. port is the same.

Regards
MauMau

On Tue, Dec 17, 2013 at 09:26:49PM +0900, MauMau wrote:
> [Problem]
> The ECPG app runs the statement:
>
> EXEC SQL CONNECT TO 'tcp:postgresql://?service=my_service';
> ...
> ECPGconnect() parses the URI and produces an empty host name. It
> passes an empty string as the value for "host" connection parameter
> to PQconnectdbParams().

Interestingly enough it works flawlessly on my system. Any idea where the
empoty host name comes from? Before accepting the patch I'd like to find out
why you got an empty host variable and I don't.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

From: "Michael Meskes" <meskes(at)postgresql(dot)org>
> On Tue, Dec 17, 2013 at 09:26:49PM +0900, MauMau wrote:
>> [Problem]
>> The ECPG app runs the statement:
>>
>> EXEC SQL CONNECT TO 'tcp:postgresql://?service=my_service';
>> ...
>> ECPGconnect() parses the URI and produces an empty host name. It
>> passes an empty string as the value for "host" connection parameter
>> to PQconnectdbParams().
>
> Interestingly enough it works flawlessly on my system. Any idea where the
> empoty host name comes from? Before accepting the patch I'd like to find
> out
> why you got an empty host variable and I don't.

You can confirm it by adding the following code fragment to
ecpglib/connect.c. The attached file contains this.

if (host)
printf("host=%s\n", host);
else
printf("host=NULL\n");

Build and run the attached sample program like this (this is an example on
Windows, but the result should be the same on Linux):

ecpg connect.pgc
cl /nologo /MD /I<pg_inst_dir>\include connect.c /link
/libpath:<pg_inst_dir>\lib libecpg.lib
connect.exe

The added code in ecpglib/connect.c displays "host=", which shows that an
empty host is passed to PQconnectdbParams().
Of course, on Linux, you can use gdb to run the sample program, set a
breakpoint at PQconnectDbParams(), and display the keywords/values arrays of
the argument.

An empty string is set to the host local variable at line 430 in
ecpglib/connect.c (this line number is that of PostgreSQL 9.4). That is the
else block shown below:

if (strncmp(dbname, "unix:", 5) == 0)
{
...
}
else
{
host = ecpg_strdup(dbname + offset, lineno);
connect_params++;
}

Regards
MauMau

On Sat, Dec 28, 2013 at 04:37:44PM +0900, MauMau wrote:
> You can confirm it by adding the following code fragment to
> ecpglib/connect.c. The attached file contains this.
> ...

Sorry for not being precide enough. I did run some tests and it works like a charm for me.

Or in other words, I used the connect command you had in your email with a
services file pointing to a local database and it connected to that database.
Instead of adding an additional output I checked the log output which suggested
that host was NULL.

While I cannot see how your patch could do any harm, I'd still like to figure
out why I don't see the same behaviour.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

From: "Michael Meskes" <meskes(at)postgresql(dot)org>
> Or in other words, I used the connect command you had in your email with a
> services file pointing to a local database and it connected to that
> database.
> Instead of adding an additional output I checked the log output which
> suggested
> that host was NULL.

Your test case seems different from my original mail. In my test case, I
want to connect to a database on another machine, not on the local one. For
example:

1. The ECPG app runs on a machine called client-host.
2. The database server to connect to runs on a machine called server-host.
3. There's no database server running on client-host.
4. The ECPG app uses the connection service file whose contents is:
[my_service]
host=server-host
... other necessary parameters like port, dbname, etc.

The app mistakenly tries to connect to the database server on the local
machine (client-host), instead of the desired server-host, and fails to
connect because the database server is not running on client-host. This is
because ECPGconnect() passes an empty string (""), not NULL, to
PQconnectdbParams().

Regards
MauMau

On Sun, Dec 29, 2013 at 05:35:22AM +0900, MauMau wrote:
> Your test case seems different from my original mail. In my test

As it turned out that wasn't the problem. It was simple typo on my side. Sorry
for the hassle.

However, I'd prefer to solve the problem slightly differently by not creating
an empty host variable instead of checking for it after the fact. But I take it
you don't mind that.

Fixed in HEAD and all back branches. Thanks for the report.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

From: "Michael Meskes" <meskes(at)postgresql(dot)org>
> However, I'd prefer to solve the problem slightly differently by not
> creating
> an empty host variable instead of checking for it after the fact. But I
> take it
> you don't mind that.
>
> Fixed in HEAD and all back branches. Thanks for the report.

Thank you for committing the fix. I'm comfortable with your change.

Regards
MauMau