Re: libpq ssl -> clear fallback looses error messages
| От | Magnus Hagander |
|---|---|
| Тема | Re: libpq ssl -> clear fallback looses error messages |
| Дата | |
| Msg-id | 48FC3994.8040809@hagander.net обсуждение исходный текст |
| Ответ на | Re: libpq ssl -> clear fallback looses error messages (Tom Lane <tgl@sss.pgh.pa.us>) |
| Список | pgsql-hackers |
Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Here's an ugly attempt towards this. Though I'm unsure if we can change
>> the "const" on the PQerrorMessage parameter without messing with library
>> versions and such?
>
> That's a bad idea in any case --- PQerrorMessage shouldn't be changing
> the state of anything. The merging needs to happen earlier.
That was my general thought, but since I'd written it, I just threw it
out there..
>> Another option could be to change all the calls that set the error
>> message to *append* to the error message instead. Thoughts on that?
>
> That might work ... but we'd probably have to add some "clear the
> message" calls in places. It might be worth trying to restrict this
> convention to the connection-startup code.
How about something like this?
//Magnus
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 699,705 **** connectNoDelay(PGconn *conn)
{
char sebuf[256];
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not set socket to TCP no delay mode: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
return 0;
--- 699,705 ----
{
char sebuf[256];
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not set socket to TCP no delay mode: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
return 0;
***************
*** 729,735 **** connectFailureMessage(PGconn *conn, int errorno)
NULL, 0,
service, sizeof(service),
NI_NUMERICSERV);
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not connect to server: %s\n"
"\tIs the server running locally and accepting\n"
"\tconnections on Unix domain socket \"%s\"?\n"),
--- 729,735 ----
NULL, 0,
service, sizeof(service),
NI_NUMERICSERV);
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not connect to server: %s\n"
"\tIs the server running locally and accepting\n"
"\tconnections on Unix domain socket \"%s\"?\n"),
***************
*** 739,745 **** connectFailureMessage(PGconn *conn, int errorno)
else
#endif /* HAVE_UNIX_SOCKETS */
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not connect to server: %s\n"
"\tIs the server running on host \"%s\" and accepting\n"
"\tTCP/IP connections on port %s?\n"),
--- 739,745 ----
else
#endif /* HAVE_UNIX_SOCKETS */
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not connect to server: %s\n"
"\tIs the server running on host \"%s\" and accepting\n"
"\tTCP/IP connections on port %s?\n"),
***************
*** 829,839 **** connectDBStart(PGconn *conn)
if (ret || !addrs)
{
if (node)
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not translate host name \"%s\" to address: %s\n"),
node, gai_strerror(ret));
else
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not translate Unix-domain socket path \"%s\" to address: %s\n"),
portstr, gai_strerror(ret));
if (addrs)
--- 829,839 ----
if (ret || !addrs)
{
if (node)
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not translate host name \"%s\" to address: %s\n"),
node, gai_strerror(ret));
else
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not translate Unix-domain socket path \"%s\" to address: %s\n"),
portstr, gai_strerror(ret));
if (addrs)
***************
*** 924,929 **** connectDBComplete(PGconn *conn)
--- 924,931 ----
switch (flag)
{
case PGRES_POLLING_OK:
+ /* Reset stored error messages since we now have a working connection */
+ resetPQExpBuffer(&conn->errorMessage);
return 1; /* success! */
case PGRES_POLLING_READING:
***************
*** 1033,1039 **** PQconnectPoll(PGconn *conn)
break;
default:
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"invalid connection state, "
"probably indicative of memory corruption\n"
--- 1035,1041 ----
break;
default:
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"invalid connection state, "
"probably indicative of memory corruption\n"
***************
*** 1077,1083 **** keep_going: /* We will come back to here until there is
conn->addr_cur = addr_cur->ai_next;
continue;
}
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not create socket: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
break;
--- 1079,1085 ----
conn->addr_cur = addr_cur->ai_next;
continue;
}
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not create socket: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
break;
***************
*** 1100,1106 **** keep_going: /* We will come back to here until there is
}
if (!pg_set_noblock(conn->sock))
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not set socket to non-blocking mode: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
closesocket(conn->sock);
--- 1102,1108 ----
}
if (!pg_set_noblock(conn->sock))
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not set socket to non-blocking mode: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
closesocket(conn->sock);
***************
*** 1112,1118 **** keep_going: /* We will come back to here until there is
#ifdef F_SETFD
if (fcntl(conn->sock, F_SETFD, FD_CLOEXEC) == -1)
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not set socket to close-on-exec mode: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
closesocket(conn->sock);
--- 1114,1120 ----
#ifdef F_SETFD
if (fcntl(conn->sock, F_SETFD, FD_CLOEXEC) == -1)
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not set socket to close-on-exec mode: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
closesocket(conn->sock);
***************
*** 1199,1205 **** keep_going: /* We will come back to here until there is
if (getsockopt(conn->sock, SOL_SOCKET, SO_ERROR,
(char *) &optval, &optlen) == -1)
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not get socket error status: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
--- 1201,1207 ----
if (getsockopt(conn->sock, SOL_SOCKET, SO_ERROR,
(char *) &optval, &optlen) == -1)
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not get socket error status: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
***************
*** 1237,1243 **** keep_going: /* We will come back to here until there is
(struct sockaddr *) & conn->laddr.addr,
&conn->laddr.salen) < 0)
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not get client address from socket: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
--- 1239,1245 ----
(struct sockaddr *) & conn->laddr.addr,
&conn->laddr.salen) < 0)
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not get client address from socket: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
***************
*** 1281,1287 **** keep_going: /* We will come back to here until there is
pv = htonl(NEGOTIATE_SSL_CODE);
if (pqPacketSend(conn, 0, &pv, sizeof(pv)) != STATUS_OK)
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not send SSL negotiation packet: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
--- 1283,1289 ----
pv = htonl(NEGOTIATE_SSL_CODE);
if (pqPacketSend(conn, 0, &pv, sizeof(pv)) != STATUS_OK)
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not send SSL negotiation packet: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
goto error_return;
***************
*** 1303,1308 **** keep_going: /* We will come back to here until there is
--- 1305,1311 ----
EnvironmentOptions);
if (!startpacket)
{
+ /* will not appendbuffer here, since it's likely to also run out of memory */
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("out of memory\n"));
goto error_return;
***************
*** 1316,1322 **** keep_going: /* We will come back to here until there is
*/
if (pqPacketSend(conn, 0, startpacket, packetlen) != STATUS_OK)
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not send startup packet: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
free(startpacket);
--- 1319,1325 ----
*/
if (pqPacketSend(conn, 0, startpacket, packetlen) != STATUS_OK)
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not send startup packet: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
free(startpacket);
***************
*** 1381,1387 **** keep_going: /* We will come back to here until there is
if (conn->sslmode[0] == 'r') /* "require" */
{
/* Require SSL, but server does not want it */
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("server does not support SSL, but SSL was required\n"));
goto error_return;
}
--- 1384,1390 ----
if (conn->sslmode[0] == 'r') /* "require" */
{
/* Require SSL, but server does not want it */
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("server does not support SSL, but SSL was required\n"));
goto error_return;
}
***************
*** 1398,1404 **** keep_going: /* We will come back to here until there is
if (conn->sslmode[0] == 'r') /* "require" */
{
/* Require SSL, but server is too old */
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("server does not support SSL, but SSL was required\n"));
goto error_return;
}
--- 1401,1407 ----
if (conn->sslmode[0] == 'r') /* "require" */
{
/* Require SSL, but server is too old */
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("server does not support SSL, but SSL was required\n"));
goto error_return;
}
***************
*** 1414,1420 **** keep_going: /* We will come back to here until there is
}
else
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("received invalid response to SSL negotiation: %c\n"),
SSLok);
goto error_return;
--- 1417,1423 ----
}
else
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("received invalid response to SSL negotiation: %c\n"),
SSLok);
goto error_return;
***************
*** 1489,1495 **** keep_going: /* We will come back to here until there is
*/
if (!(beresp == 'R' || beresp == 'E'))
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"expected authentication request from "
"server, but received %c\n"),
--- 1492,1498 ----
*/
if (!(beresp == 'R' || beresp == 'E'))
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"expected authentication request from "
"server, but received %c\n"),
***************
*** 1522,1528 **** keep_going: /* We will come back to here until there is
*/
if (beresp == 'R' && (msgLength < 8 || msgLength > 2000))
{
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"expected authentication request from "
"server, but received %c\n"),
--- 1525,1531 ----
*/
if (beresp == 'R' && (msgLength < 8 || msgLength > 2000))
{
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"expected authentication request from "
"server, but received %c\n"),
***************
*** 1534,1540 **** keep_going: /* We will come back to here until there is
{
/* Handle error from a pre-3.0 server */
conn->inCursor = conn->inStart + 1; /* reread data */
! if (pqGets(&conn->errorMessage, conn))
{
/* We'll come back when there is more data */
return PGRES_POLLING_READING;
--- 1537,1543 ----
{
/* Handle error from a pre-3.0 server */
conn->inCursor = conn->inStart + 1; /* reread data */
! if (pqGets_append(&conn->errorMessage, conn))
{
/* We'll come back when there is more data */
return PGRES_POLLING_READING;
***************
*** 1601,1607 **** keep_going: /* We will come back to here until there is
}
else
{
! if (pqGets(&conn->errorMessage, conn))
{
/* We'll come back when there is more data */
return PGRES_POLLING_READING;
--- 1604,1610 ----
}
else
{
! if (pqGets_append(&conn->errorMessage, conn))
{
/* We'll come back when there is more data */
return PGRES_POLLING_READING;
***************
*** 1788,1794 **** keep_going: /* We will come back to here until there is
if (res)
{
if (res->resultStatus != PGRES_FATAL_ERROR)
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("unexpected message from server during startup\n"));
/*
--- 1791,1797 ----
if (res)
{
if (res->resultStatus != PGRES_FATAL_ERROR)
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("unexpected message from server during startup\n"));
/*
***************
*** 1855,1861 **** keep_going: /* We will come back to here until there is
return PGRES_POLLING_OK;
default:
! printfPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"invalid connection state %c, "
"probably indicative of memory corruption\n"
--- 1858,1864 ----
return PGRES_POLLING_OK;
default:
! appendPQExpBuffer(&conn->errorMessage,
libpq_gettext(
"invalid connection state %c, "
"probably indicative of memory corruption\n"
*** a/src/interfaces/libpq/fe-misc.c
--- b/src/interfaces/libpq/fe-misc.c
***************
*** 106,119 **** pqPutc(char c, PGconn *conn)
/*
! * pqGets:
* get a null-terminated string from the connection,
* and store it in an expansible PQExpBuffer.
* If we run out of memory, all of the string is still read,
* but the excess characters are silently discarded.
*/
! int
! pqGets(PQExpBuffer buf, PGconn *conn)
{
/* Copy conn data to locals for faster search loop */
char *inBuffer = conn->inBuffer;
--- 106,119 ----
/*
! * pqGets[_append]:
* get a null-terminated string from the connection,
* and store it in an expansible PQExpBuffer.
* If we run out of memory, all of the string is still read,
* but the excess characters are silently discarded.
*/
! static int
! pqGets_internal(PQExpBuffer buf, PGconn *conn, bool resetbuffer)
{
/* Copy conn data to locals for faster search loop */
char *inBuffer = conn->inBuffer;
***************
*** 129,135 **** pqGets(PQExpBuffer buf, PGconn *conn)
slen = inCursor - conn->inCursor;
! resetPQExpBuffer(buf);
appendBinaryPQExpBuffer(buf, inBuffer + conn->inCursor, slen);
conn->inCursor = ++inCursor;
--- 129,137 ----
slen = inCursor - conn->inCursor;
! if (resetbuffer)
! resetPQExpBuffer(buf);
!
appendBinaryPQExpBuffer(buf, inBuffer + conn->inCursor, slen);
conn->inCursor = ++inCursor;
***************
*** 141,146 **** pqGets(PQExpBuffer buf, PGconn *conn)
--- 143,160 ----
return 0;
}
+ int
+ pqGets(PQExpBuffer buf, PGconn *conn)
+ {
+ return pqGets_internal(buf, conn, true);
+ }
+
+ int
+ pqGets_append(PQExpBuffer buf, PGconn *conn)
+ {
+ return pqGets_internal(buf, conn, false);
+ }
+
/*
* pqPuts: write a null-terminated string to the current message
*** a/src/interfaces/libpq/fe-protocol3.c
--- b/src/interfaces/libpq/fe-protocol3.c
***************
*** 853,859 **** pqGetErrorNotice3(PGconn *conn, bool isError)
goto fail;
pqClearAsyncResult(conn);
conn->result = res;
- resetPQExpBuffer(&conn->errorMessage);
appendPQExpBufferStr(&conn->errorMessage, workBuf.data);
}
else
--- 853,858 ----
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 519,524 **** extern int pqCheckInBufferSpace(size_t bytes_needed, PGconn *conn);
--- 519,525 ----
extern int pqGetc(char *result, PGconn *conn);
extern int pqPutc(char c, PGconn *conn);
extern int pqGets(PQExpBuffer buf, PGconn *conn);
+ extern int pqGets_append(PQExpBuffer buf, PGconn *conn);
extern int pqPuts(const char *s, PGconn *conn);
extern int pqGetnchar(char *s, size_t len, PGconn *conn);
extern int pqPutnchar(const char *s, size_t len, PGconn *conn);
В списке pgsql-hackers по дате отправления: