gh-109521: Fix obscure cases handling in PyImport_GetImporter() by serhiy-storchaka · Pull Request #109522 · python/cpython

serhiy-storchaka · 2023-09-17T18:54:07Z

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks or sys.path_importer_cache or they are not list and dict correspondingle.

Previously it could return NULL without setting error in obscure cases, crash or raise SystemError if these attributes have wrong type.

Issue: Problems with PyImport_GetImporter() #109521

PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks or sys.path_importer_cache or they are not list and dict correspondingle. Previously it could return NULL without setting error in obscure cases, crash or raise SystemError if these attributes have wrong type.

vstinner

LGTM. I left minor remarks.

Misc/NEWS.d/next/C API/2023-09-17-21-47-31.gh-issue-109521.JDF6i9.rst

vstinner · 2023-09-18T16:46:52Z

Python/import.c

+        PyErr_SetString(PyExc_RuntimeError, "sys.path_hooks is not a list");
+        return NULL;
+    }
+    if (!PyDict_Check(path_importer_cache)) {


You might replace PyDict_GetItemWithError() with PyDict_GetItemRef() below to avoid _PyErr_Occurred() call.

I am going to backport this change.

BTW, I have a separate branch in which most of PyDict_GetItemWithError() are replaced with PyDict_GetItemRef(). I will make a PR or several PRs from it in future.

I am going to backport this change.

Oh right. In this case, just ignore my remark. It's more a "coding style" change than a fix, the current code is correct.

…6i9.rst Co-authored-by: Victor Stinner <vstinner@python.org>

miss-islington · 2023-09-23T06:39:27Z

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington · 2023-09-23T06:39:32Z

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 62c7015e89cbdedb5218d4fedd45f971885f67a8 3.11

…pythonGH-109522) PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks or sys.path_importer_cache or they are not list and dict correspondingly. Previously it could return NULL without setting error in obscure cases, crash or raise SystemError if these attributes have wrong type. (cherry picked from commit 62c7015) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-app · 2023-09-23T06:39:37Z

GH-109777 is a backport of this pull request to the 3.12 branch.

bedevere-bot · 2023-09-23T06:57:04Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x Debian 3.x has failed when building commit 62c7015.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/49/builds/6646) and take a look at the build logs.
Check if the failure is related to this commit (62c7015) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/49/builds/6646

Failed tests:

test_tools

Failed subtests:

test_freeze_simple_script - test.test_tools.test_freeze.TestFreeze.test_freeze_simple_script

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Lib/test/test_tools/test_freeze.py", line 32, in test_freeze_simple_script
    outdir, scriptfile, python = helper.prepare(script, outdir)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Tools/freeze/test/freeze.py", line 146, in prepare
    copy_source_tree(srcdir, SRCDIR)
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Tools/freeze/test/freeze.py", line 95, in copy_source_tree
    shutil.copytree(oldroot, newroot, ignore=ignore_non_src)
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Lib/shutil.py", line 588, in copytree
    return _copytree(entries=entries, src=src, dst=dst, symlinks=symlinks,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Lib/shutil.py", line 542, in _copytree
    raise Error(errors)
shutil.Error: [('/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/build/test_python_1317071æ/@test_1317071_tmpæ-tardir/extractall_ctrl/ustar/fifotype', '/tmp/test_python_yj50mh7g/tmpuo8csym2/cpython/build/test_python_1317071æ/@test_1317071_tmpæ-tardir/extractall_ctrl/ustar/fifotype', '`/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/build/test_python_1317071æ/@test_1317071_tmpæ-tardir/extractall_ctrl/ustar/fifotype` is a named pipe')]

…orter() (pythonGH-109522) PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks or sys.path_importer_cache or they are not list and dict correspondingly. Previously it could return NULL without setting error in obscure cases, crash or raise SystemError if these attributes have wrong type.. (cherry picked from commit 62c7015) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-app · 2023-09-23T08:44:55Z

GH-109781 is a backport of this pull request to the 3.11 branch.

…pythonGH-109522) PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks or sys.path_importer_cache or they are not list and dict correspondingly. Previously it could return NULL without setting error in obscure cases, crash or raise SystemError if these attributes have wrong type.

