From 8143ff45b2b9edd3532fe4056d5736ecc4e0a766 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 24 Dec 2017 21:30:20 +0000 Subject: [PATCH 1/1] 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 | 7 + src/src/lookups/pgsql.c | 86 +++++----- test/confs/{9200 => 2620} | 4 +- test/log/{9200 => 2620} | 0 test/mail/{9200.CALLER => 2620.ph10} | 0 test/runtest | 1 + test/scripts/2620-Postgresql/2620 | 63 +++++++ test/scripts/2620-Postgresql/REQUIRES | 1 + test/scripts/9200-PostgreSQL/9200 | 30 ---- test/stderr/{9200 => 2620} | 227 ++++++++++++++++---------- test/stdout/2620 | 66 ++++++++ test/stdout/9200 | 28 ---- 12 files changed, 323 insertions(+), 190 deletions(-) rename test/confs/{9200 => 2620} (92%) rename test/log/{9200 => 2620} (100%) rename test/mail/{9200.CALLER => 2620.ph10} (100%) create mode 100644 test/scripts/2620-Postgresql/2620 create mode 100644 test/scripts/2620-Postgresql/REQUIRES delete mode 100644 test/scripts/9200-PostgreSQL/9200 rename test/stderr/{9200 => 2620} (60%) create mode 100644 test/stdout/2620 delete mode 100644 test/stdout/9200 diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 824ae232e..08ed2ce9d 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -5,6 +5,13 @@ affect Exim's operation, with an unchanged configuration file. For new options, and new features, see the NewStuff file next to this ChangeLog. +Since Exim version 4.90 +----------------- + +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/confs/9200 b/test/confs/2620 similarity index 92% rename from test/confs/9200 rename to test/confs/2620 index 04f52a9a0..009e74fd8 100644 --- a/test/confs/9200 +++ b/test/confs/2620 @@ -1,6 +1,6 @@ -# Exim test configuration 9200 +# Exim test configuration 2620 -SERVERS=localhost/test/ph10/ +SERVERS=localhost::PORT_N/test/CALLER/ .include DIR/aux-var/std_conf_prefix diff --git a/test/log/9200 b/test/log/2620 similarity index 100% rename from test/log/9200 rename to test/log/2620 diff --git a/test/mail/9200.CALLER b/test/mail/2620.ph10 similarity index 100% rename from test/mail/9200.CALLER rename to test/mail/2620.ph10 diff --git a/test/runtest b/test/runtest index a1259f769..d0de8622e 100755 --- a/test/runtest +++ b/test/runtest @@ -2428,6 +2428,7 @@ elsif (/^background$/) $_ =