From bbfb5dcd8ece2bbf04cc7323b7cc03581d91d7bd Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 24 Dec 2017 16:42:04 +0000 Subject: [PATCH] Lookups: fix pgsql multiple-row, single-column return Report & fix from James ; additional tidying and testcase by JGH Broken-by: acec9514b1 --- doc/doc-txt/ChangeLog | 4 ++ src/src/lookups/pgsql.c | 86 ++++++++++++++----------------- test/scripts/2620-Postgresql/2620 | 4 ++ test/stderr/2620 | 24 +++++++++ test/stdout/2620 | 6 +++ 5 files changed, 78 insertions(+), 46 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 4897f4a0c..d13004b8c 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -16,6 +16,10 @@ JH/01 Replace the store_release() internal interface with store_newblock(), JH/02 Disallow '/' characters in queue names specified for the "queue=" ACL modifier. This matches the restriction on the commandline. +JH/03 Fix pgsql lookup for multiple result-tuples with a single column. + Previously only the last row was returned. + + Exim version 4.90 ----------------- diff --git a/src/src/lookups/pgsql.c b/src/src/lookups/pgsql.c index 0b771f59c..cece43b66 100644 --- a/src/src/lookups/pgsql.c +++ b/src/src/lookups/pgsql.c @@ -140,7 +140,7 @@ has the password removed. This copy is also used for debugging output. */ for (i = 2; i >= 0; i--) { uschar *pp = Ustrrchr(server, '/'); - if (pp == NULL) + if (!pp) { *errmsg = string_sprintf("incomplete pgSQL server data: %s", (i == 2)? server : server_copy); @@ -156,18 +156,16 @@ for (i = 2; i >= 0; i--) start is the identification of the server (host or path). See if we have a cached connection to the server. */ -for (cn = pgsql_connections; cn != NULL; cn = cn->next) - { +for (cn = pgsql_connections; cn; cn = cn->next) if (Ustrcmp(cn->server, server_copy) == 0) { pg_conn = cn->handle; break; } - } /* If there is no cached connection, we must set one up. */ -if (cn == NULL) +if (!cn) { uschar *port = US""; @@ -178,7 +176,7 @@ if (cn == NULL) uschar *last_slash, *last_dot, *p; p = ++server; - while (*p != 0 && *p != ')') p++; + while (*p && *p != ')') p++; *p = 0; last_slash = Ustrrchr(server, '/'); @@ -191,10 +189,9 @@ if (cn == NULL) We have to call PQsetdbLogin with '/var/run/postgresql' as the hostname argument and put '5432' into the port variable. */ - if (last_slash == NULL || last_dot == NULL) + if (!last_slash || !last_dot) { - *errmsg = string_sprintf("PGSQL invalid filename for socket: %s", - server); + *errmsg = string_sprintf("PGSQL invalid filename for socket: %s", server); *defer_break = TRUE; return DEFER; } @@ -211,13 +208,13 @@ if (cn == NULL) else { uschar *p; - if ((p = Ustrchr(server, ':')) != NULL) + if ((p = Ustrchr(server, ':'))) { *p++ = 0; port = p; } - if (Ustrchr(server, '/') != NULL) + if (Ustrchr(server, '/')) { *errmsg = string_sprintf("unexpected slash in pgSQL server hostname: %s", server); @@ -280,37 +277,37 @@ else /* Run the query */ - pg_result = PQexec(pg_conn, CS query); - switch(PQresultStatus(pg_result)) - { - case PGRES_EMPTY_QUERY: - case PGRES_COMMAND_OK: - /* The command was successful but did not return any data since it was - not SELECT but either an INSERT, UPDATE or DELETE statement. Tell the - high level code to not cache this query, and clean the current cache for - this handle by setting *do_cache zero. */ - - result = string_cat(result, US PQcmdTuples(pg_result)); - *do_cache = 0; - DEBUG(D_lookup) debug_printf("PGSQL: command does not return any data " - "but was successful. Rows affected: %s\n", result->s); - break; +pg_result = PQexec(pg_conn, CS query); +switch(PQresultStatus(pg_result)) + { + case PGRES_EMPTY_QUERY: + case PGRES_COMMAND_OK: + /* The command was successful but did not return any data since it was + not SELECT but either an INSERT, UPDATE or DELETE statement. Tell the + high level code to not cache this query, and clean the current cache for + this handle by setting *do_cache zero. */ + + result = string_cat(result, US PQcmdTuples(pg_result)); + *do_cache = 0; + DEBUG(D_lookup) debug_printf("PGSQL: command does not return any data " + "but was successful. Rows affected: %s\n", string_from_gstring(result)); + break; - case PGRES_TUPLES_OK: - break; + case PGRES_TUPLES_OK: + break; - default: - /* This was the original code: - *errmsg = string_sprintf("PGSQL: query failed: %s\n", - PQresultErrorMessage(pg_result)); - This was suggested by a user: - */ - - *errmsg = string_sprintf("PGSQL: query failed: %s (%s) (%s)\n", - PQresultErrorMessage(pg_result), - PQresStatus(PQresultStatus(pg_result)), query); - goto PGSQL_EXIT; - } + default: + /* This was the original code: + *errmsg = string_sprintf("PGSQL: query failed: %s\n", + PQresultErrorMessage(pg_result)); + This was suggested by a user: + */ + + *errmsg = string_sprintf("PGSQL: query failed: %s (%s) (%s)\n", + PQresultErrorMessage(pg_result), + PQresStatus(PQresultStatus(pg_result)), query); + goto PGSQL_EXIT; + } /* Result is in pg_result. Find the number of fields returned. If this is one, we don't add field names to the data. Otherwise we do. If the query did not @@ -329,7 +326,7 @@ for (i = 0; i < num_tuples; i++) result = string_catn(result, US"\n", 1); if (num_fields == 1) - result = string_catn(NULL, + result = string_catn(result, US PQgetvalue(pg_result, i, 0), PQgetlength(pg_result, i, 0)); else { @@ -342,17 +339,13 @@ for (i = 0; i < num_tuples; i++) } } -/* If result is NULL then no data has been found and so we return FAIL. -Otherwise, we must terminate the string which has been built; string_cat() -always leaves enough room for a terminating zero. */ +/* If result is NULL then no data has been found and so we return FAIL. */ if (!result) { yield = FAIL; *errmsg = US"PGSQL: no data found"; } -else - store_reset(result->s + result->ptr + 1); /* Get here by goto from various error checks. */ @@ -367,6 +360,7 @@ if (pg_result) PQclear(pg_result); if (result) { + store_reset(result->s + result->ptr + 1); *resultptr = string_from_gstring(result); return OK; } diff --git a/test/scripts/2620-Postgresql/2620 b/test/scripts/2620-Postgresql/2620 index 62ee31354..ba3a3bcc8 100644 --- a/test/scripts/2620-Postgresql/2620 +++ b/test/scripts/2620-Postgresql/2620 @@ -13,6 +13,7 @@ system 'createdb -h localhost -p PORT_N test'; system 'psql -h localhost -p PORT_N -d test \ -c "CREATE TABLE them ( name text, id text );" \ -c "INSERT INTO them VALUES ( \'Philip Hazel\', \'ph10\' );" \ + -c "INSERT INTO them VALUES ( \'Aristotle\', \'aaaa\' );" \ -c "INSERT INTO them VALUES ( \'\', \'nothing\' );" \ -c "INSERT INTO them VALUES ( \'\"stquot\', \'quote2\' );" \ -c "INSERT INTO them VALUES ( \'before\' || CHR(13) || CHR(10) || \'after\', \'newline\' );" \ @@ -37,6 +38,9 @@ ${lookup pgsql {servers=x:localhost; select name from them where id='ph10';}} ${lookup pgsql {servers=localhost::PORT_N:x; select name from them where id='ph10';}} ${lookup pgsql {servers=localhost::PORT_N/test/CALLER/:x; select name from them where id='ph10';}} ${lookup pgsql {servers=(DIR/pgsql/.s.PGSQL.PORT_N)/test/CALLER/:x; select name from them where id='ph10';}} +x +${lookup pgsql {SELECT name FROM them WHERE id IN ('ph10', 'aaaa');}} +${lookup pgsql {SELECT * FROM them WHERE id IN ('ph10', 'aaaa');}} **** exim -d -bh 10.0.0.0 mail from: diff --git a/test/stderr/2620 b/test/stderr/2620 index adc0c64a6..b0bed6209 100644 --- a/test/stderr/2620 +++ b/test/stderr/2620 @@ -159,6 +159,30 @@ database lookup required for servers=(TESTSUITE/pgsql/.s.PGSQL.1223)/test/CALLER PostgreSQL query: servers=(TESTSUITE/pgsql/.s.PGSQL.1223)/test/CALLER/:x; select name from them where id='ph10'; PGSQL new connection: socket=TESTSUITE/pgsql/.s.PGSQL.1223 database=test user=CALLER lookup yielded: Philip Hazel +search_open: pgsql "NULL" + cached open +search_find: file="NULL" + key="SELECT name FROM them WHERE id IN ('ph10', 'aaaa');" partial=-1 affix=NULL starflags=0 +LRU list: +internal_search_find: file="NULL" + type=pgsql key="SELECT name FROM them WHERE id IN ('ph10', 'aaaa');" +database lookup required for SELECT name FROM them WHERE id IN ('ph10', 'aaaa'); +PostgreSQL query: SELECT name FROM them WHERE id IN ('ph10', 'aaaa'); +PGSQL using cached connection for localhost:1223/test/CALLER +lookup yielded: Philip Hazel +Aristotle +search_open: pgsql "NULL" + cached open +search_find: file="NULL" + key="SELECT * FROM them WHERE id IN ('ph10', 'aaaa');" partial=-1 affix=NULL starflags=0 +LRU list: +internal_search_find: file="NULL" + type=pgsql key="SELECT * FROM them WHERE id IN ('ph10', 'aaaa');" +database lookup required for SELECT * FROM them WHERE id IN ('ph10', 'aaaa'); +PostgreSQL query: SELECT * FROM them WHERE id IN ('ph10', 'aaaa'); +PGSQL using cached connection for localhost:1223/test/CALLER +lookup yielded: name="Philip Hazel" id=ph10 +name=Aristotle id=aaaa search_tidyup called close PGSQL connection: (TESTSUITE/pgsql/.s.PGSQL.1223)/test/CALLER close PGSQL connection: localhost:1223/test/CALLER diff --git a/test/stdout/2620 b/test/stdout/2620 index c76e65085..668eca73a 100644 --- a/test/stdout/2620 +++ b/test/stdout/2620 @@ -28,6 +28,7 @@ INSERT 0 1 INSERT 0 1 INSERT 0 1 INSERT 0 1 +INSERT 0 1 > Philip Hazel > Philip Hazel > @@ -43,6 +44,11 @@ after" id=newline > Philip Hazel > Philip Hazel > Philip Hazel +> x +> Philip Hazel +Aristotle +> name="Philip Hazel" id=ph10 +name=Aristotle id=aaaa > **** SMTP testing session as if from host 10.0.0.0 -- 2.25.1