This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: getaddrinfo raises OverflowError
Type: behavior Stage:
Components: Extension Modules Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, cheryl.sabella, remi.lapeyre, serhiy.storchaka, smejkar
Priority: normal Keywords: patch

Created on 2017-06-20 13:19 by smejkar, last changed 2022-04-11 14:58 by admin.

Files
File name Uploaded Description Edit
getaddrinfo_overflow_error.patch smejkar, 2017-06-20 16:39
Pull Requests
URL Status Linked Edit
PR 2435 open smejkar, 2017-06-27 11:21
Messages (11)
msg296421 - (view) Author: Radek Smejkal (smejkar) * Date: 2017-06-20 13:19
If the port argument is a number, getaddrinfo attempts to convert it to a C long, that raises OverflowError if the conversion fails.

Instead, getaddrinfo should convert the port to a string (bytes) directly and rely on the underlying getaddrinfo to return EAI_SERVICE. This is also consistent with the case when a numeric port is passed as a string.
msg296437 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-20 14:22
What do you mean by converting the port to a string (bytes) directly? Can you provide an example?
msg296471 - (view) Author: Radek Smejkal (smejkar) * Date: 2017-06-20 16:39
Use PyObject_Str and PyUnicode_AsUTF8 if the port argument is a PyLong instead of converting it to an intermediate C long that may raise OverflowError. See getaddrinfo_overflow_error.patch
msg296473 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-20 16:54
Now I understand your idea. It looks reasonable to me. Could you create a pull request on GitHub? Please provide also tests, add a Misc/NEWS entry and add your name in Misc/ACKS.

But how large is the performance hit of this change? It adds at least one additional memory allocation for the str object. If the slowdown is significant perhaps it is worth to keep the old code as a fast path.
msg296526 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-06-21 06:46
Why can't the user simply pass in a string service name in the first place?
msg296536 - (view) Author: Radek Smejkal (smejkar) * Date: 2017-06-21 10:08
> But how large is the performance hit of this change? It adds at least one additional memory allocation for the str object. If the slowdown is significant perhaps it is worth to keep the old code as a fast path.

commit 5cc7ac24da10568d2a910a91a24183b904118cf8
./python -m timeit -s 'import socket' 'socket.getaddrinfo(None, 1000)'
2000 loops, best of 5: 139 usec per loop
./python -m timeit -s 'import socket' 'socket.getaddrinfo(None, "1000")'
2000 loops, best of 5: 142 usec per loop

with getaddrinfo_overflow_error.patch
./python -m timeit -s 'import socket' 'socket.getaddrinfo(None, 1000)'
2000 loops, best of 5: 140 usec per loop
./python -m timeit -s 'import socket' 'socket.getaddrinfo(None, "1000")'
2000 loops, best of 5: 139 usec per loop

It seems the impact on performance is negligible/not measurable.
msg296540 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-21 10:45
I like your idea about getting rid of OverflowError. But wouldn't it make the problem with other reported by you issue, issue30711, worse?
msg296541 - (view) Author: Radek Smejkal (smejkar) * Date: 2017-06-21 10:47
> Why can't the user simply pass in a string service name in the first place?
He/she can. But I don't propose to change that. The patch only changes the way a given number is converted to a string. That is, without an intermediate C long.
msg296548 - (view) Author: Radek Smejkal (smejkar) * Date: 2017-06-21 12:00
> I like your idea about getting rid of OverflowError. But wouldn't it make the problem with other reported by you issue, issue30711, worse?

It depends on the implementation of the underlying getaddrinfo. For Modules/getaddrinfo.c, it will be worse for positive numbers outside the C long range.
msg370283 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2020-05-29 09:52
The original pull request has been closed as inactive.  If the original author is interested in pursuing this issue, it can be reopened or someone else can create a new pull request to replace it.
msg370285 - (view) Author: Rémi Lapeyre (remi.lapeyre) * Date: 2020-05-29 09:57
Hi Cheryl Sabella, the pull request that got closed is the one for issue 30711. The pull request for this one has never been reviewed.
History
Date User Action Args
2022-04-11 14:58:47adminsetgithub: 74895
2020-05-29 09:57:44remi.lapeyresetnosy: + remi.lapeyre
messages: + msg370285
2020-05-29 09:52:56cheryl.sabellasetnosy: + cheryl.sabella
messages: + msg370283
2017-06-27 11:21:41smejkarsetpull_requests: + pull_request2483
2017-06-21 12:00:15smejkarsetmessages: + msg296548
2017-06-21 10:47:28smejkarsetmessages: + msg296541
2017-06-21 10:45:21serhiy.storchakasetmessages: + msg296540
2017-06-21 10:08:41smejkarsetmessages: + msg296536
2017-06-21 06:46:09benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg296526
2017-06-20 16:54:34serhiy.storchakasetmessages: + msg296473
2017-06-20 16:39:27smejkarsetfiles: + getaddrinfo_overflow_error.patch
keywords: + patch
messages: + msg296471

components: + Extension Modules, - Library (Lib)
2017-06-20 14:22:17serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg296437
2017-06-20 13:19:55smejkarcreate