Skip to content

fix: use tomli to parse toml file, prevent decode error for new toml syntax #910

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

Merged
merged 3 commits into from
Jun 18, 2025

Conversation

PTank
Copy link
Contributor

@PTank PTank commented Jun 16, 2025

Why:

If the pyproject.toml contain some new syntax ak the dictionnary inside a list, the actual toml lib used by pip audit fail to parse the line and raise IndexError

Example who not work with toml library.

commands = [["poetry", "run", "pytest", {replace = "posargs", default = [], extend=true}]]

How:

Change with tomli and tomli_w for the dump part.

@woodruffw
Copy link
Member

Hi @PTank, thanks for opening this. If I understand you correctly: this is a limitation in toml because it only supports v0.5 of the TOML specification, right?

Specifically, it looks like a variant of this: uiri/toml#270

Switching to tomli makes sense to me -- perhaps we could also switch to the stdlib's tomllib at some point, although that's probably a bit further down the line due to the Python version requirement + lack of writing support.

@woodruffw woodruffw added the dependencies Pull requests that update a dependency file label Jun 16, 2025
@woodruffw
Copy link
Member

P.S.: Do you have a full pyproject.toml I could reproduce this with? That would make it easy for us to add a regression/backstop test here 🙂

@PTank
Copy link
Contributor Author

PTank commented Jun 16, 2025

Hi @PTank, thanks for opening this. If I understand you correctly: this is a limitation in toml because it only supports v0.5 of the TOML specification, right?
Specifically, it looks like a variant of this: uiri/toml#270

Yes, and the toml last pypi update is 2020

perhaps we could also switch to the stdlib's tomllib at some point, although that's probably a bit further down the line due to the Python version requirement + lack of writing support.

I already tested it with a switch for the < 3.11, but we do not reach 100% coverage without tools like tox for the pipeline

lines like:

import sys

if sys.version_info >= (3, 11):
    import tomllib
else:
    import tomli as tomllib

P.S.: Do you have a full pyproject.toml I could reproduce this with? That would make it easy for us to add a regression/backstop test here 🙂

[project]
dependencies = [
  "flask==2.0.1"
]
[tool.other]
must_work = ["test", {"work" = true}] # toml.decoder.TomlDecodeError: Not a homogeneous array
must_work_too = ["test", {"work" = true, other_list = []}] # IndexError: list index out of range

I did not add this to the tests, because I did not want to test toml or tomli input here

@PTank
Copy link
Contributor Author

PTank commented Jun 17, 2025

Hi, for the lint error, maybe a change in mypy ?
but I can fix it with a result typed dict in _cli.py

# 547
result: dict[Dependency, list[VulnerabilityResult]] = {}

@woodruffw
Copy link
Member

Ah, don't worry about that -- I fixed that in another PR that hasn't landed yet. It's not related to these changes at all 🙂

@woodruffw woodruffw merged commit cfbe121 into pypa:main Jun 18, 2025
12 checks passed
@woodruffw
Copy link
Member

Thanks @PTank!

woodruffw added a commit that referenced this pull request Jun 18, 2025
Signed-off-by: William Woodruff <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants