Skip to content

Null and duplicate top-level variable names handled asymmetrically across Expand overloads #60

Description

@brenpike

Found during a repeat repo-wide read-only correctness sweep (2026-08-23).

Severity: info. Requires an adversarial or unusual custom container to reach.

Problem

Top-level null and duplicate variable-name handling is not consistent across the Expand overloads.

Null names. The tuple overloads guard explicitly with ThrowIfKeyIsNull (src/Chatter.Rest.UriTemplates/UriTemplate.cs:250, :296, :315-323). The dictionary overloads do not (:93-97, :134-139). A custom IDictionary<string, object?> that enumerates a null key therefore reaches ordinal[kvp.Key] = ... and throws ArgumentNullException with paramName: "key" — naming a parameter that does not exist on the overload the caller invoked. That is precisely the wart ThrowIfKeyIsNull was written to prevent on the tuple paths.

Duplicate names. The tuple overloads document first-wins. The dictionary overloads are last-wins in the pathological case of a caller dictionary with a finer-than-ordinal comparer holding two ordinally-equal names.

Existing coverage

UriTemplateArgumentContractTests covers null keys inside composite values thoroughly, but not a null top-level name arriving via a custom dictionary.

Requested change

Guard kvp.Key is null in the two dictionary-copy loops using the same message shape as ThrowIfKeyIsNull, and settle the duplicate-name policy so all overloads agree. Non-breaking; patch-level.

Root-cause note

This shares a single root cause with the companion issue in this sweep about DictionaryValue collapsing comparer-distinct keys: argument-contract hardening was applied in waves (tuple overloads first, then the memoized-view seam) without re-auditing the older entry points against the new invariants.

The recommendation is to fix these two together by sweeping all five Expand entry points plus UriTemplateValue against one written contract covering null names, duplicate-name policy, and comparer preservation — rather than patching each site independently and inviting a third wave of the same drift.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions