Percent-encode ids and query values when building URLs

[samhashemi]

Summary

Ids are interpolated straight into the URL path with str.format, and the five query-string call sites build their query with f-strings. Neither encodes, so a value that isn't already URL-safe silently changes the request instead of being rejected cleanly.

Found while stress-testing the endpoints added in #35 — passing an id with a space to create_source_credential raised http.client.InvalidURL from deep inside the stdlib rather than returning a 404. The pattern is repo-wide and predates that PR; every module does it and none quote.

What goes wrong today

Input	Today	After
map_id="not a real id"	http.client.InvalidURL before any request	/maps/not%20a%20real%20id → 404
map_id="abc?x=1"	path truncated to /maps/abc, x=1 becomes a query param	/maps/abc%3Fx%3D1 → 404
map_id="abc#frag"	everything after # dropped	/maps/abc%23frag → 404
map_id="../../sources"	extra path segments, different endpoint	/maps/..%2F..%2Fsources → 404
user_email="a+b@example.com"	+ decodes server-side as a space	a%2Bb%40example.com
source="felt&admin=true"	injects a second query parameter	felt%26admin%3Dtrue

Approach

Two helpers in felt_python/api.py:

• build_url(template, **path_params) — fills the template with each value quoted as a single path segment (safe="", so / is encoded too)
• build_query(url, **params) — urlencodes query params and skips None, replacing the if x: url = f"{url}?x={x}" pattern

Then a mechanical conversion of all 42 path call sites and all 5 query-string sites across 8 modules.

Compatibility

Ordinary Felt ids — short slugs like PF0ve5FaSWujSB5402D8wD and UUIDs — contain no characters that need encoding, so request URLs are byte-identical for every existing caller. Behaviour only changes for ids that would previously have produced a wrong request or a stdlib exception.

Test plan

No token needed

python3 -m unittest tests.url_building_test -v

Expect: 14 tests, OK — covers segment encoding (space, ?, #, /, unicode), query encoding (+, &, None-skipping), and that ordinary slugs/UUIDs pass through byte-identical.

With a FELT_API_TOKEN (any workspace)

1. A malformed id is now a clean 404 instead of a stdlib crash:

   fromfelt_pythonimportget_mapget_map("not a real id")

   Before:http.client.InvalidURL: URL can't contain control characters raised before any request is sent.
   After:urllib.error.HTTPError: HTTP Error 404: Not Found.

2. An id can no longer silently escape the path. With a real map id:

   get_map(f"{REAL_MAP_ID}?x=1")

   Before: returns the map — the ?x=1 suffix silently became a query string, so you got a different resource than the id you passed.
   After:HTTP Error 404 — the suffixed id is one path segment and matches nothing.

3. Regression spot-check — one live suite that exercises rewritten call sites end-to-end (creates and deletes its own map):

   python3 -m unittest tests.maps_test

   Expect: passes; request URLs are byte-identical for well-formed ids, so all existing behavior is unchanged.

Already verified

All seven live suites (maps, layers — 13 rewritten call sites, the most of any module — sources, elements, layer_groups, library, projects) pass against felt.com on this branch. ruff format clean; ruff check reports only the same 14 pre-existing I001/RUF022 findings as unmodified main.

Not covered by a live test: comments.py's three call sites and the export query string, since the repo has no comments test — the unit tests cover their URL construction.

Merge order note

This branches off main, so it does not convert the 9 call sites in #35's new components.py and source-credential functions. Whichever merges second will need those converted — happy to rebase this on top of #35 instead if you'd rather do it in one pass.

🤖 Generated with Claude Code