gh-98415: Fix UUID ifconfig MAC address detection #98423
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
There was a problem hiding this comment.
Nice catch!
I reproduce the issue with the following script:
import subprocess
"""Get the hardware address on Unix by running ifconfig."""
# This works on Linux ('' or '-a'), Tru64 ('-av'), but not all Unixes.
def foo(*args):
command = ("/sbin/ifconfig",) + args
proc = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL)
if not proc:
return
stdout, stderr = proc.communicate()
mac = stdout # This is fine for this example (normally it'd have parsed out a mac
if not mac:
print(args, mac, stderr)
for args in ('', '-a', '-av'):
foo(args)
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Misc/NEWS.d/next/Core and Builtins/2022-10-19-01-01-08.gh-issue-98415.ZS2eWh.rst
Outdated
Show resolved
Hide resolved
|
I have a preference for |
It looks hard to write the unittest due to system-specific implementation. |
|
Thanks @csanders-git for the PR, and @vstinner for merging it |
|
Sorry @csanders-git and @vstinner, I had trouble checking out the |
…98423) The uuid.getnode() function has multiple implementations, tested sequentially. The ifconfig implementation was incorrect and always failed: fix it. In practice, functions of libuuid library are preferred, if available: uuid_generate_time_safe(), uuid_create() or uuid_generate_time(). (cherry picked from commit e3ec272) Co-authored-by: Chaim Sanders <csanders-git@users.noreply.github.com> Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
|
GH-99020 is a backport of this pull request to the 3.10 branch. |
|
Thanks @csanders-git for the PR, and @vstinner for merging it |
…98423) The uuid.getnode() function has multiple implementations, tested sequentially. The ifconfig implementation was incorrect and always failed: fix it. In practice, functions of libuuid library are preferred, if available: uuid_generate_time_safe(), uuid_create() or uuid_generate_time(). (cherry picked from commit e3ec272) Co-authored-by: Chaim Sanders <csanders-git@users.noreply.github.com> Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
|
GH-99021 is a backport of this pull request to the 3.11 branch. |
|
Ok, thanks @csanders-git for the fix: I merged it. I rewrote the commit message to clarify its scope. |
It makes me sad, but I'm fine with not adding a test :-) |
|
In the past, I fixed a risk of shell injection by avoiding a shell in the uuid module: commit b9d0199. I didn't add any test for that neither :-( |
The uuid.getnode() function has multiple implementations, tested sequentially. The ifconfig implementation was incorrect and always failed: fix it. In practice, functions of libuuid library are preferred, if available: uuid_generate_time_safe(), uuid_create() or uuid_generate_time(). (cherry picked from commit e3ec272) Co-authored-by: Chaim Sanders <csanders-git@users.noreply.github.com> Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
The uuid.getnode() function has multiple implementations, tested sequentially. The ifconfig implementation was incorrect and always failed: fix it. In practice, functions of libuuid library are preferred, if available: uuid_generate_time_safe(), uuid_create() or uuid_generate_time(). (cherry picked from commit e3ec272) Co-authored-by: Chaim Sanders <csanders-git@users.noreply.github.com> Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
See #98415 for explanation