Skip to content

Commit c67c3e2

Browse files
committed
pythongh-127081: use re-entrant variants of get{proto,serv}by{name,port}
Add configure tests and defines for getservbyname_r, getservbyport_r, and getprotobyname_r. Use these if available, otherwise fallback to the thread-unsafe variants. Add a unit test to exercise getprotobyname, which is currently untested. TODO: - Are there any platforms which define the unsafe variants but not the re-entrant ones? If not we can simplify the #ifdef hell somewhat. - Do the re-entrant functions have the same signature on all platforms? - These changes follow the existing code's practice: allocate a fixed-size (and overly large) buffer, and don't properly handle the error case if it is too small. Should this be fixed? If so should existing code also be fixed?
1 parent ccad61e commit c67c3e2

File tree

6 files changed

+177
-6
lines changed

6 files changed

+177
-6
lines changed

Lib/test/test_socket.py

+4
Original file line numberDiff line numberDiff line change
@@ -1297,6 +1297,10 @@ def testGetServBy(self):
12971297
self.assertRaises(OverflowError, socket.getservbyport, -1)
12981298
self.assertRaises(OverflowError, socket.getservbyport, 65536)
12991299

1300+
def testGetProtoByName(self):
1301+
self.assertEqual(socket.getprotobyname('tcp'), 6)
1302+
self.assertRaises(OSError, socket.getprotobyname, 'non-existent proto')
1303+
13001304
def testDefaultTimeout(self):
13011305
# Testing default timeout
13021306
# The default timeout should initially be None
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix thread safety issues with getservbyname, getservbyport, and
2+
getprotobyname.

Modules/socketmodule.c

+36-6
Original file line numberDiff line numberDiff line change
@@ -6269,7 +6269,7 @@ Return the true host name, a list of aliases, and a list of IP addresses,\n\
62696269
for a host. The host argument is a string giving a host name or IP number.");
62706270
#endif
62716271

6272-
#ifdef HAVE_GETSERVBYNAME
6272+
#if defined(HAVE_GETSERVBYNAME_R) || defined (HAVE_GETSERVBYNAME)
62736273
/* Python interface to getservbyname(name).
62746274
This only returns the port number, since the other info is already
62756275
known or not useful (like the list of aliases). */
@@ -6279,6 +6279,12 @@ static PyObject *
62796279
socket_getservbyname(PyObject *self, PyObject *args)
62806280
{
62816281
const char *name, *proto=NULL;
6282+
#ifdef HAVE_GETSERVBYNAME_R
6283+
struct servent entry;
6284+
/* TODO: The man page says 1024 is usually enough, start with that and
6285+
retry if insufficient? */
6286+
char buf[16384];
6287+
#endif
62826288
struct servent *sp;
62836289
if (!PyArg_ParseTuple(args, "s|s:getservbyname", &name, &proto))
62846290
return NULL;
@@ -6288,7 +6294,11 @@ socket_getservbyname(PyObject *self, PyObject *args)
62886294
}
62896295

