Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add additional check for project.license.file #725

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Feb 20, 2025

Closes #724

@@ -608,9 +608,13 @@ def read_pep621_metadata(proj, path) -> LoadedConfig:
raise ConfigError(
f"License file path ({license_f}) cannot be an absolute path"
)
if ".." in license_f:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps? Although, normpath just does lexical manipulation and e.g. a/spam/../b might refer to a symlink.

Suggested change
if ".." in license_f:
if ".." in os.path.normpath(license_f):

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, as e.g. a file might be named a..b. I think backslashes are prohibited?

Suggested change
if ".." in license_f:
if ".." in license_f.split('/'):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps? Although, normpath just does lexical manipulation and e.g. a/spam/../b might refer to a symlink.

Technically, yes. Although I'm not sure it's worth the effort. Same for file names which contain ...
For the overwhelming majority, checking if ".." in license_f: to provide a better error message should be enough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the check we do for paths in the sdist include/exclude lists:

normp = osp.normpath(p)
if isabs_ish(normp):
raise ConfigError(
f'{clude} pattern {p!r} is an absolute path'
)
if normp.startswith('..' + os.sep):
raise ConfigError(
'{} pattern {!r} points out of the directory containing pyproject.toml'
.format(clude, p)
)

I can't think of a use case for a path like foo/../bar (which will normalise to just bar), but it is currently allowed in that case. Whatever we do, I'd like it to be consistent across all the places where a relative path is specified.

@cdce8p cdce8p force-pushed the license-file-check branch from b9e7333 to 5f5982b Compare February 20, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

license.file failure in flit-core 3.11
3 participants