Skip to content

fix: report malformed BoxList bracket indices - #335

Open
arindamsikder wants to merge 1 commit into
cdgriffith:developfrom
arindamsikder:fix/malformed-bracket-path-errors
Open

arindamsikder wants to merge 1 commit into
cdgriffith:developfrom
arindamsikder:fix/malformed-bracket-path-errors

Conversation

@arindamsikder

Copy link
Copy Markdown

Summary

Raise a clear BoxTypeError when a dotted BoxList path has no numeric bracket index, instead of leaking AttributeError from a failed regex match.

Problem

With box_dots=True, reads, assignments and deletions such as items["[x]"] call groups() on None. The regex-shaped YAML key in the maintainer's comment on #265 reaches the same assignment path when loaded with default_box=True.

This addresses the requested diagnostic for that case. I did not reproduce the original segfault and am not claiming to resolve the entire issue.

Solution

  • Guard the unmatched regex in all three BoxList access methods and include the offending path in the error.
  • Keep the existing bracket grammar, valid-path behavior and frozen-write checks unchanged. No nested default-write or conversion redesign.
  • Cover malformed indices, default mode, nested access, frozen lists, disabled dot parsing and the YAML entry point. Check that rejected direct list writes/deletes leave the list unchanged.
  • Include the changelog and AUTHORS entries requested by CONTRIBUTING; target develop.

Testing

Executed offline in a credential-free Bubblewrap sandbox on Linux, CPython 3.11.15, Cython 3.3.0. Optional TOON dependency loaded from toon-format/toon-python commit 6034550ceec432adba37480c82839d8428a80361.

  • python -m pytest -q -p no:cacheprovider --basetemp=/tmp/pytest test/: 191 passed in pure Python and again with freshly built Cython modules; pristine base: 159 passed in each mode.
  • python -m pytest -q -p no:cacheprovider --basetemp=/tmp/pytest test/test_box_list.py test/test_box.py -k invalid with final tests and original runtime: 29 expected failures / 3 passing controls in each mode. Restored fix: full suites pass.
  • python -m pytest --cov=box -vv -p no:cacheprovider --basetemp=/tmp/pytest test/: 191 passed in each mode. Pure-Python coverage: 92%; the compiled build is not line-traced (1% reported), so compiled coverage is not claimed.
  • python -m black --check --config=.black.toml box test setup.py (24.10.0): passed.
  • python -m mypy --cache-dir=/tmp/mypy box (1.13.0): passed, eight source files.
  • Applicable configured pre-commit-hooks v5.0.0 entry points on changed files: passed; no YAML/JSON/TOML/requirements or executable files changed. Build/test/formatter/type hooks exercised through the commands listed here rather than pre-commit's environment installer.
  • CC=/usr/bin/x86_64-linux-gnu-gcc-15 python setup.py build_ext --inplace and python setup.py sdist bdist_wheel: passed.
  • python -m twine check --strict dist/python_box-7.4.1-cp311-cp311-linux_x86_64.whl dist/python_box-7.4.1.tar.gz: passed. Installed the built wheel offline and reran the full suite with imports verified from that installation: 191 passed.
  • Same packaging checks on the pristine base pass with the same setuptools deprecation and packaging warnings (including missing box.py/.pyd, disabled byte-compilation and git-file listing in the disposable non-git build copy).
  • git diff --check, added-line security scan and independent agent diff review: passed.

Other interpreters, Windows/macOS and manylinux wheel jobs were not run locally; upstream CI remains authoritative for those platforms.

Related Issue

Refs #265 — the malformed-bracket diagnostic remainder only, not an automatic issue closure.

AI disclosure: implemented and validated autonomously with Hermes DEV; independent review was by a separate agent, not a claim of human pre-review.

Guard unmatched index expressions in dotted reads, writes and deletes.
Add regressions for nested paths, default/frozen modes and YAML loading.

Independent review: passed (security and logic).
AI-assisted contribution prepared with Hermes DEV.
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