[issue28332] silent truncations in socket.htons and socket.ntohs

[Oren Milman]

Changes by Oren Milman :


--
title: Deprecated silent truncations in socket.htons and socket.ntohs. -> 
silent truncations in socket.htons and socket.ntohs

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Donald Stufft]

Changes by Donald Stufft :


--
pull_requests: +929

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for you contribution Oren.

Rules for promotion unsinged integers to signed integers are not simple. I 
changed PyLong_FromLong() to PyLong_FromUnsignedLong() for the clarity.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Roundup Robot]

Roundup Robot added the comment:

New changeset 3da460ca854b by Serhiy Storchaka in branch 'default':
Issue #28332: Deprecated silent truncations in socket.htons and socket.ntohs.
https://hg.python.org/cpython/rev/3da460ca854b

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Oren Milman]

Changes by Oren Milman :


Added file: http://bugs.python.org/file44923/patchedCPythonTestOutput_ver2.txt

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Oren Milman]

Oren Milman added the comment:

Thanks for the review :)
I changed some stuff, according to your comments (and replied to one comment in 
Rietveld).
Version2 diff and test output are attached.

--
Added file: http://bugs.python.org/file44922/issue28332_ver2.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Looks reasonable. ntohl() and htonl() already raise an exception if the 
argument exceeds 32 bit. Added comments on Rietveld.

--
nosy: +serhiy.storchaka
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Oren Milman]

Changes by Oren Milman :


--
keywords: +patch
Added file: http://bugs.python.org/file44913/issue28332_ver1.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Oren Milman]

Changes by Oren Milman :


Added file: http://bugs.python.org/file44912/patchedCPythonTestOutput_ver1.txt

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28332] silent truncations in socket.htons and socket.ntohs

[Oren Milman]

New submission from Oren Milman:

 current state 
Due to the implementation of socket_htons (in Modules/socketmodule.c), in case 
the received integer 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 (before 
converting to network byte order):
>>> import socket
>>> hex(socket.htons(0x1234))
'0x3412'
>>> hex(socket.htons(0x81234))
'0x3412'
>>> hex(socket.htons(0x881234))
'0x3412'
>>> hex(socket.htons(0x8881234))
'0x3412'
>>> hex(socket.htons(0x1234))
Traceback (most recent call last):
  File "", line 1, in 
OverflowError: Python int too large to convert to C long
>>>

Likewise, socket.ntohs has the same silent truncation feature, due to the 
implementation of socket_ntohs.

ISTM this silent truncation feature has the potential to conceal nasty bugs, 
and I guess it is rarely used in purpose.

With regard to relevant changes made in the past:
* The silent truncation was there since the two functions were first added, 
in changeset 3673 (https://hg.python.org/cpython/rev/f6ace61c3dfe).
* A check whether the received integer is negative was added (to each of 
the two functions) in changeset 40632 
(https://hg.python.org/cpython/rev/6efe3a4b10ac), as part of #1635058.
Note the lack of discussion in #1635058 and #1619659 about backward 
compatibility. It might suggest that Guido didn't hesitate to make the change, 
even though at the time, the four conversion functions (socket.htons, 
socket.ntohs, socket.htonl and socket.ntohl) were already in the wild for 10 
years.


 proposed changes 
1. In Modules/socketmodule.c, raise a DeprecationWarning before silently 
truncating the received integer. In Python 3.8, replace the DeprecationWarning 
with an OverflowError.

2. In Lib/test/test_socket.py, add tests to verify a DeprecationWarning is 
raised as expected.

3. In Doc/library/socket.rst, add a description of the silent truncation 
feature, and declare it is deprecated.


 diff 
The proposed patches diff file is attached.

(I wasn't sure you would approve deprecating a feature that was in the wild for 
so long, but I implemented it anyway, as it was quite simple.)


 tests 
I ran 'python_d.exe -m test -j3' (on my 64-bit Windows 10) with and without the 
patches, and got quite the same output. (That also means my new tests in 
test_socket passed.)
The outputs of both runs are attached.

--
components: Library (Lib)
files: CPythonTestOutput.txt
messages: 277820
nosy: Oren Milman
priority: normal
severity: normal
status: open
title: silent truncations in socket.htons and socket.ntohs
type: behavior
versions: Python 3.7
Added file: http://bugs.python.org/file44911/CPythonTestOutput.txt

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com