From 1db9c9b8ce99bdc863d232cf06301edb53e2ed5b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 23 Feb 2011 01:53:25 +0000 Subject: [PATCH] The cursor name can be a non-valid PostgreSQL identifier --- NEWS | 1 + doc/src/connection.rst | 21 +++++++++++++++++---- psycopg/cursor_type.c | 22 ++++++++++------------ psycopg/psycopg.h | 2 +- psycopg/utils.c | 37 +++++++++++++++++++++++++++++++++++++ tests/test_cursor.py | 10 ++++++++++ 6 files changed, 76 insertions(+), 17 deletions(-) diff --git a/NEWS b/NEWS index 08d377cb..c0c60478 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,7 @@ What's new in psycopg 2.4 into Python tuples/namedtuples. - More efficient iteration on named cursors, fetching 'itersize' records at time from the backend. + - The named cursors name can be an invalid identifier. - 'cursor.description' is provided in named tuples if available. - Connections and cursors are weakly referenceable. - Added 'b' and 't' mode to large objects: write can deal with both bytes diff --git a/doc/src/connection.rst b/doc/src/connection.rst index bb46a49f..01032587 100644 --- a/doc/src/connection.rst +++ b/doc/src/connection.rst @@ -25,11 +25,24 @@ The ``connection`` class Return a new `cursor` object using the connection. - If `name` is specified, the returned cursor will be a *server - side* (or *named*) cursor. Otherwise the cursor will be *client side*. - See :ref:`server-side-cursors` for further details. + If *name* is specified, the returned cursor will be a :ref:`server + side cursor ` (also known as *named cursor*). + Otherwise it will be a regular *client side* cursor. - The `cursor_factory` argument can be used to create non-standard + The name can be a string not valid as a PostgreSQL identifier: for + example it may start with a digit and contain non-alphanumeric + characters and quotes. + + .. versionchanged:: 2.4 + previously only valid PostgreSQL identifiers were accepted as + cursor name. + + .. warning:: + It is unsafe to expose the *name* to an untrusted source, for + instance you shouldn't allow *name* to be read from a HTML form. + Consider it as part of the query, not as a query parameter. + + The *cursor_factory* argument can be used to create non-standard cursors. The class returned should be a subclass of `psycopg2.extensions.cursor`. See :ref:`subclassing-cursor` for details. diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 4eee0d5d..d166de73 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -59,7 +59,7 @@ psyco_curs_close(cursorObject *self, PyObject *args) char buffer[128]; EXC_IF_NO_MARK(self); - PyOS_snprintf(buffer, 127, "CLOSE %s", self->name); + PyOS_snprintf(buffer, 127, "CLOSE \"%s\"", self->name); if (pq_execute(self, buffer, 0) == -1) return NULL; } @@ -391,7 +391,7 @@ _psyco_curs_execute(cursorObject *self, if (self->name != NULL) { self->query = Bytes_FromFormat( - "DECLARE %s CURSOR WITHOUT HOLD FOR %s", + "DECLARE \"%s\" CURSOR WITHOUT HOLD FOR %s", self->name, Bytes_AS_STRING(fquery)); Py_DECREF(fquery); } @@ -402,7 +402,7 @@ _psyco_curs_execute(cursorObject *self, else { if (self->name != NULL) { self->query = Bytes_FromFormat( - "DECLARE %s CURSOR WITHOUT HOLD FOR %s", + "DECLARE \"%s\" CURSOR WITHOUT HOLD FOR %s", self->name, Bytes_AS_STRING(operation)); } else { @@ -748,7 +748,7 @@ psyco_curs_fetchone(cursorObject *self, PyObject *args) EXC_IF_NO_MARK(self); EXC_IF_TPC_PREPARED(self->conn, fetchone); - PyOS_snprintf(buffer, 127, "FETCH FORWARD 1 FROM %s", self->name); + PyOS_snprintf(buffer, 127, "FETCH FORWARD 1 FROM \"%s\"", self->name); if (pq_execute(self, buffer, 0) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; } @@ -802,7 +802,7 @@ psyco_curs_next_named(cursorObject *self) if (self->row >= self->rowcount) { char buffer[128]; - PyOS_snprintf(buffer, 127, "FETCH FORWARD %ld FROM %s", + PyOS_snprintf(buffer, 127, "FETCH FORWARD %ld FROM \"%s\"", self->itersize, self->name); if (pq_execute(self, buffer, 0) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; @@ -862,7 +862,7 @@ psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) EXC_IF_NO_MARK(self); EXC_IF_TPC_PREPARED(self->conn, fetchone); - PyOS_snprintf(buffer, 127, "FETCH FORWARD %d FROM %s", + PyOS_snprintf(buffer, 127, "FETCH FORWARD %d FROM \"%s\"", (int)size, self->name); if (pq_execute(self, buffer, 0) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; @@ -933,7 +933,7 @@ psyco_curs_fetchall(cursorObject *self, PyObject *args) EXC_IF_NO_MARK(self); EXC_IF_TPC_PREPARED(self->conn, fetchall); - PyOS_snprintf(buffer, 127, "FETCH FORWARD ALL FROM %s", self->name); + PyOS_snprintf(buffer, 127, "FETCH FORWARD ALL FROM \"%s\"", self->name); if (pq_execute(self, buffer, 0) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; } @@ -1144,11 +1144,11 @@ psyco_curs_scroll(cursorObject *self, PyObject *args, PyObject *kwargs) EXC_IF_TPC_PREPARED(self->conn, scroll); if (strcmp(mode, "absolute") == 0) { - PyOS_snprintf(buffer, 127, "MOVE ABSOLUTE %d FROM %s", + PyOS_snprintf(buffer, 127, "MOVE ABSOLUTE %d FROM \"%s\"", value, self->name); } else { - PyOS_snprintf(buffer, 127, "MOVE %d FROM %s", value, self->name); + PyOS_snprintf(buffer, 127, "MOVE %d FROM \"%s\"", value, self->name); } if (pq_execute(self, buffer, 0) == -1) return NULL; if (_psyco_curs_prefetch(self) < 0) return NULL; @@ -1666,11 +1666,9 @@ cursor_setup(cursorObject *self, connectionObject *conn, const char *name) Dprintf("cursor_setup: parameters: name = %s, conn = %p", name, conn); if (name) { - if (!(self->name = PyMem_Malloc(strlen(name)+1))) { - PyErr_NoMemory(); + if (!(self->name = psycopg_escape_identifier_easy(name, 0))) { return 1; } - strncpy(self->name, name, strlen(name)+1); } /* FIXME: why does this raise an excpetion on the _next_ line of code? diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index d40a2399..2f06c378 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -125,7 +125,7 @@ HIDDEN void psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, HIDDEN char *psycopg_escape_string(PyObject *conn, const char *from, Py_ssize_t len, char *to, Py_ssize_t *tolen); - +HIDDEN char *psycopg_escape_identifier_easy(const char *from, Py_ssize_t len); HIDDEN char *psycopg_strdup(const char *from, Py_ssize_t len); HIDDEN PyObject * psycopg_ensure_bytes(PyObject *obj); HIDDEN PyObject * psycopg_ensure_text(PyObject *obj); diff --git a/psycopg/utils.c b/psycopg/utils.c index 4e5c83ed..2e81c113 100644 --- a/psycopg/utils.c +++ b/psycopg/utils.c @@ -71,6 +71,43 @@ psycopg_escape_string(PyObject *obj, const char *from, Py_ssize_t len, return to; } +/* Escape a string to build a valid PostgreSQL identifier + * + * Allocate a new buffer on the Python heap containing the new string. + * 'len' is optional: if 0 the length is calculated. + * + * The returned string doesn't include quotes. + * + * WARNING: this function is not so safe to allow untrusted input: it does no + * check for multibyte chars. Such a function should be built on + * PQescapeIndentifier, which is only available from PostgreSQL 9.0. + */ +char * +psycopg_escape_identifier_easy(const char *from, Py_ssize_t len) +{ + char *rv; + const char *src; + char *dst; + + if (!len) { len = strlen(from); } + if (!(rv = PyMem_New(char, 1 + 2 * len))) { + PyErr_NoMemory(); + return NULL; + } + + /* The only thing to do is double quotes */ + for (src = from, dst = rv; *src; ++src, ++dst) { + *dst = *src; + if ('"' == *src) { + *++dst = '"'; + } + } + + *dst = '\0'; + + return rv; +} + /* Duplicate a string. * * Allocate a new buffer on the Python heap containing the new string. diff --git a/tests/test_cursor.py b/tests/test_cursor.py index 74f86a4d..af5a9d37 100755 --- a/tests/test_cursor.py +++ b/tests/test_cursor.py @@ -141,6 +141,16 @@ class CursorTests(unittest.TestCase): del curs self.assert_(w() is None) + def test_invalid_name(self): + curs = self.conn.cursor() + curs.execute("create temp table invname (data int);") + curs.execute("insert into invname values (10), (20), (30)") + curs.close() + + curs = self.conn.cursor(r'1-2-3 \ "test"') + curs.execute("select data from invname order by data") + self.assertEqual(curs.fetchall(), [(10,), (20,), (30,)]) + @skip_before_postgres(8, 2) def test_iter_named_cursor_efficient(self): curs = self.conn.cursor('tmp')