Issue #28332: Deprecated silent truncations in socket.htons and socket.ntohs.

Original patch by Oren Milman.

4 files changed, 75 insertions, 22 deletions

diff --git a/Doc/library/socket.rst b/Doc/library/socket.rst
index 6909a64..d33bbc3 100644
--- a/Doc/library/socket.rst
+++ b/Doc/library/socket.rst
@@ -664,6 +664,12 @@ The :mod:`socket` module also offers various network-related services:
    where the host byte order is the same as network byte order, this is a no-op;
    otherwise, it performs a 2-byte swap operation.
 
+   .. deprecated:: 3.7
+      In case *x* does not fit in 16-bit unsigned integer, but does fit in a
+      positive C int, it is silently truncated to 16-bit unsigned integer.
+      This silent truncation feature is deprecated, and will raise an
+      exception in future versions of Python.
+
 
 .. function:: htonl(x)
 
@@ -678,6 +684,12 @@ The :mod:`socket` module also offers various network-related services:
    where the host byte order is the same as network byte order, this is a no-op;
    otherwise, it performs a 2-byte swap operation.
 
+   .. deprecated:: 3.7
+      In case *x* does not fit in 16-bit unsigned integer, but does fit in a
+      positive C int, it is silently truncated to 16-bit unsigned integer.
+      This silent truncation feature is deprecated, and will raise an
+      exception in future versions of Python.
+
 
 .. function:: inet_aton(ip_string)
 
diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index c9add6c..fcadd5b 100644
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -888,18 +888,28 @@ class GeneralModuleTests(unittest.TestCase):
             self.assertRaises(OverflowError, func, 1<<34)
 
     def testNtoHErrors(self):
-        good_values = [ 1, 2, 3, 1, 2, 3 ]
-        bad_values = [ -1, -2, -3, -1, -2, -3 ]
-        for k in good_values:
-            socket.ntohl(k)
+        import _testcapi
+        s_good_values = [0, 1, 2, 0xffff]
+        l_good_values = s_good_values + [0xffffffff]
+        l_bad_values = [-1, -2, 1<<32, 1<<1000]
+        s_bad_values = l_bad_values + [_testcapi.INT_MIN - 1,
+                                       _testcapi.INT_MAX + 1]
+        s_deprecated_values = [1<<16, _testcapi.INT_MAX]
+        for k in s_good_values:
             socket.ntohs(k)
-            socket.htonl(k)
             socket.htons(k)
-        for k in bad_values:
-            self.assertRaises(OverflowError, socket.ntohl, k)
+        for k in l_good_values:
+            socket.ntohl(k)
+            socket.htonl(k)
+        for k in s_bad_values:
             self.assertRaises(OverflowError, socket.ntohs, k)
-            self.assertRaises(OverflowError, socket.htonl, k)
             self.assertRaises(OverflowError, socket.htons, k)
+        for k in l_bad_values:
+            self.assertRaises(OverflowError, socket.ntohl, k)
+            self.assertRaises(OverflowError, socket.htonl, k)
+        for k in s_deprecated_values:
+            self.assertWarns(DeprecationWarning, socket.ntohs, k)
+            self.assertWarns(DeprecationWarning, socket.htons, k)
 
     def testGetServBy(self):
         eq = self.assertEqual
diff --git a/Misc/NEWS b/Misc/NEWS
index 4689360..7d13a33 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -54,6 +54,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #28332: Deprecated silent truncations in socket.htons and socket.ntohs.
+  Original patch by Oren Milman.
+
 - Issue #27358: Optimized merging var-keyword arguments and improved error
   message when pass a non-mapping as a var-keyword argument.
 
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index d25bd7f..9709186 100644
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -5498,24 +5498,38 @@ AF_UNIX if defined on the platform; otherwise, the default is AF_INET.");
 static PyObject *
 socket_ntohs(PyObject *self, PyObject *args)
 {
-    int x1, x2;
+    int x;
 
-    if (!PyArg_ParseTuple(args, "i:ntohs", &x1)) {
+    if (!PyArg_ParseTuple(args, "i:ntohs", &x)) {
         return NULL;
     }
-    if (x1 < 0) {
+    if (x < 0) {
         PyErr_SetString(PyExc_OverflowError,
-            "can't convert negative number to unsigned long");
+                        "ntohs: can't convert negative Python int to C "
+                        "16-bit unsigned integer");
         return NULL;
     }
-    x2 = (unsigned int)ntohs((unsigned short)x1);
-    return PyLong_FromLong(x2);
+    if (x > 0xffff) {
+        if (PyErr_WarnEx(PyExc_DeprecationWarning,
+                         "ntohs: Python int too large to convert to C "
+                         "16-bit unsigned integer (The silent truncation "
+                         "is deprecated)",
+                         1)) {
+            return NULL;
+        }
+    }
+    return PyLong_FromUnsignedLong(ntohs((unsigned short)x));
 }
 
 PyDoc_STRVAR(ntohs_doc,
 "ntohs(integer) -> integer\n\
 \n\
-Convert a 16-bit integer from network to host byte order.");
+Convert a 16-bit unsigned integer from network to host byte order.\n\
+Note that in case the received integer does not fit in 16-bit unsigned\n\
+integer, but does fit in a positive C int, it is silently truncated to\n\
+16-bit unsigned integer.\n\
+However, this silent truncation feature is deprecated, and will raise an \n\
+exception in future versions of Python.");
 
 
 static PyObject *
@@ -5555,24 +5569,38 @@ Convert a 32-bit integer from network to host byte order.");
 static PyObject *
 socket_htons(PyObject *self, PyObject *args)
 {
-    int x1, x2;
+    int x;
 
-    if (!PyArg_ParseTuple(args, "i:htons", &x1)) {
+    if (!PyArg_ParseTuple(args, "i:htons", &x)) {
         return NULL;
     }
-    if (x1 < 0) {
+    if (x < 0) {
         PyErr_SetString(PyExc_OverflowError,
-            "can't convert negative number to unsigned long");
+                        "htons: can't convert negative Python int to C "
+                        "16-bit unsigned integer");
         return NULL;
     }
-    x2 = (unsigned int)htons((unsigned short)x1);
-    return PyLong_FromLong(x2);
+    if (x > 0xffff) {
+        if (PyErr_WarnEx(PyExc_DeprecationWarning,
+                         "htons: Python int too large to convert to C "
+                         "16-bit unsigned integer (The silent truncation "
+                         "is deprecated)",
+                         1)) {
+            return NULL;
+        }
+    }
+    return PyLong_FromUnsignedLong(htons((unsigned short)x));
 }
 
 PyDoc_STRVAR(htons_doc,
 "htons(integer) -> integer\n\
 \n\
-Convert a 16-bit integer from host to network byte order.");
+Convert a 16-bit unsigned integer from host to network byte order.\n\
+Note that in case the received integer does not fit in 16-bit unsigned\n\
+integer, but does fit in a positive C int, it is silently truncated to\n\
+16-bit unsigned integer.\n\
+However, this silent truncation feature is deprecated, and will raise an \n\
+exception in future versions of Python.");
 
 
 static PyObject *