[Patches] PyObject_GetAttr/PyObject_SetAttr core dumps if given
non-string names
M.-A. Lemburg
mal@lemburg.com
Fri, 23 Jun 2000 10:04:48 +0200
Gisle Aas wrote:
>
> "M.-A. Lemburg" <mal@lemburg.com> writes:
>
> > > Index: Objects/object.c
> > > ===================================================================
> > > RCS file: /cvsroot/python/python/dist/src/Objects/object.c,v
> > > retrieving revision 2.71
> > > diff -u -p -u -r2.71 object.c
> > > --- Objects/object.c 2000/06/09 16:20:39 2.71
> > > +++ Objects/object.c 2000/06/22 19:27:51
> > > @@ -600,8 +600,12 @@ PyObject_GetAttr(v, name)
> > > {
> > > if (v->ob_type->tp_getattro != NULL)
> > > return (*v->ob_type->tp_getattro)(v, name);
> > > - else
> > > + else if (PyString_Check(name))
> > > return PyObject_GetAttrString(v, PyString_AsString(name));
> > > + else {
> > > + PyErr_SetString(PyExc_TypeError,
> > > + "attribute name must be string");
> > > + }
> > > }
> >
> > Hmm, I think it would be more generic to add a NULL
> > check to PyObject_GetAttrString() (PyString_AsString() returns
> > NULL in case name is not a string object).
>
> Ok. But it also set up the exception to be BadInternalCall() which
> does not feel right :-)
But that's what it is, right ? ;-)
> I also forgot to "return NULL" here.
>
> > > int
> > > @@ -626,12 +630,18 @@ PyObject_SetAttr(v, name, value)
> > > {
> > > int err;
> > > Py_INCREF(name);
> > > - PyString_InternInPlace(&name);
> > > if (v->ob_type->tp_setattro != NULL)
> > > err = (*v->ob_type->tp_setattro)(v, name, value);
> > > - else
> > > + else if (PyString_Check(name)) {
> > > + PyString_InternInPlace(&name);
> > > err = PyObject_SetAttrString(
> > > v, PyString_AsString(name), value);
> > > + }
> > > + else {
> > > + PyErr_SetString(PyExc_TypeError,
> > > + "attribute name must be string");
> > > + err = -1;
> > > + }
> >
> > Here, I think you ought to move the string test just before
> > the interning call.
>
> But you still want to be able to pass non-string objects to
> v->ob_type->tp_setattro I guess. Anyway, an alternative patch is
> included here.
>
> Thanks for the quick feedback!
>
> Regards,
> Gisle
>
> Index: Objects/object.c
> ===================================================================
> RCS file: /cvsroot/python/python/dist/src/Objects/object.c,v
> retrieving revision 2.71
> diff -u -p -u -r2.71 object.c
> --- Objects/object.c 2000/06/09 16:20:39 2.71
> +++ Objects/object.c 2000/06/22 20:37:21
> @@ -598,10 +598,17 @@ PyObject_GetAttr(v, name)
> PyObject *v;
> PyObject *name;
> {
> + char *name_str;
> if (v->ob_type->tp_getattro != NULL)
> return (*v->ob_type->tp_getattro)(v, name);
> - else
> - return PyObject_GetAttrString(v, PyString_AsString(name));
> +
> + name_str = PyString_AsString(name);
> + if (name_str == NULL) {
> + PyErr_SetString(PyExc_TypeError,
> + "attribute name must be string");
> + return NULL;
> + }
> + return PyObject_GetAttrString(v, name_str);
> }
This one looks ok. I would still be curious how the size
increase affects performance (you should note that the
above API is one of the more often used ones in Python).
I would guess that the addition of a variable makes inlining
harder -- so why not inline PyString_AsString() here:
if (!PyString_Check(name)) {
...error...
return NULL;
}
return PyObject_GetAttrString(v, PyString_AS_STRING(name));
> int
> @@ -625,13 +632,23 @@ PyObject_SetAttr(v, name, value)
> PyObject *value;
> {
> int err;
> + int name_is_string = 0;
> Py_INCREF(name);
> - PyString_InternInPlace(&name);
> + if (PyString_Check(name)) {
> + PyString_InternInPlace(&name);
> + name_is_string = 1;
> + }
> if (v->ob_type->tp_setattro != NULL)
> err = (*v->ob_type->tp_setattro)(v, name, value);
> - else
> + else if (name_is_string) {
It's better to avoid this variable and repeat the PyString_Check()
(the check only tests a pointer, so it doesn't cost much more).
This would also allow using PyString_AS_STRING() instead
of the function _AsString().
> err = PyObject_SetAttrString(
> v, PyString_AsString(name), value);
> + }
> + else {
> + PyErr_SetString(PyExc_TypeError,
> + "attribute name must be string");
> + err = -1;
> + }
> Py_DECREF(name);
> return err;
> }
>
> I confirm that, to the best of my knowledge and belief, this
> contribution is free of any claims of third parties under
> copyright, patent or other rights or interests ("claims"). To
> the extent that I have any such claims, I hereby grant to CNRI a
> nonexclusive, irrevocable, royalty-free, worldwide license to
> reproduce, distribute, perform and/or display publicly, prepare
> derivative versions, and otherwise use this contribution as part
> of the Python software and its related documentation, or any
> derivative versions thereof, at no cost to CNRI or its licensed
> users, and to authorize others to do so.
>
> I acknowledge that CNRI may, at its sole discretion, decide
> whether or not to incorporate this contribution in the Python
> software and its related documentation. I further grant CNRI
> permission to use my name and other identifying information
> provided to CNRI by me for use in connection with the Python
> software and its related documentation.
--
Marc-Andre Lemburg
______________________________________________________________________
Business: http://www.lemburg.com/
Python Pages: http://www.lemburg.com/python/