libpq++ tracing considered harmful (was Re: libpq++ memory problems)

Поиск
Список
Период
Сортировка
От Tom Lane
Тема libpq++ tracing considered harmful (was Re: libpq++ memory problems)
Дата
Msg-id 8163.956252492@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: libpq++ memory problems  (Lamar Owen <lamar.owen@wgcr.org>)
Список pgsql-interfaces
Ah-hah, figured it out.  There *was* a recent relevant change, but the
log message for it didn't say anything about leaks.  The problem is
in the "debug" code for PgConnection, which in 6.5.* looks like:

// PgConnection::connect
// establish a connection to a backend
ConnStatusType PgConnection::Connect(const char* conninfo)
{
ConnStatusType cst; // Turn the trace on
#if defined(DEBUG) FILE *debug = fopen("/tmp/trace.out","w"); PQtrace(pgConn, debug);
#endif  // Connect to the database pgConn = PQconnectdb(conninfo);

This is busted in at least five ways:

1. "DEBUG" is a symbol defined in the backend header files (it's a  severity level constant for elog()).  So although
libpq++'sMakefile  thinks it can turn the code on or off via -DDEBUG, it's mistaken;  that debug-tracing code always
getscompiled.  Oliver Elphick fixed that a month or so ago by changing the  controlling symbol to "DEBUGFILE".  So
that'swhy you see the leak  in 6.5.3 and I don't see it in current sources.
 

2. The fopen'd file is never closed.  (The destructor calls PQuntrace  but that routine isn't responsible for closing
thefile.)  That's  the direct cause of the memory and file descriptor leak you see:  a stdio file is fopen'd and never
fclose'dfor each PgConnection  created.
 

3. The whole thing is a complete waste of time because the pgConn  object doesn't exist yet --- it's not created till
thenext line!  So PQtrace is being handed either a null pgConn pointer (which I  believe it will silently ignore) or a
garbagepointer (which could  have who-knows-what unpleasant consequences).  But in no case will  any tracing actually
occur. Sheesh.
 

4. If any tracing did occur, all of the output from (perhaps many)  different PgConnection objects in different program
runswould all  get dumped into the same temp file, leaving it of doubtful use to  anybody.
 

5. One could reasonably doubt that it's a good idea to have a compiled-in  option to dump the entire trace of a
program'sinteraction with the  backend into a publicly readable temp file.  Can you say "security  hole"?  Seems to me
thatthis function should require a specific  request from the application, not be something that could accidentally
getinstalled as the default behavior of a system library.  (Think  what would happen if RPMs containing such behavior
gotdistributed...)
 

Perhaps something can be salvaged from the wreckage, but for now the
right answer is just to make sure that this code is not compiled.
        regards, tom lane


В списке pgsql-interfaces по дате отправления:

Предыдущее
От: "Ken J. Wright"
Дата:
Сообщение: Re: Inserting NULL with pgaccess
Следующее
От: Lamar Owen
Дата:
Сообщение: Re: libpq++ tracing considered harmful (was Re: libpq++ memory problems)