Sprint review: URL templates from a spec may only name arguments; form requests send their fields

[thorwhalen]

Sprint review (2026-10-11 → 13) of #20, #22, #24, #27: adversarial review, design review, coverage first.

Findings fixed here

• A spec could read the caller's arguments into the URL. A request function fills its url_template with str.format(**arguments), and the template comes from the spec, which HttpClient may fetch from the server. A placeholder like {x.__class__} or {x.f.__globals__[KEY]} reads attributes of the caller's own arguments (for pickled object arguments, module globals) into the URL sent to that server. mk_request_function now refuses any placeholder, or query_arg_names entry, that looks up an attribute or item (., [, or a nested field in a format spec), via check_url_template, so a client built from such a spec fails at construction. Other names, such as OpenAPI's hyphenated {user-id}, still work. This is the threat model of Never unpickle binary responses unless explicitly asked (binary_loads= / binary_dumps= seams) #20 and Routes live in client.routes; a route name cannot replace a client attribute (#23) #24 (a spoofed or compromised server). No dependent in the local package fleet uses dotted or indexed placeholders.
• Multipart form requests sent nothing unless the method was GET. The form branch of request_func hung off if method != "GET" as an elif.

Reviewed, not defects

• Never unpickle binary responses unless explicitly asked (binary_loads= / binary_dumps= seams) #20: binary responses are never unpickled unless binary_loads= says so; check_loads warns on a bare pickle.loads; default_binary_output_trans now raises rather than unpickling.
• binary_loads passthrough for mk_cli and api_pkg_maker; the generated package compiles (#21) #22: generated packages load the spec as JSON data and only write validated identifiers into source. setup.cfg values are collapsed to one line with % escaped.
• Routes live in client.routes; a route name cannot replace a client attribute (#23) #24: a route cannot replace a client attribute; client.routes holds every route.
• Name routes without x-method_name; pin http2py floor in generated packages (#25, #26) #27: route names fall back to operationId, then method plus path, sanitised into identifiers. Duplicates warn (now tested).

Review

An independent adversarial review (Opus) found two should-fix problems, both now fixed and tested. First, query_arg_names bypassed the check. Second, refusing every non-identifier placeholder broke hyphenated {user-id} paths, which worked on master. It confirmed that the form fix never sends both data and files. On master, form fields were never sent through HttpClient at all, since it passes lowercase methods.

Tests

Characterisation tests first (they pass on master). Full suite: 109 passed (was 93). Coverage: decorators 88% → 90%, py2request 67% → 69%, total 66% → 67%. Most of client.py's remaining gap is the login/refresh flow, which the sprint did not touch.

🤖 Generated with Claude Code