fix(utils): reject path separators in wheel/sdist filenames by r266-tech · Pull Request #1337 · pypa/packaging

r266-tech · 2026-07-18T08:55:36Z

Summary

reject /, \\, and NUL characters in parse_wheel_filename() and parse_sdist_filename()
keep PackageWheel.filename and PackageSdist.filename non-raising for manually constructed objects
cover the public parsers directly and verify that percent-decoded pylock URL filenames flow through the existing package validation

Why

#1314 correctly percent-decodes the final URL path component, including valid names such as %2B local versions. The same decoding can also produce path-shaped values such as example/../evil-1.0.tar.gz.

The public filename parsers are the canonical validation boundary, and pylock already delegates wheel/sdist validation to them. Rejecting separators and NUL there protects pylock and other downstream callers consistently while preserving non-raising filename property access.

Testing

.venv/bin/python -m pytest tests/test_utils.py tests/test_pylock.py -q (148 passed)
uvx ruff check src/packaging/utils.py src/packaging/pylock.py tests/test_utils.py tests/test_pylock.py
uvx ruff format --check src/packaging/utils.py src/packaging/pylock.py tests/test_utils.py tests/test_pylock.py
git diff --check

sbidoul · 2026-07-18T12:06:52Z

    # The last path component is percent-encoded, so decode it to the file name
-    return unquote(url_path.rsplit("/", 1)[-1])
+    filename = unquote(url_path.rsplit("/", 1)[-1])
+    if "/" in filename or "\\" in filename or "\0" in filename:


And/or, reject os.sep, os.altsep ? That would make the behaviour platform dependent, though, so maybe not.

I'm thinking it should not be platform dependent? Not following closely though.

sbidoul · 2026-07-18T18:22:22Z

Please dismiss my approval. I think the filename property access should not raise. This should be handled at validation time.

r266-tech · 2026-07-19T00:48:14Z

Moved the separator/NUL check to parsing validation in b55ec2d.

PackageSdist.filename and PackageWheel.filename now remain non-raising for manually constructed objects, while _from_dict() (and therefore Pylock.from_dict() / validate()) rejects decoded URL filenames that are not a single component. The focused property/validation tests pass for both distribution types, along with Ruff and diff checks.

I cannot dismiss another user's approval as an external contributor, but the requested behavioral change is now on the branch.

sbidoul · 2026-07-19T08:51:39Z

Actually, we already call parse_wheel_filename and parse_sdist_filename in Package validation. So maybe these functions should be expanded to catch these cases? They are supposed to return a NormalizedName but don't in such case.

So I suggest closing this as it will be addressed by #873.

sbidoul · 2026-07-19T08:57:18Z

parse_sdist_filename accepts a/b-1.0.tar.gz.

parse_wheel_filename correctly raises InvalidWheelFilename on a/b-1.0-py3-non-any.whl.

sbidoul

We could add a pylock-specific test, but the validation must be done in parse_wheel_filename (should be ok) and parse_sdist_filename.

r266-tech · 2026-07-19T16:39:28Z

Reworked in 3636593 along the parser boundary you requested.

moved separator/NUL rejection into parse_wheel_filename() and parse_sdist_filename();
removed the pylock-specific validation helper, so PackageWheel.filename / PackageSdist.filename remain non-raising;
added direct parser regressions plus pylock integration coverage for percent-decoded /, \\, and NUL values.

Focused verification: 148 tests passed, Ruff check/format passed, and git diff --check passed.

sbidoul · 2026-07-20T07:30:03Z

    """
+    if "/" in filename or "\\" in filename or "\0" in filename:
+        raise InvalidWheelFilename(f"Invalid wheel filename: {filename!r}")
+


This change is unnecessary. Something in this function already catches this.

I kept the whole-filename guard after finding that the existing parser accepts these characters outside the project-name field—for example, example-1.0-1/../evil-py3-none-any.whl parses with /../evil as the build suffix, and separators are also accepted in tag components. a50fa5e moves the regression cases into those build/tag positions so the guard's necessity is covered.

Interesting find. Still I would rather address that by refining parse_tag() and _build_tag_regex. I've not checked, but surely the specification for these things do not allow arbitrary characters?

Implemented that parser-boundary approach in fcfaa6f.

parse_tag() now rejects non-ASCII or otherwise invalid interpreter/ABI/platform components;

_build_tag_regex now uses an ASCII-digit prefix plus fullmatch(), rejecting /, \\, and NUL while preserving the existing free-form suffix behavior (covered by a positive 1+vendor case);

parse_sdist_filename() continues to validate the name through canonicalize_name(..., validate=True) rather than a separate character list.

I removed the whole-wheel-filename guard, rebased the unchanged patch onto current main, and reran the focused parser/Pylock suite: 384 tests passed, with Ruff and diff checks clean. The full suite on the same patch also passed previously (62,360 passed, 1 platform skip).

sbidoul · 2026-07-20T07:36:34Z

    .. _Source distribution format: https://packaging.python.org/specifications/source-distribution-format/#source-distribution-file-name
    """
+    if "/" in filename or "\\" in filename or "\0" in filename:
+        raise InvalidSdistFilename(f"Invalid sdist filename: {filename!r}")


If hard coding these characters is unnecessary for wheel filenames, then there is probably a better way to do it for sdist filenames.

