Skip to content

Out-of-order value-vs-table redefinition parses OK and only raises on access (should reject at parse) #523

Description

@tfoutrein

Summary

An out-of-order value-vs-table redefinition is invalid TOML, but tomlkit.parse() accepts it and the conflict only surfaces lazily on first access (raising KeyAlreadyPresent), instead of failing at parse time. The equivalent in-order document is correctly rejected at parse. Both are invalid per the TOML spec and per stdlib tomllib.

Minimal repro

import tomlkit, tomllib

src = '[a]\nb = true\n[zz]\n[a.b]\nc = 1\n'   # the splitter table [zz] may be empty

doc = tomlkit.parse(src)   # -> OK, no exception   (BUG: should raise here)

doc.unwrap()               # -> raises KeyAlreadyPresent: Key "b" already exists.
doc['a']                   # -> same raise (also .get('a'), 'a' in doc, dict(doc))
doc.value                  # -> SUCCEEDS but silently drops b=true:
                           #    {'a': {'b': {'c': 1}}, 'zz': {}}

# In-order twin is correctly rejected at parse:
tomlkit.parse('[a]\nb = true\n[a.b]\nc = 1\n')   # -> raises KeyAlreadyPresent AT PARSE

# Stdlib oracle agrees the input is invalid TOML:
tomllib.loads(src)         # -> tomllib.TOMLDecodeError: Cannot overwrite a value

Expected vs actual

  • Expected: tomlkit.parse(src) raises a TOMLKitError (e.g. ParseError / KeyAlreadyPresent) at parse time, matching the in-order form and tomllib.
  • Actual: parse() returns a document that looks healthy (len(doc) == 2, list(doc.keys()) == ['a', 'zz'], doc.as_string() round-trips byte-for-byte), but materializing key a raises KeyAlreadyPresent('Key "b" already exists.'). The raise is deterministic on every access (no caching of a good value).

Two failure modes from one defect

  1. Value-then-table split (above): parse() OK, then raise on access.
  2. Dotted-key-then-table splitstrictly worse, a silent false-accept:
    doc = tomlkit.parse('[a]\nb.c = 1\n[zz]\nq = 9\n[a.b]\nd = 2\n')
    doc.unwrap()   # NO error; silently merges to {'a': {'b': {'c': 1, 'd': 2}}, ...}
    # tomllib rejects this: "Cannot declare (a, b) twice"

Accessor divergence on the same conflicting document: doc.unwrap(), doc['a'], doc.get('a'), 'a' in doc, dict(doc) all raise, while doc.value succeeds with silent data loss (drops b = true).

Why it happens (root cause)

The in-order path raises synchronously in Container.append (the isinstance(item, Table) / not isinstance(current, (Table, AoT)) branch). On the out-of-order path, the existing entry for a is a concrete table and the new entry is a super-table wrapper for [a.b]. In Container.append the out-of-order validation (_validate_out_of_order_table, _validate_table_candidate) lives only inside the current.is_super_table() branch; because current here is concrete, that whole block is skipped, no validation runs, and the unvalidated out-of-order tuple is recorded. The clash is then only discovered when OutOfOrderTableProxy materializes that tuple (terminal raise if not isinstance(current, Table): raise KeyAlreadyPresent(key) in Container._raw_append) — i.e. at access, not parse.

Spec & conformance

  • TOML v1.0.0 and v1.1.0 both state: "Like keys, you cannot define a table more than once. Doing so is invalid." with the canonical [fruit] / [fruit.apple] example — exactly this value-then-table-reopen shape.
  • toml-test ships the in-order twin as invalid/table/redefine-01.toml ([a] / b = 1 / [a.b] / c = 2), and its contract requires decoders to reject invalid fixtures. There is no out-of-order (intervening-table) variant in the suite, so this exact permutation isn't exercised.
  • tomlkit's own suite already encodes parse-time rejection: tests/test_toml_tests.py runs the whole invalid/ suite under pytest.raises(TOMLKitError) on parse(), and tests/test_api.py::test_parse_rejects_collisions_across_out_of_order_fragments ([Security] Quadratic complexity DoS when parsing dotted keys with shared prefix (CWE-400) #479) demands a parse-time error for out-of-order collisions — but only for table-vs-table fragments, leaving value-vs-table out-of-order uncovered.

KeyAlreadyPresent is already a TOMLKitError subclass, so raising it at parse needs no new exception type — the fix is about when it raises (parse, not access).

Suggested fix direction

Validate the out-of-order concrete-existing + super-new (and dotted-value + super-new) cases during parse so invalid documents fail at parse(), consistent with the in-order path and with the project's recent direction (#509 "collisions across fragments still raise at parse time"; #516 "reject a table redefined after its parent table header").

Related

Environment: tomlkit 0.15.0 (also reproduces under 0.14.0 per #505); cross-checked against stdlib tomllib.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions