Obscure the dsn password before storing it into the connectionobscure-password-before-connect

This avoids the need to juggle with exceptions in order to scrub the
password after a connection error, which may also swallow signals
(see #898).

3 files changed, 38 insertions, 42 deletions

diff --git a/psycopg/connection.h b/psycopg/connection.h
index acf4e45..3d94dbf 100644
--- a/psycopg/connection.h
+++ b/psycopg/connection.h
@@ -164,7 +164,7 @@ HIDDEN void conn_notice_process(connectionObject *self);
 HIDDEN void conn_notice_clean(connectionObject *self);
 HIDDEN void conn_notifies_process(connectionObject *self);
 RAISES_NEG HIDDEN int conn_setup(connectionObject *self);
-HIDDEN int  conn_connect(connectionObject *self, long int async);
+HIDDEN int  conn_connect(connectionObject *self, const char *dsn, long int async);
 HIDDEN void conn_close(connectionObject *self);
 HIDDEN void conn_close_locked(connectionObject *self);
 RAISES_NEG HIDDEN int  conn_commit(connectionObject *self);
diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c
index cdc6f02..8d3b06a 100644
--- a/psycopg/connection_int.c
+++ b/psycopg/connection_int.c
@@ -705,7 +705,7 @@ exit:
 /* conn_connect - execute a connection to the database */
 
 static int
-_conn_sync_connect(connectionObject *self)
+_conn_sync_connect(connectionObject *self, const char *dsn)
 {
     int green;
 
@@ -714,26 +714,26 @@ _conn_sync_connect(connectionObject *self)
     green = psyco_green();
     if (!green) {
         Py_BEGIN_ALLOW_THREADS;
-        self->pgconn = PQconnectdb(self->dsn);
+        self->pgconn = PQconnectdb(dsn);
         Py_END_ALLOW_THREADS;
         Dprintf("conn_connect: new PG connection at %p", self->pgconn);
     }
     else {
         Py_BEGIN_ALLOW_THREADS;
-        self->pgconn = PQconnectStart(self->dsn);
+        self->pgconn = PQconnectStart(dsn);
         Py_END_ALLOW_THREADS;
         Dprintf("conn_connect: new green PG connection at %p", self->pgconn);
     }
 
     if (!self->pgconn)
     {
-        Dprintf("conn_connect: PQconnectdb(%s) FAILED", self->dsn);
+        Dprintf("conn_connect: PQconnectdb(%s) FAILED", dsn);
         PyErr_SetString(OperationalError, "PQconnectdb() failed");
         return -1;
     }
     else if (PQstatus(self->pgconn) == CONNECTION_BAD)
     {
-        Dprintf("conn_connect: PQconnectdb(%s) returned BAD", self->dsn);
+        Dprintf("conn_connect: PQconnectdb(%s) returned BAD", dsn);
         PyErr_SetString(OperationalError, PQerrorMessage(self->pgconn));
         return -1;
     }
@@ -763,23 +763,23 @@ _conn_sync_connect(connectionObject *self)
 }
 
 static int
-_conn_async_connect(connectionObject *self)
+_conn_async_connect(connectionObject *self, const char *dsn)
 {
     PGconn *pgconn;
 
-    self->pgconn = pgconn = PQconnectStart(self->dsn);
+    self->pgconn = pgconn = PQconnectStart(dsn);
 
     Dprintf("conn_connect: new postgresql connection at %p", pgconn);
 
     if (pgconn == NULL)
     {
-        Dprintf("conn_connect: PQconnectStart(%s) FAILED", self->dsn);
+        Dprintf("conn_connect: PQconnectStart(%s) FAILED", dsn);
         PyErr_SetString(OperationalError, "PQconnectStart() failed");
         return -1;
     }
     else if (PQstatus(pgconn) == CONNECTION_BAD)
     {
-        Dprintf("conn_connect: PQconnectdb(%s) returned BAD", self->dsn);
+        Dprintf("conn_connect: PQconnectdb(%s) returned BAD", dsn);
         PyErr_SetString(OperationalError, PQerrorMessage(pgconn));
         return -1;
     }
@@ -800,17 +800,17 @@ _conn_async_connect(connectionObject *self)
 }
 
 int
-conn_connect(connectionObject *self, long int async)
+conn_connect(connectionObject *self, const char *dsn, long int async)
 {
     int rv;
 
     if (async == 1) {
       Dprintf("con_connect: connecting in ASYNC mode");
-      rv = _conn_async_connect(self);
+      rv = _conn_async_connect(self, dsn);
     }
     else {
       Dprintf("con_connect: connecting in SYNC mode");
-      rv = _conn_sync_connect(self);
+      rv = _conn_sync_connect(self, dsn);
     }
 
     if (rv != 0) {
diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c
index f472c4b..99ef6ea 100644
--- a/psycopg/connection_type.c
+++ b/psycopg/connection_type.c
@@ -1297,21 +1297,28 @@ static struct PyGetSetDef connectionObject_getsets[] = {
 
 /* initialization and finalization methods */
 
-RAISES_NEG static int
-obscure_password(connectionObject *conn)
+/* Return a copy of the 'dsn' string with the password scrubbed.
+ *
+ * The string returned is allocated on the Python heap.
+ *
+ * In case of error return NULL and raise an exception.
+ */
+static char *
+obscure_password(const char *dsn)
 {
-    PQconninfoOption *options;
-    PyObject *d = NULL, *v = NULL, *dsn = NULL;
-    char *tmp;
-    int rv = -1;
+    PQconninfoOption *options = NULL;
+    PyObject *d = NULL, *v = NULL, *pydsn = NULL;
+    char *rv = NULL;
 
-    if (!conn || !conn->dsn) {
-        return 0;
+    if (!dsn) {
+        PyErr_SetString(InternalError, "unexpected null string");
+        goto exit;
     }
 
-    if (!(options = PQconninfoParse(conn->dsn, NULL))) {
+    if (!(options = PQconninfoParse(dsn, NULL))) {
         /* unlikely: the dsn was already tested valid */
-        return 0;
+        PyErr_SetString(InternalError, "the connection string is not valid");
+        goto exit;
     }
 
     if (!(d = psyco_dict_from_conninfo_options(
@@ -1320,28 +1327,24 @@ obscure_password(connectionObject *conn)
     }
     if (NULL == PyDict_GetItemString(d, "password")) {
         /* the dsn doesn't have a password */
-        rv = 0;
+        psyco_strdup(&rv, dsn, -1);
         goto exit;
     }
 
     /* scrub the password and put back the connection string together */
     if (!(v = Text_FromUTF8("xxx"))) { goto exit; }
     if (0 > PyDict_SetItemString(d, "password", v)) { goto exit; }
-    if (!(dsn = psyco_make_dsn(Py_None, d))) { goto exit; }
-    if (!(dsn = psyco_ensure_bytes(dsn))) { goto exit; }
-
-    /* Replace the connection string on the connection object */
-    tmp = conn->dsn;
-    psyco_strdup(&conn->dsn, Bytes_AS_STRING(dsn), -1);
-    PyMem_Free(tmp);
+    if (!(pydsn = psyco_make_dsn(Py_None, d))) { goto exit; }
+    if (!(pydsn = psyco_ensure_bytes(pydsn))) { goto exit; }
 
-    rv = 0;
+    /* Return the connection string with the password replaced */
+    psyco_strdup(&rv, Bytes_AS_STRING(pydsn), -1);
 
 exit:
     PQconninfoFree(options);
     Py_XDECREF(v);
     Py_XDECREF(d);
-    Py_XDECREF(dsn);
+    Py_XDECREF(pydsn);
 
     return rv;
 }
@@ -1356,7 +1359,7 @@ connection_setup(connectionObject *self, const char *dsn, long int async)
             self, async, Py_REFCNT(self)
       );
 
-    if (0 > psyco_strdup(&self->dsn, dsn, -1)) { goto exit; }
+    if (!(self->dsn = obscure_password(dsn))) { goto exit; }
     if (!(self->notice_list = PyList_New(0))) { goto exit; }
     if (!(self->notifies = PyList_New(0))) { goto exit; }
     self->async = async;
@@ -1378,7 +1381,7 @@ connection_setup(connectionObject *self, const char *dsn, long int async)
         goto exit;
     }
 
-    if (conn_connect(self, async) != 0) {
+    if (conn_connect(self, dsn, async) != 0) {
         Dprintf("connection_init: FAILED");
         goto exit;
     }
@@ -1390,13 +1393,6 @@ connection_setup(connectionObject *self, const char *dsn, long int async)
         self, Py_REFCNT(self));
 
 exit:
-    /* here we obfuscate the password even if there was a connection error */
-    {
-        PyObject *ptype = NULL, *pvalue = NULL, *ptb = NULL;
-        PyErr_Fetch(&ptype, &pvalue, &ptb);
-        obscure_password(self);
-        PyErr_Restore(ptype, pvalue, ptb);
-    }
     return rv;
 }