GH-109522) (#109777) gh-109521: Fix obscure cases handling in PyImport_GetImporter() (GH-109522) PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks or sys.path_importer_cache or they are not list and dict correspondingly. Previously it could return NULL without setting error in obscure cases, crash or raise SystemError if these attributes have wrong type. (cherry picked from commit 62c7015) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

GH-109522) (GH-109781) PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks or sys.path_importer_cache or they are not list and dict correspondingly. Previously it could return NULL without setting error in obscure cases, crash or raise SystemError if these attributes have wrong type. (cherry picked from commit 62c7015)

…pythonGH-109522) PyImport_GetImporter() now sets RuntimeError if it fails to get sys.path_hooks or sys.path_importer_cache or they are not list and dict correspondingly. Previously it could return NULL without setting error in obscure cases, crash or raise SystemError if these attributes have wrong type.

serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Sep 17, 2023

serhiy-storchaka requested review from ncoghlan and vstinner September 17, 2023 18:54

serhiy-storchaka requested a review from kumaraditya303 as a code owner September 17, 2023 18:54

bedevere-app bot added the awaiting review label Sep 17, 2023

bedevere-app bot mentioned this pull request Sep 17, 2023

Problems with PyImport_GetImporter() #109521

Closed

vstinner approved these changes Sep 18, 2023

View reviewed changes

bedevere-app bot added awaiting core review and removed awaiting review labels Sep 18, 2023

Update Misc/NEWS.d/next/C API/2023-09-17-21-47-31.gh-issue-109521.JDF…

d6b9a14

…6i9.rst Co-authored-by: Victor Stinner <vstinner@python.org>

serhiy-storchaka merged commit 62c7015 into python:main Sep 23, 2023

bedevere-app bot removed the awaiting core review label Sep 23, 2023

serhiy-storchaka deleted the capi-PyImport_GetImporter-errors branch September 23, 2023 06:39

miss-islington assigned serhiy-storchaka Sep 23, 2023

bedevere-app bot removed the needs backport to 3.12 only security fixes label Sep 23, 2023

bedevere-app bot removed the needs backport to 3.11 only security fixes label Sep 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

gh-109521: Fix obscure cases handling in PyImport_GetImporter()#109522

gh-109521: Fix obscure cases handling in PyImport_GetImporter()#109522
serhiy-storchaka merged 2 commits intopython:mainfrom
serhiy-storchaka:capi-PyImport_GetImporter-errors

serhiy-storchaka commented Sep 17, 2023 •

edited by bedevere-app bot

Loading

Uh oh!

vstinner left a comment

Uh oh!

Uh oh!

vstinner Sep 18, 2023

Uh oh!

serhiy-storchaka Sep 18, 2023

Uh oh!

vstinner Sep 18, 2023

Uh oh!

miss-islington commented Sep 23, 2023

Uh oh!

miss-islington commented Sep 23, 2023

Uh oh!

bedevere-app bot commented Sep 23, 2023

Uh oh!

bedevere-bot commented Sep 23, 2023

Uh oh!

bedevere-app bot commented Sep 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

serhiy-storchaka commented Sep 17, 2023 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vstinner Sep 18, 2023

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 18, 2023

Choose a reason for hiding this comment

Uh oh!

vstinner Sep 18, 2023

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Sep 23, 2023

Uh oh!

miss-islington commented Sep 23, 2023

Uh oh!

bedevere-app bot commented Sep 23, 2023

Uh oh!

bedevere-bot commented Sep 23, 2023

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

bedevere-app bot commented Sep 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

serhiy-storchaka commented Sep 17, 2023 •

edited by bedevere-app bot

Loading