62906296
Py_BEGIN_ALLOW_THREADS
6297+
#ifdef HAVE_GETSERVBYNAME_R
6298+
getservbyname_r(name, proto, &entry, buf, sizeof(buf), &sp);
6299+
#else
62916300
sp = getservbyname(name, proto);
6301+
#endif
62926302
Py_END_ALLOW_THREADS
62936303
if (sp == NULL) {
62946304
PyErr_SetString(PyExc_OSError, "service/proto not found");
@@ -6305,7 +6315,7 @@ The optional protocol name, if given, should be 'tcp' or 'udp',\n\
63056315
otherwise any protocol will match.");
63066316
#endif
63076317

6308-
#ifdef HAVE_GETSERVBYPORT
6318+
#if defined(HAVE_GETSERVBYPORT_R) || defined (HAVE_GETSERVBYPORT)
63096319
/* Python interface to getservbyport(port).
63106320
This only returns the service name, since the other info is already
63116321
known or not useful (like the list of aliases). */
@@ -6316,6 +6326,12 @@ socket_getservbyport(PyObject *self, PyObject *args)
63166326
{
63176327
int port;
63186328
const char *proto=NULL;
6329+
#ifdef HAVE_GETSERVBYPORT_R
6330+
struct servent entry;
6331+
/* TODO: The man page says 1024 is usually enough, start with that and
6332+
retry if insufficient? */
6333+
char buf[16384];
6334+
#endif
63196335
struct servent *sp;
63206336
if (!PyArg_ParseTuple(args, "i|s:getservbyport", &port, &proto))
63216337
return NULL;
@@ -6331,7 +6347,11 @@ socket_getservbyport(PyObject *self, PyObject *args)
63316347
}
63326348

63336349
Py_BEGIN_ALLOW_THREADS
6350+
#ifdef HAVE_GETSERVBYPORT_R
6351+
getservbyport_r(htons((short)port), proto, &entry, buf, sizeof(buf), &sp);
6352+
#else
63346353
sp = getservbyport(htons((short)port), proto);
6354+
#endif
63356355
Py_END_ALLOW_THREADS
63366356
if (sp == NULL) {
63376357
PyErr_SetString(PyExc_OSError, "port/proto not found");
@@ -6348,7 +6368,7 @@ The optional protocol name, if given, should be 'tcp' or 'udp',\n\
63486368
otherwise any protocol will match.");
63496369
#endif
63506370

6351-
#ifdef HAVE_GETPROTOBYNAME
6371+
#if defined(HAVE_GETPROTOBYNAME_R) || defined (HAVE_GETPROTOBYNAME)
63526372
/* Python interface to getprotobyname(name).
63536373
This only returns the protocol number, since the other info is
63546374
already known or not useful (like the list of aliases). */
@@ -6358,11 +6378,21 @@ static PyObject *
63586378
socket_getprotobyname(PyObject *self, PyObject *args)
63596379
{
63606380
const char *name;
6381+
#ifdef HAVE_GETPROTOBYNAME_R
6382+
struct protoent entry;
6383+
/* TODO: The man page says 1024 is usually enough, start with that and
6384+
retry if insufficient? */
6385+
char buf[16384];
6386+
#endif
63616387
struct protoent *sp;
63626388
if (!PyArg_ParseTuple(args, "s:getprotobyname", &name))
63636389
return NULL;
63646390
Py_BEGIN_ALLOW_THREADS
6391+
#ifdef HAVE_GETPROTOBYNAME_R
6392+
getprotobyname_r(name, &entry, buf, sizeof(buf), &sp);
6393+
#else
63656394
sp = getprotobyname(name);
6395+
#endif
63666396
Py_END_ALLOW_THREADS
63676397
if (sp == NULL) {
63686398
PyErr_SetString(PyExc_OSError, "protocol not found");
@@ -7415,15 +7445,15 @@ static PyMethodDef socket_methods[] = {
74157445
{"sethostname", socket_sethostname,
74167446
METH_VARARGS, sethostname_doc},
74177447
#endif
7418-
#ifdef HAVE_GETSERVBYNAME
7448+
#if defined(HAVE_GETSERVBYNAME_R) || defined (HAVE_GETSERVBYNAME)
74197449
{"getservbyname", socket_getservbyname,
74207450
METH_VARARGS, getservbyname_doc},
74217451
#endif
7422-
#ifdef HAVE_GETSERVBYPORT
7452+
#if defined(HAVE_GETSERVBYPORT_R) || defined (HAVE_GETSERVBYPORT)
74237453
{"getservbyport", socket_getservbyport,
74247454
METH_VARARGS, getservbyport_doc},
74257455
#endif
7426-
#ifdef HAVE_GETPROTOBYNAME
7456+
#if defined (HAVE_GETPROTOBYNAME_R) || defined (HAVE_GETPROTOBYNAME)
74277457
{"getprotobyname", socket_getprotobyname,
74287458
METH_VARARGS, getprotobyname_doc},
74297459
#endif

configure

+123
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

configure.ac

+3
Original file line numberDiff line numberDiff line change
@@ -5395,6 +5395,9 @@ PY_CHECK_NETDB_FUNC([getservbyport])
53955395
PY_CHECK_NETDB_FUNC([gethostbyname])
53965396
PY_CHECK_NETDB_FUNC([gethostbyaddr])
53975397
PY_CHECK_NETDB_FUNC([getprotobyname])
5398+
PY_CHECK_NETDB_FUNC([getservbyname_r])
5399+
PY_CHECK_NETDB_FUNC([getservbyport_r])
5400+
PY_CHECK_NETDB_FUNC([getprotobyname_r])
53985401

53995402
dnl PY_CHECK_SOCKET_FUNC(FUNCTION)
54005403
AC_DEFUN([PY_CHECK_SOCKET_FUNC], [PY_CHECK_FUNC([$1], [

pyconfig.h.in

+9
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,9 @@
557557
/* Define if you have the 'getprotobyname' function. */
558558
#undef HAVE_GETPROTOBYNAME
559559

560+
/* Define if you have the 'getprotobyname_r' function. */
561+
#undef HAVE_GETPROTOBYNAME_R
562+
560563
/* Define to 1 if you have the 'getpwent' function. */
561564
#undef HAVE_GETPWENT
562565

@@ -587,9 +590,15 @@
587590
/* Define if you have the 'getservbyname' function. */
588591
#undef HAVE_GETSERVBYNAME
589592

593+
/* Define if you have the 'getservbyname_r' function. */
594+
#undef HAVE_GETSERVBYNAME_R
595+
590596
/* Define if you have the 'getservbyport' function. */
591597
#undef HAVE_GETSERVBYPORT
592598

599+
/* Define if you have the 'getservbyport_r' function. */
600+
#undef HAVE_GETSERVBYPORT_R
601+
593602
/* Define to 1 if you have the 'getsid' function. */
594603
#undef HAVE_GETSID
595604

0 commit comments

Comments
 (0)