Skip to content

Reject ISO 8601 durations with missing components - #494

Open
shkyyy18 wants to merge 1 commit into
sloria:mainfrom
shkyyy18:fix/reject-empty-iso-duration
Open

shkyyy18 wants to merge 1 commit into
sloria:mainfrom
shkyyy18:fix/reject-empty-iso-duration

Conversation

@shkyyy18

@shkyyy18 shkyyy18 commented Oct 1, 2026

Copy link
Copy Markdown

Summary

Env.timedelta(..., format="iso8601") currently accepts duration strings without any components (P, PT, signed/whitespace variants) as zero. It also accepts an empty time section after a day component (P1DT becomes one day). This silently accepts malformed configuration instead of raising EnvValidationError.

Require a component after P, and a time component whenever T is present. Valid zero durations such as P0W, P0D, and PT0S remain supported. The change is limited to two regex lookaheads; invalid strings fall through to the existing validation path.

Regression coverage

  • Nine malformed-duration cases: all fail on the unchanged upstream implementation and pass with the fix.
  • Six explicit-zero controls: pass both before and after.
  • No new dependency or API change.

Validation

Python 3.12.10, marshmallow 4.3.1, dependencies from the existing frozen lockfile:

  • New ISO 8601 regression cases: 15 passed.
  • Complete suite: 149 passed, 1 failed. The failure is the pre-existing Windows-only behavior of test_read_from_file_path_is_unreadable: chmod(0o000) does not make the file unreadable here. On the unchanged upstream checkout, the same test fails (134 passed, 1 failed). No unrelated permission test was modified.
  • Excluding only that baseline permission test: 149 passed, 1 deselected.
  • mypy --warn-unused-ignores: passed.
  • Configured pre-commit hooks on both changed files: Ruff check/format and blacken-docs passed; workflow, lockfile, and README-only hooks had no applicable files.
  • git diff --check: passed.

I have not run the full upstream multi-version/Linux CI matrix locally.

AI assistance was used to investigate, implement, and test this change.

This branch has not been deployed

No deployments
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.

1 participant