[BUG] [doctest] pkg_resources.NullProvider._validate_resource_path and TestWriteEntries.test_invalid_entry_point fail with Python 3.13.0a3 #4196

befeleme · 2024-01-29T15:48:48Z

setuptools version

69.0.3

Python version

Python 3.13.0a3

OS

Fedora Linux 40

Additional environment information

This happens in the context of the RPM build of setuptools for Fedora Linux.

Description

Two test fail when building setuptools with Python 3.13.0a3

Expected behavior

Test pass

How to Reproduce

Happy to test it in the RPM environment.

Output

=================================== FAILURES ===================================
_________ [doctest] pkg_resources.NullProvider._validate_resource_path _________
1609 >>> vrp('foo/../../bar.txt')
1610 >>> bool(warned)
1611 True
1612 >>> warned.clear()
1613 >>> vrp('foo/f../bar.txt')
1614 >>> bool(warned)
1615 False
1616 
1617 Windows path separators are straight-up disallowed.
1618 >>> vrp(r'\foo/bar.txt')
Expected:
    Traceback (most recent call last):
    ...
    ValueError: Use of .. or absolute path in a resource path is not allowed.
Got nothing

/builddir/build/BUILD/setuptools-69.0.3/pkg_resources/__init__.py:1618: DocTestFailure
__________________ TestWriteEntries.test_invalid_entry_point ___________________

self = <setuptools.tests.test_egg_info.TestWriteEntries object at 0x7ff1e7d28a40>
tmpdir_cwd = local('/builddir/build/BUILD/setuptools-69.0.3')
env = '/tmp/setuptools-test.no9jl_q9'

    def test_invalid_entry_point(self, tmpdir_cwd, env):
        dist = Distribution({"name": "foo", "version": "0.0.1"})
        dist.entry_points = {"foo": "foo = invalid-identifier:foo"}
        cmd = dist.get_command_obj("egg_info")
        expected_msg = r"Problems to parse .*invalid-identifier.*"
        with pytest.raises(errors.OptionError, match=expected_msg) as ex:
>           write_entries(cmd, "entry_points", "entry_points.txt")

/builddir/build/BUILD/setuptools-69.0.3/setuptools/tests/test_egg_info.py:1310: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/builddir/build/BUILD/setuptools-69.0.3/setuptools/command/egg_info.py:722: in write_entries
    eps = _entry_points.load(cmd.distribution.entry_points)
/usr/lib64/python3.13/functools.py:911: in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
/builddir/build/BUILD/setuptools-69.0.3/setuptools/_entry_points.py:59: in load
    return validate(metadata.EntryPoints(groups))
/builddir/build/BUILD/setuptools-69.0.3/setuptools/_entry_points.py:47: in validate
    consume(map(ensure_valid, ensure_unique(eps, key=by_group_and_name)))
/builddir/build/BUILD/setuptools-69.0.3/setuptools/_vendor/more_itertools/recipes.py:139: in consume
    deque(iterator, maxlen=0)
/builddir/build/BUILD/setuptools-69.0.3/setuptools/_entry_points.py:19: in ensure_valid
    ep.extras
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = EntryPoint(name='foo', value='invalid-identifier:foo', group='foo')

    @property
    def extras(self) -> List[str]:
        match = self.pattern.match(self.value)
>       assert match is not None
E       AssertionError

/usr/lib64/python3.13/importlib/metadata/__init__.py:197: AssertionError

hroncok · 2024-01-29T16:19:24Z

I can reproduce directly from the main branch @ 04a6bfe via tox -e py313 -- -k "_validate_resource_path or test_invalid_entry_point".

============================= test session starts ==============================
platform linux -- Python 3.13.0a3, pytest-8.0.0, pluggy-1.4.0
cachedir: .tox/py313/.pytest_cache
rootdir: .../pypa/setuptools
configfile: pytest.ini
plugins: ruff-0.2.1, xdist-3.5.0, timeout-2.2.0, mypy-0.10.3, home-0.5.1, enabler-3.0.0, cov-4.1.0, checkdocs-2.10.1, perf-0.14.0
created: 8/8 workers
8 workers [2 items]

FF                                                                       [100%]
=================================== FAILURES ===================================
_________ [doctest] pkg_resources.NullProvider._validate_resource_path _________
[gw0] linux -- Python 3.13.0 .../pypa/setuptools/.tox/py313/bin/python
1598 >>> vrp('foo/../../bar.txt')
1599 >>> bool(warned)
1600 True
1601 >>> warned.clear()
1602 >>> vrp('foo/f../bar.txt')
1603 >>> bool(warned)
1604 False
1605 
1606 Windows path separators are straight-up disallowed.
1607 >>> vrp(r'\foo/bar.txt')
Expected:
    Traceback (most recent call last):
    ...
    ValueError: Use of .. or absolute path in a resource path is not allowed.
Got nothing

.../pypa/setuptools/pkg_resources/__init__.py:1607: DocTestFailure
__________________ TestWriteEntries.test_invalid_entry_point ___________________
[gw1] linux -- Python 3.13.0 .../pypa/setuptools/.tox/py313/bin/python

self = <setuptools.tests.test_egg_info.TestWriteEntries object at 0x7f81f22d1160>
tmpdir_cwd = local('.../pypa/setuptools')
env = '/tmp/setuptools-test.7xqq8vhm'

    def test_invalid_entry_point(self, tmpdir_cwd, env):
        dist = Distribution({"name": "foo", "version": "0.0.1"})
        dist.entry_points = {"foo": "foo = invalid-identifier:foo"}
        cmd = dist.get_command_obj("egg_info")
        expected_msg = r"Problems to parse .*invalid-identifier.*"
        with pytest.raises(errors.OptionError, match=expected_msg) as ex:
>           write_entries(cmd, "entry_points", "entry_points.txt")

.../pypa/setuptools/setuptools/tests/test_egg_info.py:1291: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.../pypa/setuptools/setuptools/command/egg_info.py:721: in write_entries
    eps = _entry_points.load(cmd.distribution.entry_points)
/usr/lib64/python3.13/functools.py:911: in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
.../pypa/setuptools/setuptools/_entry_points.py:59: in load
    return validate(metadata.EntryPoints(groups))
.../pypa/setuptools/setuptools/_entry_points.py:47: in validate
    consume(map(ensure_valid, ensure_unique(eps, key=by_group_and_name)))
.../pypa/setuptools/setuptools/_vendor/more_itertools/recipes.py:139: in consume
    deque(iterator, maxlen=0)
.../pypa/setuptools/setuptools/_entry_points.py:19: in ensure_valid
    ep.extras
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = EntryPoint(name='foo', value='invalid-identifier:foo', group='foo')

    @property
    def extras(self) -> List[str]:
        match = self.pattern.match(self.value)
>       assert match is not None
E       AssertionError

/usr/lib64/python3.13/importlib/metadata/__init__.py:197: AssertionError

hroncok · 2024-05-14T12:56:48Z

Still the same result with Python 3.13.0b1 and setuptools @ 544b332

Going to take a closer look.

abravalheri · 2024-05-14T13:37:42Z

The code originating the first failure has the following:

        msg = "Use of .. or absolute path in a resource path is not allowed."

        # Aggressively disallow Windows absolute paths
        if ntpath.isabs(path) and not posixpath.isabs(path):
            raise ValueError(msg)

So I suppose in Python 3.13 ntpath.isabs(r'\foo/bar.txt')) now is false, while previously it was true?

hroncok · 2024-05-14T13:42:59Z

This is what I discovered as well:

$ python3.12 -c 'import ntpath; path = r"\foo"; print(ntpath.isabs(path))'
True
$ python3.13 -c 'import ntpath; path = r"\foo"; print(ntpath.isabs(path))'
False

Let me see if this is intentional change in CPython or a bug.

hroncok · 2024-05-14T13:44:33Z

https://docs.python.org/3.13/library/os.path.html#os.path.isabs

os.path.isabs(path)

Changed in version 3.13: On Windows, returns False if the given path starts with exactly one (back)slash.

hroncok · 2024-05-14T13:49:56Z

I wonder what the function wants to do here. We can check if the first character is a backslash and use that to preserve backward compatibility.

However, the docstring actually says "Windows path separators are straight-up disallowed" which was only true for paths starting with them.

hroncok · 2024-05-14T13:52:49Z

