Skip to content

Commit c74929a

Browse files
author
James William Pye
committed
Cleanup protocol.optimized.
Give more "useful" error messages are Tuple parse failures.
1 parent 8bc3d62 commit c74929a

2 files changed

Lines changed: 43 additions & 16 deletions

File tree

‎postgresql/protocol/optimized.c‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,21 @@
1818
static PyObject *
1919
parse_tuple_message(PyObject *self, PyObject *args)
2020
{
21-
PyObject *typ;
22-
PyObject *ob;
2321
PyObject *rob;
22+
PyObject *ob;
23+
PyObject *typ;
2424
const char *data;
25+
Py_ssize_t dlen = 0;
2526
uint16_t cnatt = 0, natts = 0;
2627
uint32_t attsize = 0;
2728
uint32_t position = 0;
28-
Py_ssize_t dlen = 0;
2929

30-
if (PyArg_ParseTuple(args, "Oy#", &typ, &data, &dlen) < 0)
30+
if (!PyArg_ParseTuple(args, "Oy#", &typ, &data, &dlen))
3131
return(NULL);
3232

33+
/*
34+
* Validate that the given "typ" is in fact a PyTuple_Type subtype.
35+
*/
3336
if (typ != Py_None)
3437
{
3538
if (!PyObject_IsSubclass(typ, (PyObject *) &PyTuple_Type))
@@ -67,9 +70,11 @@ parse_tuple_message(PyObject *self, PyObject *args)
6770
/*
6871
* FEARME: A bit much for saving a reallocation/copy?
6972
*
70-
* This is expected to be used as a classmethod on a tuple subtype.
73+
* This is expected to be used as a classmethod on a tuple subtype that
74+
* has *no* additional attributes.
75+
*
7176
* If the subtype has a custom __new__ routine, this could
72-
* be problematic, but it would probably only lead to AttributeErrors
77+
* be problematic, but it *should* only lead to AttributeErrors.
7378
*/
7479
rob = ((PyTypeObject *) typ)->tp_alloc((PyTypeObject *) typ, natts);
7580
if (rob == NULL)
@@ -86,7 +91,10 @@ parse_tuple_message(PyObject *self, PyObject *args)
8691
if (position + 4 > dlen)
8792
{
8893
PyErr_Format(PyExc_ValueError,
89-
"not enough data for attribute %d", cnatt);
94+
"not enough data available for attribute %d's size header: "
95+
"needed %d bytes, but only %lu remain at position %lu",
96+
cnatt, 4, dlen - position, position
97+
);
9098
goto cleanup;
9199
}
92100

@@ -110,21 +118,23 @@ parse_tuple_message(PyObject *self, PyObject *args)
110118
* so it is unexpected for an attsize to cause wrap-around.
111119
*/
112120
PyErr_Format(PyExc_ValueError,
113-
"tuple data caused position (uint32_t) wrap-around on attribute %d",
114-
cnatt
121+
"tuple data caused position (uint32_t) "
122+
"to wrap on attribute %d, position %lu + size %lu",
123+
cnatt, position, attsize
115124
);
116125
goto cleanup;
117126
}
118127

119128
if (position + attsize > dlen)
120129
{
121130
PyErr_Format(PyExc_ValueError,
122-
"not enough data for attribute %d, size %d, "
123-
"but only %d remaining bytes in message",
131+
"not enough data for attribute %d, size %lu, "
132+
"as only %lu bytes remain in message",
124133
cnatt, attsize, dlen - position
125134
);
126135
goto cleanup;
127136
}
137+
128138
ob = PyBytes_FromStringAndSize(data + position, attsize);
129139
if (ob == NULL)
130140
{
@@ -143,7 +153,7 @@ parse_tuple_message(PyObject *self, PyObject *args)
143153
if (position != dlen)
144154
{
145155
PyErr_Format(PyExc_ValueError,
146-
"invalid tuple message, %d remaining "
156+
"invalid tuple message, %lu remaining "
147157
"bytes after processing %d attributes",
148158
dlen - position, cnatt
149159
);
@@ -157,6 +167,10 @@ parse_tuple_message(PyObject *self, PyObject *args)
157167
return(NULL);
158168
}
159169

170+
/*
171+
* process the tuple with the associated callables while
172+
* calling the third object in cases of failure to generalize the exception.
173+
*/
160174
static PyObject *
161175
process_tuple(PyObject *self, PyObject *args)
162176
{
@@ -249,11 +263,13 @@ process_tuple(PyObject *self, PyObject *args)
249263
failargs = PyTuple_New(3);
250264
if (failargs != NULL)
251265
{
266+
/* args for the exception "handler" */
252267
PyTuple_SET_ITEM(failargs, 0, procs);
253268
Py_INCREF(procs);
254269
PyTuple_SET_ITEM(failargs, 1, tup);
255270
Py_INCREF(tup);
256271
PyTuple_SET_ITEM(failargs, 2, failedat);
272+
257273
r = PyObject_CallObject(fail, failargs);
258274
Py_DECREF(failargs);
259275
if (r != NULL)

‎postgresql/test/test_protocol.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -735,23 +735,34 @@ def testConsistency(self):
735735
class test_optimized(unittest.TestCase):
736736
def test_parse_tuple_message(self):
737737
ptm = protocol_optimized.parse_tuple_message
738+
self.failUnlessRaises(TypeError, ptm, tuple, "stringzor")
739+
self.failUnlessRaises(TypeError, ptm, tuple, 123)
738740
self.failUnlessRaises(ValueError, ptm, tuple, b'')
739741
self.failUnlessRaises(ValueError, ptm, tuple, b'0')
740742

741743
notenoughdata = struct.pack('!H', 2)
742744
self.failUnlessRaises(ValueError, ptm, tuple, notenoughdata)
743745

744-
wraparound = \
745-
struct.pack('!HL', 2, 10) + (b'0' * 10) + struct.pack('!L', 0xFFFFFFFE)
746+
wraparound = struct.pack('!HL', 2, 10) + (b'0' * 10) + struct.pack('!L', 0xFFFFFFFE)
746747
self.failUnlessRaises(ValueError, ptm, tuple, wraparound)
747748

748-
oneatt_notenough = \
749-
struct.pack('!HL', 2, 10) + (b'0' * 10) + struct.pack('!L', 15)
749+
oneatt_notenough = struct.pack('!HL', 2, 10) + (b'0' * 10) + struct.pack('!L', 15)
750750
self.failUnlessRaises(ValueError, ptm, tuple, oneatt_notenough)
751751

752752
toomuchdata = struct.pack('!HL', 1, 3) + (b'0' * 10)
753753
self.failUnlessRaises(ValueError, ptm, tuple, toomuchdata)
754754

755+
class faketup(tuple):
756+
def __new__(subtype, geeze):
757+
r = tuple.__new__(subtype, ())
758+
r.foo = geeze
759+
return r
760+
zerodata = struct.pack('!H', 0)
761+
r = ptm(tuple, zerodata)
762+
self.failUnlessRaises(AttributeError, getattr, r, 'foo')
763+
self.failUnlessRaises(AttributeError, setattr, r, 'foo', 'bar')
764+
self.failUnlessEqual(len(r), 0)
765+
755766
def test_process_tuple(self):
756767
def funpass(procs, tup, col):
757768
pass

0 commit comments

Comments
 (0)