Replaced the sdist character list with canonicalize_name(name_part, validate=True), translating InvalidName to InvalidSdistFilename. a50fa5e adds general invalid-name/cause coverage plus path and %00 Pylock cases.

sbidoul · 2026-07-21T08:16:25Z

+    try:
+        name = canonicalize_name(name_part, validate=True)
+    except InvalidName as e:
+        raise InvalidSdistFilename(f"Invalid project name: {filename!r}") from e


Suggested change

raise InvalidSdistFilename(f"Invalid project name: {filename!r}") from e

raise InvalidSdistFilename(f"Invalid sdist filename (invalid project name): {filename!r}") from e

sbidoul · 2026-07-21T08:32:27Z

    """


+_tag_component_regex = re.compile(r"\w+", re.ASCII)


PEP 425 says The platform tag is simply distutils.util.get_platform() with all hyphens - and periods . replaced with underscore _. In its FAQ it has Why normalise hyphens and other non-alphanumeric characters to underscores?.

So I suppose this regex is ok?

I think so based on the definition of \w from the re docs.

sbidoul · 2026-07-21T08:37:55Z

            raise InvalidTag(f"Tag {tag!r} has an empty component: {component!r}")
+        for part in parts:
+            if _tag_component_regex.fullmatch(part) is None:
+                raise InvalidTag(f"Tag {tag!r} has an invalid component: {part!r}")


Suggested change

raise InvalidTag(f"Tag {tag!r} has an invalid component: {part!r}")

component = ".".join(parts)

raise InvalidTag(f"Tag {tag!r} has an invalid component: {component!r}")

?

sbidoul · 2026-07-21T08:51:13Z

In addition to the PR title, the changelog should also mention that parse_tag() becomes stricter.

sbidoul · 2026-07-21T09:13:14Z

-
-    name = canonicalize_name(name_part)
+    try:
+        name = canonicalize_name(name_part, validate=True)


While this sounds correct, parse_wheel_filename does not use canonicalize_name(validate=True) and accepts non ascii characters (foo_bár is tested), which are rejected by canonicalize_name(validate=True) and is_normalized_name(). However parse_wheel_filename is typed to return NormalizedName.

PEP 427 explicitly allows non ascii characters in the project name: The archive filename is Unicode. It will be some time before the tools are updated to support non-ASCII filenames, but they are supported in this specification.

So there is a discrepancy that's probably worth discussing separately.

I opened #1350 to discuss that.

r266-tech · 2026-07-21T17:20:57Z

Added the requested changelog entry; it now explicitly calls out that parse_tag() rejects invalid tag components.

A post-change adversarial review also found one remaining cross-platform filename edge: : in the wheel build suffix can address an NTFS alternate data stream. The parser now rejects that delimiter while retaining the existing free-form suffix behavior for values such as 1+vendor; direct parser and percent-decoded %3A Pylock regressions cover it. The focused suite passes (386 tests), and all changed-file pre-commit hooks pass, including Ruff, mypy, RST checks, and typos. CI is running on b6910f3.

sbidoul reviewed Jul 18, 2026

View reviewed changes

sbidoul approved these changes Jul 18, 2026

View reviewed changes

sbidoul suggested changes Jul 19, 2026

View reviewed changes

r266-tech force-pushed the r266/pylock-url-filename-component branch from b55ec2d to 3636593 Compare July 19, 2026 16:38

henryiii changed the title ~~fix(pylock): reject path separators decoded from artifact URLs~~ fix(utils): reject path separators in wheel/sdist filenames Jul 20, 2026

sbidoul reviewed Jul 20, 2026

View reviewed changes

r266-tech and others added 4 commits July 21, 2026 09:01

fix(pylock): reject path separators decoded from artifact URLs

961bca3

fix(utils): reject path-shaped distribution filenames

0bf82f4

fix(utils): use canonical filename validation

27a6e67

Refine wheel filename validation boundaries

fcfaa6f

r266-tech force-pushed the r266/pylock-url-filename-component branch from a50fa5e to fcfaa6f Compare July 21, 2026 01:02

sbidoul reviewed Jul 21, 2026

View reviewed changes

r266-tech added 2 commits July 22, 2026 00:47

docs: document stricter filename parsing

dd23896

fix(utils): reject NTFS stream delimiters

b6910f3

	raise InvalidSdistFilename(f"Invalid project name: {filename!r}") from e
	raise InvalidSdistFilename(f"Invalid sdist filename (invalid project name): {filename!r}") from e

	raise InvalidTag(f"Tag {tag!r} has an invalid component: {part!r}")
	component = ".".join(parts)
	raise InvalidTag(f"Tag {tag!r} has an invalid component: {component!r}")

Uh oh!

Conversation

r266-tech commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbidoul commented Jul 18, 2026

Uh oh!

r266-tech commented Jul 19, 2026

Uh oh!

sbidoul commented Jul 19, 2026

Uh oh!

sbidoul commented Jul 19, 2026

Uh oh!

sbidoul left a comment

Choose a reason for hiding this comment

Uh oh!

r266-tech commented Jul 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbidoul Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbidoul commented Jul 21, 2026

Uh oh!

sbidoul Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

r266-tech commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

r266-tech commented Jul 18, 2026 •

edited

Loading

sbidoul Jul 20, 2026 •

edited

Loading

sbidoul Jul 21, 2026 •

edited

Loading