diff --git a/pkg_resources/__init__.py b/pkg_resources/__init__.py
index 713d9bdfa..0ff232f6d 100644
--- a/pkg_resources/__init__.py
+++ b/pkg_resources/__init__.py
@@ -1604,6 +1604,7 @@ is not allowed.
             os.path.pardir in path.split(posixpath.sep)
             or posixpath.isabs(path)
             or ntpath.isabs(path)
+            or path.startswith("\\")
         )
         if not invalid:
             return
@@ -1611,7 +1612,8 @@ is not allowed.
         msg = "Use of .. or absolute path in a resource path is not allowed."
 
         # Aggressively disallow Windows absolute paths
-        if ntpath.isabs(path) and not posixpath.isabs(path):
+        if ((path.startswith("\\") or ntpath.isabs(path))
+          and not posixpath.isabs(path)):
             raise ValueError(msg)
 
         # for compatibility, warn; in future

This makes the doctest pass. Is it the intended behavior?

abravalheri · 2024-05-14T13:52:59Z

I think it is a problem with the escaping (because of the docstring)...

I suppose the original idea was to verify \\foo/bar.txt. But the r"..." is not working in a obvious way because the whole thing is inside a regular docstring.

Does this change make sense?

diff --git i/pkg_resources/__init__.py w/pkg_resources/__init__.py
index 713d9bdfa..9f4d90150 100644
--- i/pkg_resources/__init__.py
+++ w/pkg_resources/__init__.py
@@ -1575,7 +1575,7 @@ class NullProvider:
         False

         Windows path separators are straight-up disallowed.
-        >>> vrp(r'\\foo/bar.txt')
+        >>> vrp(r'\\\\foo/bar.txt')
         Traceback (most recent call last):
         ...
         ValueError: Use of .. or absolute path in a resource path \

hroncok · 2024-05-14T13:55:44Z

Your change also makes the doctest pass. However, do we want to allow paths like \whatnot/foo.txt in the resource path? The docs https://setuptools.pypa.io/en/latest/pkg_resources.html#basic-resource-access would make me think we should not.

abravalheri · 2024-05-14T14:03:01Z

This is the reference for Windows paths starting with \\: https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats#unc-paths.

Your change also makes the doctest pass. However, do we want to allow paths like \whatnot/foo.txt in the resource path? The docs https://setuptools.pypa.io/en/latest/pkg_resources.html#basic-resource-access would make me think we should not.

But the result of the function would still be a warning and taken as invalid, right?

hroncok · 2024-05-14T14:03:08Z

The second test failure reason is quite simple.

In here:

setuptools/setuptools/_entry_points.py

Lines 18 to 25 in 544b332

    
           try: 
        
               ep.extras 
        
           except AttributeError as ex: 
        
               msg = ( 
        
                   f"Problems to parse {ep}.\nPlease ensure entry-point follows the spec: " 
        
                   "https://packaging.python.org/en/latest/specifications/entry-points/" 
        
               ) 
        
               raise OptionError(msg) from ex

setuptools expect AttributeError. importlib.metadata give AssertionError instead. Again, will check if this is intentional or not (however, throwing assertion errors for invalid inputes is bad IMHO).

hroncok · 2024-05-14T14:05:19Z

But the result of the function would still be a warning and taken as invalid, right?

Nope. It would be valid (return None, no warning) for paths starting with a single backslash.

abravalheri · 2024-05-14T14:12:56Z

Nope. It would be valid (return None, no warning) for paths starting with a single backslash.

That is weird... posixpath.isabs('\foo') should be false, ntpath.isabs('\foo') is now false in Python 3.13. Then the code in

         invalid = (
             os.path.pardir in path.split(posixpath.sep)
             or posixpath.isabs(path)
             or ntpath.isabs(path)
         )
         if not invalid:
             return

Should not return... Instead the function should reach the end with a warning...

Anyway, I think that the change you proposed make sense (and it avoids this problem). Thank you!

hroncok · 2024-05-14T14:16:33Z

Nope. It would be valid (return None, no warning) for paths starting with a single backslash.

That is weird... posixpath.isabs('\foo') should be false, ntpath.isabs('\foo') is now false in Python 3.13. Then the code in
         invalid = (
             os.path.pardir in path.split(posixpath.sep)
             or posixpath.isabs(path)
             or ntpath.isabs(path)
         )
         if not invalid:
             return
Should not return... Instead the function should reach the end with a warning...

Well, it is now False or False or Flase, which is False, which means it will return because not invalid is True.

Anyway, I think that the change you proposed make sense (and it avoids this problem). Thank you!

I'll send a PR.

hroncok · 2024-05-14T14:16:52Z

The second test failure reason is quite simple.

In here:

setuptools/setuptools/_entry_points.py

Lines 18 to 25 in 544b332

try:

ep.extras

except AttributeError as ex:

msg = (

f"Problems to parse {ep}.\nPlease ensure entry-point follows the spec: "

"https://packaging.python.org/en/latest/specifications/entry-points/"

)

raise OptionError(msg) from ex

setuptools expect AttributeError. importlib.metadata give AssertionError instead. Again, will check if this is intentional or not (however, throwing assertion errors for invalid inputes is bad IMHO).

python/importlib_metadata#488

abravalheri · 2024-05-14T14:18:35Z

Well, it is now False or False or Flase, which is False, which means it will return because not invalid is True.

Oh, of course... sorry, messed thoughts!

Thank you very much.

hroncok · 2024-05-14T14:22:32Z

#4356

The exception in importlib.metadata has changed. See python/importlib_metadata#488 This make an existing test pass with Python 3.13. Partially fixes pypa#4196

The exception in importlib.metadata has changed. See python/importlib_metadata#488 This makes an existing test pass with Python 3.13. Partially fixes pypa#4196

hroncok · 2024-05-14T14:26:23Z

And #4357

Previously, such paths were disallowed implicitly as they were treated as Windows absolute paths. Since Python 3.13, paths starting with a single backslash are not considered Windows-absolute, so we treat them specially. This change makes the existing doctest pass with Python 3.13. Partially fixes #4196

abravalheri · 2024-05-14T14:59:29Z

Thank you very much.

befeleme added bug Needs Triage Issues that need to be evaluated for severity and status. labels Jan 29, 2024

hroncok mentioned this issue May 14, 2024

Explicitly disallow resource paths starting with single backslash #4356

Merged

2 tasks

hroncok mentioned this issue May 14, 2024

Make the validation test for entry-points work with Python 3.13+ #4357

Merged

2 tasks

abravalheri closed this as completed in #4356 May 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] [doctest] pkg_resources.NullProvider._validate_resource_path and TestWriteEntries.test_invalid_entry_point fail with Python 3.13.0a3 #4196

[BUG] [doctest] pkg_resources.NullProvider._validate_resource_path and TestWriteEntries.test_invalid_entry_point fail with Python 3.13.0a3 #4196

befeleme commented Jan 29, 2024 •

edited

Loading

hroncok commented Jan 29, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024

hroncok commented May 14, 2024 •

edited

Loading

hroncok commented May 14, 2024 •

edited

Loading

os.path.isabs(path)

hroncok commented May 14, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024 •

edited

Loading

hroncok commented May 14, 2024

abravalheri commented May 14, 2024

hroncok commented May 14, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024

hroncok commented May 14, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024 •

edited

Loading

hroncok commented May 14, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024

[BUG] [doctest] pkg_resources.NullProvider._validate_resource_path and TestWriteEntries.test_invalid_entry_point fail with Python 3.13.0a3 #4196

[BUG] [doctest] pkg_resources.NullProvider._validate_resource_path and TestWriteEntries.test_invalid_entry_point fail with Python 3.13.0a3 #4196

Comments

befeleme commented Jan 29, 2024 • edited Loading

setuptools version

Python version

OS

Additional environment information

Description

Expected behavior

How to Reproduce

Output

hroncok commented Jan 29, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024

hroncok commented May 14, 2024 • edited Loading

hroncok commented May 14, 2024 • edited Loading

os.path.isabs(path)

hroncok commented May 14, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024 • edited Loading

hroncok commented May 14, 2024

abravalheri commented May 14, 2024

hroncok commented May 14, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024

hroncok commented May 14, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024 • edited Loading

hroncok commented May 14, 2024

hroncok commented May 14, 2024

abravalheri commented May 14, 2024

befeleme commented Jan 29, 2024 •

edited

Loading

hroncok commented May 14, 2024 •

edited

Loading

hroncok commented May 14, 2024 •

edited

Loading

abravalheri commented May 14, 2024 •

edited

Loading

abravalheri commented May 14, 2024 •

edited

Loading