diff --git a/RECOMMENDATIONS.md b/RECOMMENDATIONS.md deleted file mode 100644 index 592c104..0000000 --- a/RECOMMENDATIONS.md +++ /dev/null @@ -1,722 +0,0 @@ -# UCE Code Review — Full Findings (2026-06-11) - -Scope: the pending working-tree changes (73 modified tracked files plus new -untracked sources — notably `src/lib/sqlite-connector.cpp/.h` and the -uce-starter theme/router rework). - -Method: seven independent review angles (line-by-line diff scan, -removed-behavior audit, cross-file call tracing, reuse, simplification, -efficiency, altitude), followed by a verification pass on the correctness -candidates. Status legend: - -- **Confirmed** — verified against the working tree, decisive lines quoted. -- **Plausible** — surfaced by a review angle, not individually re-verified. -- **Refuted** — investigated and found not to be an issue (kept here so - nobody re-flags it). - ---- - -## Part 1 — Correctness (all confirmed) - -### 1.1 XSS: island props break out of single-quoted attribute - -**File:** `site/examples/uce-starter/components/theme/web_affordances.uce:12` - -The island component emits JSON props inside a single-quoted attribute -(`data-props=''`), but `html_escape()` -(`src/lib/functionlib.cpp:936`) escapes only `& < > "`, and `json_encode` / -`json_escape` (`src/lib/dvalue.cpp:794`) never escape apostrophes. Any `'` in a -prop value terminates the attribute. - -**Failure:** a view passes user-influenced prop text containing an apostrophe, -e.g. `x' autofocus onfocus='alert(1)` — the attribute terminates early and -attacker-controlled attributes/event handlers land on the div. Even benign -values like `Don't` silently truncate the payload. - -**Fix:** use a double-quoted attribute (html_escape covers `"`), or extend -`html_escape` to escape `'` as `'`. - -**Status:** fixed, but fix not verified yet. - -### 1.2 Crash: positional `?` placeholders in sqlite_query - -**File:** `src/lib/sqlite-connector.cpp:86` - -`bind_params` constructs a `String` directly from -`sqlite3_bind_parameter_name(stmt, i)`, which returns NULL for nameless -positional `?` parameters. `std::string(nullptr)` is UB and crashes before the -`if(name == "") continue;` guard on the next line can run. - -**Failure:** any page calling -`sqlite_query(db, "select * from notes where id = ?", params)` with a bare `?` -instead of `:name` segfaults the worker (500 recovery page). - -**Fix:** remove support for positional placeholders from sqlite and mysql API in favor of named placeholders, adjust the documentation accordingly. - -**Status:** fixed, but fix not verified yet. - -### 1.3 HTTP parsing: split_http_headers assumes lines[0] is the request line - -**File:** `src/lib/functionlib.cpp:754` - -The rewritten parser unconditionally treats the first line as the HTTP request -line. The old parser located the first colon-free line, which tolerated a -leading CRLF (RFC 7230 §3.5) and header-only input. Two failure modes: - -- The direct-HTTP caller (`src/fastcgi/src/fcgicc.cc:693`) passes the raw - buffer unstripped — a client sending a stray leading CRLF before - `GET /x.uce HTTP/1.1` gets the request line silently dropped (empty - `REQUEST_METHOD`, request fails). -- Header-only input (`Host: example.test\nX-Token: abc`, e.g. page code - parsing a raw header block) parses `Host:` as `REQUEST_METHOD` and loses - `HTTP_HOST` entirely. The function is directly callable from .uce pages. - -**Fix:** skip leading empty lines before consuming the request line, and only -treat a colon-free first line as the request line. - -**Status:** fixed, but fix not verified yet. - -### 1.4 Silent data loss: multi-statement SQL drops everything after the first `;` - -**File:** `src/lib/sqlite-connector.cpp:178` - -`SQLite::query` passes `0` for `pzTail` to `sqlite3_prepare_v2` and never -checks for remaining SQL, so multi-statement strings execute only the first -statement and still report `ok`. - -**Failure:** a migration page runs -`sqlite_query(db, "create table t(id integer); insert into t values(1);")` — -only the CREATE executes, the INSERT is silently dropped, `sqlite_error()` -says `ok`, leaving the database half-migrated with no error signal. - -**Fix:** capture `pzTail` and either loop over remaining statements or raise -an error when trailing SQL is present. - -**Status:** fixed, but fix not verified yet. - -### 1.5 Diagnostics regression: fault backtrace captured after siglongjmp - -**File:** `src/linux_fastcgi.cpp:895` - -The in-handler `capture_backtrace_string` call (`request_fault_trace`) was -deleted; the trace is now captured after `siglongjmp` back in -`handle_complete`, where the faulting stack has already been unwound. No -capture remains in `on_request_fault_signal`. - -**Failure:** a `.uce` page null-derefs → SIGSEGV → the error page's Trace -shows only `handle_complete`/`main` frames. The faulting unit's frames are -gone, making crash reports undiagnosable beyond the signal number (the README -still advertises a native backtrace of the failure). - -**Fix:** restore the capture inside `on_request_fault_signal` (on the faulting -stack) and hand the result across the longjmp, as the previous code did. - -**Status:** fixed, but fix not verified yet. - -### 1.7 Memory leak: SQLite wrapper objects never freed by request cleanup - -**File:** `src/lib/sqlite-connector.cpp:248` - -`cleanup_sqlite_connections()` closes the raw `sqlite3` handles tracked in -`resources.sqlite_connections` but never deletes the heap-allocated `SQLite` -wrappers from `sqlite_connect` (`new` without `delete`; no arena allocator is -compiled in). `sqlite_disconnect` itself is correct (`delete db` after -unregistering). Note: this mirrors a pre-existing identical leak in the MySQL -connector (`cleanup_mysql_connections`, `mysql-connector.cpp:298-304`). - -**Failure:** a page calls `sqlite_connect()` per request and relies on -end-of-request cleanup instead of `sqlite_disconnect()` — exactly what the -cleanup path exists for. One wrapper leaks per request; unbounded RSS growth -in the long-lived FastCGI worker. - -**Fix:** track the wrapper objects (not raw handles) in -`resources.sqlite_connections` and delete them in cleanup; fix the MySQL -connector the same way, or factor a shared registry (see 5.1). - -**Status:** fixed, but fix not verified yet. - -### 1.8 Asset shims emit stylesheets mid-`` - -**File:** `site/examples/uce-starter/components/example/marketing_assets.uce:8` -(also `theme_assets.uce`, `gauges/assets.uce`) - -The new ONCE-based asset shims fire inside the view's `ob_start()` capture in -`index.uce` (lines 67-85: view is captured into `fragments["main"]`, then -`themes/page` renders), so their `` output is baked -into the main fragment and spliced into the content div — inside ``, -not ``. - -**Failure:** every marketing/theme/gauges page ships its stylesheet mid-body -(FOUC, invalid-ish markup). The deleted `page_shell` asset registry rendered -these in ``. While this behavior is often okay in practice, we should -improve on this to encourage cleaner output. - -**Fix:** Part A: restore a registration mechanism: record component and asset output -that's intended as once per page in context.call["fragments"]["once"] by default -and the page template component can then explicitly slot this in where appropriate. - -Part B: Modify the preprocessor so directives support attributes like this: -ONCE(Request& context) -@fragment my-fragment-name -{ - ... -} - -Which will then automatically (in this example) slot the output into -context.call["fragments"]["my-fragment-name"] instead of the default slot name "once". -In the future we'll introduce more attributes with this syntax. Using this flexible mechanism for the fragment slot -name, we can leave it up to the page template where to slot in what. ONCE, COMPONENT, -and RENDER should support the fragment attribute. - -**Status:** fixed, but fix not verified yet. - -### 1.9 Error page "Generated C++" hint prints a nonexistent path - -**File:** `src/linux_fastcgi.cpp:79` - -`render_request_failure` computes the hint as -`path_join(BIN_DIRECTORY, SCRIPT_FILENAME) + ".cpp"`, but `path_join` returns -an absolute child unchanged (`src/lib/sys.cpp:179`), while the compiler builds -the real path by plain concatenation `BIN_DIRECTORY + src_path` -(`src/lib/compiler.cpp:631-636`). SCRIPT_FILENAME is always absolute in the -FastCGI path. - -**Failure:** every runtime failure page prints e.g. -`Generated C++: /var/www/site/page.uce.cpp` — the BIN_DIRECTORY prefix is -silently dropped and the path never exists (real file: -`/var/cache/uce/work/var/www/site/page.uce.cpp`). - -**Fix:** export one helper from `compiler.cpp` that maps a source file to its -generated artifact (`su->pre_path + "/" + su->pre_file_name` — the new -`compiler_format_compile_failure` in this same changeset already computes it -correctly) and use it in both places. - -**Status:** fixed, but fix not verified yet. - -### 1.10 O(n²) and per-row deep copies in dv_map / dv_filter - -**File:** `src/lib/functionlib.cpp:182` - -Both helpers call `tree.is_list()` inside the per-element callback — -`is_list()` (`src/lib/dvalue.cpp:165`) walks the entire map, making the -operation O(n²) — even though both already compute `is_list()` once before -the loop. Additionally, `DValue::each` (`dvalue.h:22`) takes the callback -element by value (`DValue t`), deep-copying every subtree per iteration. - -**Failure:** mapping over a 1000-row `sqlite_query` result does ~1M -key-validation checks plus a full deep copy of each row tree. - -**Fix:** hoist `is_list()` into a `bool` local reused in the callback; change -`each()` to pass `const DValue&`. - -**Status:** fixed, but fix not verified yet. - -### 1.11 Proactive compiler child can std::terminate on lock failure - -**File:** `src/lib/compiler.cpp:288` - -`compiler_with_registry_lock` now throws `std::runtime_error` when the -registry lock file can't be opened (previously: warning + proceed unlocked). -`run_proactive_compiler` (`src/linux_fastcgi.cpp:1270-1372`) calls -`compiler_list_known_units` / `compiler_set_known_units` with no try/catch -anywhere in the chain, so a transient failure (fd exhaustion, disk full) -aborts the forked child via `std::terminate`. - -**Blast radius is small:** the proactive compiler runs in a forked child that -the main loop respawns each iteration, and request-path callers are protected -by the try/catch in `handle_complete`. Still, a catch-and-log around the -proactive scan is cheap insurance. - -**Status:** fixed, but fix not verified yet. - ---- - -## Part 2 — Reuse / duplication (plausible unless noted) - -### 2.1 request_query_path() duplicates request_query_route() - -**File:** `src/lib/uri.cpp:311` - -`request_query_path()` copy-pastes `request_query_route()`'s (line 325) -first-keyless-segment scan verbatim — two identical "find first &-part -without =" loops plus `route_path_sanitize` calls that must be kept in sync. -It also has **zero callers** in `src/` or `site/` (only a doc page references -it). - -**Fix:** implement as -`return request_query_route(context, default_path)["l_path"].to_string();`. - -**Status:** fixed, but fix not verified yet. - -### 2.2 list_filter / list_map re-implement the generic filter / map templates - -**File:** `src/lib/functionlib.cpp:71` - -`StringList` is `typedef std::vector` (`types.h:52`), so the existing -`filter` template (`functionlib.h:68`, declared ~10 lines above the new -`list_*` declarations) already does exactly what `list_filter` does. Two -filter implementations now live in the same module; behavior fixes must be -applied twice. - -**Fix:** drop it and point the doc page at `filter`); same consideration for `list_map`. - -**Status:** fixed, but fix not verified yet. - -### 2.3 Hand-rolled redirects instead of the redirect() helper - -**Files:** `site/examples/uce-starter/views/account/login.uce:13`, -`logout.uce:6`, `profile.uce:8`, `site/demo/sqlite.uce:21-22` - -Four call sites inline `context.set_status(302, "Found"); -context.header["Location"] = ...` instead of calling the existing runtime -helper `redirect(String url, s32 code = 302)` (`src/lib/uri.cpp:414`). This -replaced the single `app_redirect()` helper the changeset deleted. - -**Note — security aspect refuted (see 6.1):** headers are sanitized centrally -on write-out, so this is a pure reuse nit, not a vulnerability. - -**Fix:** use `redirect(app_link("account/profile", context))` or restore one -`app_redirect` wrapper. - -**Status:** fixed, but fix not verified yet. - -### 2.5 Asset tag boilerplate repeated across ~a dozen files - -**Files:** `components/example/marketing_assets.uce:5`, -`components/example/theme_assets.uce`, `components/gauges/assets.uce`, -plus inline `` -blocks in `components/theme/head.uce`, `components/data/widgets.uce`, -`components/workspace/primitives.uce`, `views/dashboard.uce` - -About a dozen hand-written copies of the stylesheet/script tag pattern around -`app_asset_url()`; a versioning or attribute change (defer/integrity) touches -every file. The marketing and theme shims differ only in one CSS path. - -**Fix:** collapse the three shims into one parameterizable assets component. - -**Status:** fixed, but fix not verified yet. - ---- - -## Part 3 — Simplification (plausible) - -### 3.1 request_query_route() emits a derivable "rejected" flag - -**File:** `src/lib/uri.cpp:347` - -The route tree contains both `route["valid"]` and `route["rejected"]` where -`rejected` is exactly `!valid`, and nothing in the codebase reads -`"rejected"`. Redundant derivable state doubles the invariant surface — a -future path that sets one flag without the other produces a route tree that -lies. - -**Fix:** drop the `"rejected"` key; callers needing it can write -`!route["valid"].to_bool()`. - -**Status:** fixed, but fix not verified yet. - -### 3.2 Starter router carries write-only diagnostic state - -**File:** `site/examples/uce-starter/index.uce:38` - -The router rewrite adds candidate `kind`/`matched` fields and -`context.call["route"]["candidates"]/["resolved"]/["missed"]`, none of which -is read by any component, theme, or test. It also uses `dv_filter` to -`file_exists`-stat every candidate after the first match is already found. -~55 lines plus a builder helper replace what the deleted `app_resolve_view` -did with three early-return `file_exists` checks. - -**Fix:** remove unused parts. - -**Status:** fixed, but fix not verified yet. - -### 3.4 Single-link shim components where a direct ONCE block suffices - -**File:** `site/examples/uce-starter/components/example/marketing_assets.uce:8` -(and `theme_assets.uce`) - -These are single-`` shim files with empty COMPONENT bodies, invoked via -`print(component(...))` of an empty string. Routed views are mutually -exclusive per request, so the cross-unit dedup the shims provide can never -trigger; they exist only to host one ONCE line at the cost of two extra files -and an indirection on every view. `dashboard.uce:3-6` already demonstrates -the simpler form (ONCE block directly in the view). `gauges/assets.uce` is -the justified case (multiple sibling components per page) and can stay. - -**Status:** fixed, but fix not verified yet. - -### 3.5 Compile-failure artifact file is self-referential - -**File:** `src/lib/compiler.cpp:847` - -`compile_shared_unit()` overwrites `su->compiler_messages` with the formatted -failure report, then writes that report into `compile_output_file_name` — the -same artifact the report's own "Compile output:" line -(`compiler_format_compile_failure`, line 800) points readers at. No copy of -the raw, unformatted compiler output survives for tooling to parse. - -**Fix:** keep raw messages as the stored/recorded artifact content and format -only at the print/response boundary. - -**Status:** fixed, but fix not verified yet. - ---- - -## Part 4 — Efficiency (plausible unless noted) - -### 4.1 QUERY_STRING parsed twice per request - -**File:** `src/linux_fastcgi.cpp:744` - -`prepare_request_body_maps` calls `request_populate_context_params` (which -splits QUERY_STRING on `&` + uri-decodes inside `request_query_route`) and -then `parse_query(QUERY_STRING)` re-splits and re-decodes the identical -string on the next line. Runs on every HTTP request, CLI invocation, and -websocket event. Route params are also computed eagerly for requests that -never read `ROUTE_*`/`BASE_URL`. - -**Fix:** parse QUERY_STRING once into `request.get` first and derive the -route token from that single pass, or populate the route params lazily. - -**Status:** fixed, but fix not verified yet. - -### 4.2 Repeated normalization in request_populate_context_params - -**File:** `src/lib/uri.cpp:351` - -Per request: `route_path_normalize` runs three times on the same path -(directly, inside `route_path_sanitize`, and inside `route_path_is_safe`), -`request_script_url` is computed twice (for SCRIPT_URL and again inside -`request_base_url`), and `route_path_normalize` (line 255) strips slashes via -`path = path.substr(1)` in a while loop — O(n²) copies per slash run. - -**Fix:** normalize once and pass the normalized string down; compute -script_url into a local used by both params; replace the substr loops with -`find_first_not_of("/")` / `find_last_not_of("/")` and a single substr. - -**Status:** fixed, but fix not verified yet. - -### 4.4 collect_rows rebuilds column-name strings for every row - -**File:** `src/lib/sqlite-connector.cpp:118` - -For R rows × C columns the row loop does R×C `sqlite3_column_name` calls plus -R×C String heap constructions and map inserts keyed on the fresh string, -though column names are invariant across rows. A 10k-row, 8-column result -builds 80k redundant name strings per query. - -**Fix:** build a `std::vector names(column_count)` once before the -step loop and index it inside (`row[names[i]]`). - -**Status:** fixed, but fix not verified yet. - -### 4.5 bind_params copies the params map twice and binds SQLITE_TRANSIENT - -**File:** `src/lib/sqlite-connector.cpp:95` - -`SQLite::query` takes `StringMap` by value and passes it by value again to -`bind_params`, then binds with `SQLITE_TRANSIENT`, forcing SQLite to memcpy -each value a third time — even though the copied map outlives the statement -(it lives until after `sqlite3_finalize`). - -**Fix:** pass `const StringMap&` through `query()`/`bind_params` and bind -with `SQLITE_STATIC` (or at least drop the two by-value map copies). - -**Status:** fixed, but fix not verified yet. - -### 4.6 Per-request connection opens re-run pragmas; busy_timeout set twice - -**File:** `src/lib/sqlite-connector.cpp:15` - -`connect()` calls `sqlite3_busy_timeout(5000)` and then -`apply_default_pragmas` runs `PRAGMA busy_timeout = 5000` again (pure -duplicate). `PRAGMA journal_mode = WAL` is persistent in the database file -but re-issued on every open. Because cleanup closes all handles at request -end, a page like `site/demo/sqlite.uce` pays `sqlite3_open_v2` + 4 pragmas + -`CREATE TABLE IF NOT EXISTS` on every request. - -**Fix:** drop the redundant busy_timeout pragma; cache open -connections per worker keyed by path across requests (resetting state at -request end) so pragmas and schema checks run once per worker. - -**Status:** fixed, but fix not verified yet. - ---- - -## Part 5 — Altitude / design (plausible unless noted) - -### 5.3 Test suites hand-registered in three parallel lists — already drifting - -**Files:** `site/tests/index.uce:15`, `tests/plugins/uce_site_suite.py` - -A test page must be registered in three places: the `site/tests/*.uce` file -itself, a `site_tests_card()` line in `index.uce`, and a tuple in -`uce_site_suite.py`. The new `sqlite.uce` was added to all three by hand. The -lists have already drifted: `site/tests/call_helpers.uce` exists on disk but -appears in neither `index.uce` nor any plugin list, and -`security_headers.uce` is covered only by the security-smoke plugin, not the -index cards — new suites can silently fall out of the dashboard and/or CI. - -**Fix:** enumerate `site/tests/*.uce` with ls()/glob in both `index.uce` and -`uce_site_suite.py`, with title/tags metadata declared once (in the test page -or a single shared manifest). - -**Status:** fixed, but fix not verified yet. - -### 5.4 compiler_developer_hints() couples the runtime to clang's English message text - -**File:** `src/lib/compiler.cpp:778` - -The hint table pattern-matches hardcoded English clang diagnostic substrings -("no member named", "expected ';'", ...). A clang version bump, a switch to -gcc (COMPILE_SCRIPT is user-configurable server config), or localized -diagnostics silently degrades every hint to the generic fallback, and each -new error class means hand-extending an if-chain in runtime C++. - -**Fix:** the excerpt mechanism added in the same change -(`compiler_format_compile_failure`'s source/generated excerpts + artifact -paths) already carries the diagnostic value; make the hints a data-driven -table or drop them in favor of the excerpts. - -**Status:** fixed, but fix not verified yet. - -### 5.5 Generated-artifact path computed by hand in two places - -**File:** `src/linux_fastcgi.cpp:79` vs `src/lib/compiler.cpp:631-636` - -The deeper-fix framing of 1.9: the moment the compiler's artifact layout or -naming changes (hashing, per-config subdirs), any hand-recomputed path goes -stale. One exported source-file → artifact-path helper in `compiler.cpp`, -used by both the compile-failure formatter and the runtime failure page. - -**Status:** fixed, but fix not verified yet. - ---- - -## Part 7 — Verification of the fixes (review pass, 2026-06-11) - -All fixes above were re-reviewed against commit `7f75765` and, where possible, -verified live against the dev server (network suite: 21/21 public tests pass). - -> **Status update (later the same day):** 7.1–7.8 are fixed, built, deployed, -> and verified — full suite including internal tests passes 76/76, once-init -> renders again, dashboard/workspace ship their ONCE assets (now asserted by -> the smoke suite), and a new `uce_demo_smoke.py` plugin covers every -> `site/demo/*.uce` page. Unit ABI bumped to 3 (the @fragment prelude now -> instantiates the shared `UceFragmentCapture` from `functionlib.h`). Two notes: -> -> - Fixing 7.3 exposed a latent bug: on a failed `mysql_real_connect`, the old -> code left `connection` pointing at the freed handle, and `error()` read the -> error message out of freed memory — that's the only reason the services -> page's "skip when no MySQL" branch ever worked. `connect()` now nulls the -> handle and signals failure via `_preload_next_error_code`, and `query()` / -> `error()` guard against a null connection (clean error message instead of -> the SIGSEGV this otherwise caused). -> - The 7.8 nit about the `assets.uce` lambda taking `DValue` by value is -> withdrawn: `DValue::to_string()` is non-const, so a `const DValue&` parameter -> cannot call it. Making the DValue read accessors const-correct would be the -> real fix and is a separate, larger change (worth doing before the WASM -> DValue C ABI freezes the surface). - -### Verified clean - -**1.1** (double-quoted attr + `html_escape` escapes `'`, regression test), -**1.4** (multi-statement rejection is comment/whitespace-tolerant, test), -**1.9/5.5** (`compiler_generated_cpp_path` agrees with `setup_unit_paths`; -confirmed live — the error page now prints the real `/tmp/uce/work/...` path), -**1.10** (`each`/`push` take `const DValue&`, `is_list()` hoisted), -**1.11** (both the initial scan and the loop body are wrapped), -**2.1, 2.2, 2.3, 2.5/3.4, 3.1, 3.2, 3.5, 4.1, 4.2, 4.4, 4.5** (SQLITE_STATIC -is safe: the params map outlives the statement), **5.3, 5.4**. - -### 7.1 REGRESSION (live): ONCE output captured into a slot nobody prints - -**Files:** `src/lib/compiler-parser.cpp` (default slot `once`), -`components/theme/head.uce:23` - -`ONCE` now defaults to `@fragment once`, but no theme prints -`context.call["fragments"]["once"]`. Verified live: the dashboard page serves -without `views/dashboard.css`, the widgets page without any ag-grid assets — -the ONCE blocks in `views/dashboard.uce`, `components/data/widgets.uce`, and -`components/workspace/primitives.uce` are silently swallowed. The suite passed -anyway because it only checks body markers, not asset tags. - -**Fix:** print `fragments["once"]` in `head.uce` (next to `fragments["head"]`), -or convert those three ONCE blocks to `@fragment head`. Also worth adding an -asset-tag assertion to the dashboard smoke test so this can't regress silently. - -### 7.2 REGRESSION (live): fragment rewriter fires on literal text and duplicates lines - -**File:** `src/lib/compiler-parser.cpp` (`compiler_rewrite_fragment_attributes`) - -The rewriter is a line-based pre-pass with no literal awareness, so any literal -HTML line starting with `ONCE(` / `RENDER(` / `COMPONENT(` is treated as an -entry point. Verified live: `/demo/once-init.uce` is currently a compile error -("extraneous closing brace") because the heading text `ONCE() and INIT()` -matches. Two compounding defects: - -- When the entry line has no `{`, the code scans forward across arbitrary - lines to inject the prelude into whatever `{` it finds next. -- If no `{` is ever found, the inner loop emits the remaining lines but never - advances `i`, so the outer loop emits them all a second time. - -**Fix (minimal):** only treat a match as an entry point when the `{` is on the -entry line or the immediately following non-`@`-attribute line, and advance -`i = j` when the forward scan exhausts. **Fix (right altitude):** literal -tracking lives in the char-wise pass — fold fragment rewriting into it -eventually; `compiler_rewrite_named_render_syntax` shares the same blind spot. - -### 7.3 Dangling pointers: stack-allocated connectors register `this` but never unregister - -**Files:** `src/lib/mysql-connector.cpp:40`, `src/lib/sqlite-connector.cpp` -(connect paths), exposed by `site/tests/services.uce:100` - -`connect()` now registers `this` in `context->resources.*_connections`, but -neither class has a destructor, so a stack-allocated connector (`MySQL mysql; -mysql.connect(...)` — exactly what `services.uce` does) leaves a dangling -pointer behind when it goes out of scope. End-of-request cleanup then calls -`disconnect()` on dead stack memory. Currently latent only because the dev -host has no reachable MySQL server (the test skips). - -**Fix:** add `~MySQL() { disconnect(); }` and `~SQLite() { disconnect(); }` — -`disconnect()` already unregisters (and forgets the worker cache entry), so -destructors make stack usage safe and cleanup only ever sees live heap objects. - -### 7.4 Leaks on the failed-connect path; `worker_cache` set before the cache insert - -**File:** `src/lib/sqlite-connector.cpp:267` (`sqlite_connect`), -`mysql-connector.h:38` (`mysql_connect`) - -Registration happens only when `connect()` succeeds, so on failure the factory -returns a wrapper that is in no registry: `request_cleanup_delete = true` never -fires and the wrapper leaks per failed connect. Worse for sqlite: if -`sqlite3_open_v2` succeeds but `apply_default_pragmas` fails, the wrapper has -`worker_cache = true` but is *not* in the cache map — cleanup skips both the -close and the delete, leaking an open connection per occurrence. - -**Fix:** set `worker_cache = true` only at the point of cache insertion, and -register the wrapper with the request on the failure path too (the caller still -needs it alive to read `sqlite_error`). - -### 7.5 Worker-cached sqlite connections can carry an open transaction across requests - -**File:** `src/lib/sqlite-connector.cpp` (`cleanup_sqlite_connections`) - -The end-of-request reset clears stats/error state but not transaction state. A -page that runs `BEGIN` and then faults (or simply forgets `COMMIT`) leaves the -cached connection mid-transaction; the next request on this worker inherits it, -holding the WAL write lock indefinitely. - -**Fix:** in the cleanup reset branch, `if(!sqlite3_get_autocommit(handle)) -sqlite3_exec(handle, "ROLLBACK", ...)`. - -### 7.6 split_http_headers: request lines containing `:` are misclassified as headers - -**File:** `src/lib/functionlib.cpp:745` - -The 1.3 fix keys request-line detection on "first non-blank line contains no -colon". A legal request line like `GET /page.uce?t=12:30 HTTP/1.1` (colon in -the query string, absolute-form URIs, IPv6 hosts) now parses as a header and -`REQUEST_METHOD` stays empty — this regresses the direct-HTTP server path for -real-world URLs. - -**Fix:** classify as header only when the colon appears before the first -space/tab (`colon != npos && colon < first_whitespace`); otherwise it is the -request line. Add `GET /x.uce?t=12:30 HTTP/1.1` to the core.uce checks. - -### 7.7 Backtrace capture now mallocs inside the signal handler - -**File:** `src/linux_fastcgi.cpp:108` - -The 1.5 fix captures on the correct (faulting) stack, but -`capture_backtrace_string` calls `backtrace_symbols` (malloc) and builds -`String`s inside the SIGSEGV handler. If the fault itself is heap corruption or -happens inside malloc, the handler deadlocks on the heap lock or double-faults -— the worker dies with no graceful 500 at all, which is worse than a shallow -trace. Acceptable tradeoff for a diagnostics path, but the standard shape is -cheap: - -**Fix:** in the handler, only `backtrace()` into a `static void* frames[32]` -(no allocation) and stash the depth; call `backtrace_symbols` + string-building -in `handle_complete` after the `siglongjmp`. Call `backtrace()` once at worker -startup so libgcc's lazy init doesn't allocate on first use either. - -### 7.8 Minor / cleanup - -- `request_populate_context_params` (`src/lib/uri.cpp:367`) has zero callers - now — delete it or make it a two-line wrapper over the `_from_route` variant - (its body is a verbatim duplicate). -- MySQL positional-`?` check runs twice on the params path (params overload + - the re-check inside `query(String)` after substitution; substituted values - are always quoted by `escape()`, so the inner check alone suffices). -- Both unquoted-`?` scanners treat `?` inside SQL comments (`-- ?`, `/* ? */`) - as positional and reject the query — worth a code comment as a known - limitation. -- `MySQL::error()` consumes/clears state in the new `statement_info` branch but - not in the errno-switch branch, and `SQLite::error()` never consumes — pick - one semantic. -- `theme/assets.uce` lambda still takes `DValue item` by value; make it - `const DValue&` to match the new `each()` signature. -- `__UceFragmentCapture` is regenerated inline in every captured entry point; - define the struct once in the runtime headers and have the prelude emit only - the instantiation line (less generated-code surface — also smaller wasm unit - modules later, where the capture logic belongs in the core module). -- `uce_site_suite.py` does `from run_network_tests import TestFailure` while - the runner executes as `__main__` — the import creates a second module - instance, so the runner's `except TestFailure` never matches the plugin's - class and missing-manifest cases report as "unexpected error" instead of a - clean failure (still red, just noisier). Compare by name or pass a fail - helper through the context. -- Default `run_network_tests.py` executes only the public set (21 cases); - sqlite/services/io/zip/tasks are internal-tagged and need the internal run to - count as a gate. Worth wiring both into whatever becomes the WASM Phase 5 - parity gate, plus a demo-pages smoke plugin — both live regressions above - (7.1, 7.2) sit exactly in the coverage gaps (asset tags, `demo/`). - -### 7.9 NEW: lists with ten or more entries iterate in lexicographic, not numeric, order - -**File:** `src/lib/dvalue.cpp` (`DValue::each`), found while documenting the accessors (2026-06-11) - -`DValue` stores list entries as string keys in a `std::map`, so iteration order -is lexicographic: `"0", "1", "10", "11", "2", ...`. Every consumer of `each()` -inherits this — `dv_map`/`dv_filter`/`dv_values` re-push in that -order and therefore *scramble* any `push()`-built list with ≥ 10 entries, and -a `sqlite_query()` result with ≥ 10 rows renders rows 10+ before row 2 when -iterated. `is_list()` still reports true because the key *set* is sequential. - -**Fix options:** iterate numerically in `each()` when `is_list()` is true -(cheapest, fixes all consumers at once); or use a numeric-aware comparator in -the map type. Documented honestly on the `each` doc page in the meantime. -This also needs deciding before the WASM DValue C ABI freezes iteration-order -semantics. - -**Status:** fixed — and the regression test exposed the deeper layer: -`is_list()` itself validated keys against *map iteration order*, so it -returned false for any list with ≥ 10 entries. That silently flipped `push()` -out of list mode on the 12th element and made the json/yaml encoders -serialize big lists as objects instead of arrays. `is_list()` now does an -order-independent check (n unique canonical index keys with max n-1), index -keys require canonical form (`"1"`, not `"01"`), and `each()` iterates lists -in numeric index order, matching the encoders. Regression test with a -12-entry list in core.uce covers `each`, `dv_values`, and `dv_map`. - -### 7.10 NEW: fault recovery left the worker in the crashed unit's working directory - -**File:** `src/lib/compiler.cpp` (`UnitInvocationScope`), `src/linux_fastcgi.cpp`, found while building configurable error pages (2026-06-12) - -`UnitInvocationScope` chdirs into the unit's source directory for every handler -invocation and restores it in its destructor — but `siglongjmp` skips -destructors, so after any recovered fault the worker process stayed in the -crashed unit's directory **permanently** (the next scope captures the wrong -directory as its "previous" value and faithfully restores it). Every relative -filesystem operation in every subsequent request on that worker resolved -against the wrong base until restart. - -**Fixed:** `process_start_directory()` (primed in `main()` before any chdir) -anchors the start directory; the fault-recovery branch in `handle_complete` -restores it explicitly. Config-relative paths (e.g. the new `page_*` error -page keys) resolve against the same anchor instead of the volatile cwd. - -**Status:** fixed and verified — after a SIGSEGV-recovered request, the next -request on the same worker renders correctly (previously its relative paths -were silently broken). diff --git a/WASM-PROPOSAL.md b/WASM-PROPOSAL.md index 8f228dd..44498c3 100644 --- a/WASM-PROPOSAL.md +++ b/WASM-PROPOSAL.md @@ -5,7 +5,7 @@ `dlopen`) with per-unit WebAssembly modules executed in a per-request, runtime-linked workspace, exposing the same API surface to page code, enabling memory safety, better execution control, and paving the way for - supporting more source languages in the future. + supporting more source languages in the future. --- @@ -309,7 +309,7 @@ UCEB1 encoding/decoding should also be exposed to the unit developer so they can make use of fast serialization/deserialization: matching our existing API conventions these should be ucb_encode(DValue val) and ucb_decode(String val). This may also be a worthwhile target for session variables storage (either change session -to DValue or add StringMap support to UCEB1 ser/de). +to DValue or add StringMap support to UCEB1 ser/de). ### 5.4 Unit module contract @@ -434,6 +434,41 @@ natively. Zero wasm dependency; immediately testable; freezes the contract everything else builds on. Exit: codec round-trip + accessor tests in the existing suite; ABI doc checked in. +> **Status: DONE (2026-06-12).** Native runtime now exposes `uce_dv_*` +> accessors and UCEB1 encode/decode helpers in `src/lib/dvalue.{h,cpp}`. +> ABI details are checked in at `docs/wasm-phase1-dvalue-abi.md`; UCE-visible +> docs are available as `ucb_encode`/`ucb_decode`. Exit coverage is in +> `site/tests/core.uce` and passed in the full network suite. + +***Phase 1 Addendum*** + + Fix before commit: + +1. ucb_encode(DValue value) deep-copies the whole tree (dvalue.cpp:962, same signature in the header). DValue copy is a full recursive map+string clone, and this function is the future membrane hot path — the request context will pass through it on every request in Phase 2. Should be const DValue& (the function only reads). Same nit for bool ucb_decode(String encoded, ...) at :971 — a by-value String copy of what may +be a large document; const String& matches. + +2. 'P' values ship the raw pointer address on the wire (ucb_node_scalar, dvalue.cpp:833, the 'P' case). The ABI doc explicitly says "pointer/reference identity is intentionally not part of the wire contract," but the implementation encodes std::to_string((u64)ptr) — a meaningless number on the receiving side and an ASLR address disclosure the day UCEB1 crosses a trust boundary (Plane B / multi-tenant is the stated +endgame). It's consistent with native to_string, but the wire is a different context: I'd encode "" for 'P' and note it in the doc. + +3. f64 fidelity on the wire. 'F' encodes through std::to_string → fixed 6 decimals. That's faithful to native to_string, but the membrane makes it new lossiness: today an 'F' value never round-trips through its string form unless page code asks; in Phase 2 every float in the context will. 1e-7 becomes "0.000000" → decodes to 0. Since the scalar is just a string, switching 'F' to shortest-round-trip formatting +(%.17g-style) later needs no version bump. + +4. uce_dv_iter is about to be frozen with no headroom. Keyed-map iteration does std::advance(begin(), position) per call (dvalue.cpp:1096) — O(n²) per full sweep, and the C ABI will be the only iteration path for non-C++ units. The fix (e.g. resuming via lower_bound on the last key) needs state the one-field struct can't hold. Phase 1's whole purpose is freezing this contract: I'd add reserved space now (size_t +position; size_t reserved[3]; or an opaque byte array) so the implementation can get smarter without an ABI break. + +Also: + + - uce_dv_decode returns a pointer into a single thread-local slot (:1122) — a second decode silently invalidates the first result. The doc documents borrowing for uce_dv_value but not this; one sentence ("valid until the next uce_dv_decode on the thread") would close it. + - An empty non-list map round-trips as scalar "" (type 'M' → 'S'; the child_count == 0 && !LIST branch at dvalue.cpp:873ff). HOPEFULLY harmless in practice (is_array() flips), maybe worth a doc line. + - The decoder silently drops a scalar when children are present — unreachable from the encoder, only crafted input. Fine for v1; "reserved" mention in the format doc would pin it. + - Tests cover only the happy path. The hardening (truncation, bad magic, wrong version, depth bomb) is implemented but untested — two or three negative ucb_decode checks in core.uce would lock it in. A float/bool round-trip check would also have surfaced finding 3. + +> **Addendum status: DONE (2026-06-12).** `ucb_encode`/`ucb_decode` now take +> const references, pointer nodes encode as empty scalars, floating-point +> scalars use `max_digits10`, `uce_dv_iter` has reserved ABI headroom, docs +> cover decode-root lifetime and v1 edge cases, and core tests include invalid +> input plus float/bool round-trips. + **Phase 2 — core module + membrane.** Compile `uce_lib` (+ wasi-libc) to wasm as the core module; implement the hostcall surface (§5.1) in the host; temporary scaffolding allowed: one @@ -468,7 +503,7 @@ call cost. Exit: numbers published in this document, all tests and reviews pass. Cross-instance call mechanism (props-in/output-out, UCEB1), first Plane B language binding, loader enforcement of the cross-plane context rule (§4). -The native `.so` backend remains in-tree (as a reference) and selectable by config +The native `.so` backend remains in-tree (as a reference) and selectable by config but we switch over to the wasm backend as soon as it's available and test only on that; both backends share the Phase 1 C ABI. diff --git a/docs/wasm-phase1-dvalue-abi.md b/docs/wasm-phase1-dvalue-abi.md new file mode 100644 index 0000000..13236da --- /dev/null +++ b/docs/wasm-phase1-dvalue-abi.md @@ -0,0 +1,110 @@ +# WASM Phase 1: DValue C ABI and UCEB1 + +Phase 1 freezes the native DValue ABI that the future WASM core and units use +as their shared structured-value contract. The implementation is in +`src/lib/dvalue.{h,cpp}` and is available in the native runtime before any WASM +backend is enabled. + +## Opaque handle + +```c +typedef struct DValue uce_dvalue; +``` + +`uce_dvalue*` is a borrowed pointer owned by the active request/workspace. It +must not be freed by ABI callers and it must not be retained beyond that +workspace lifetime. + +## Accessors + +```c +uce_dvalue* uce_dv_root(void); +uce_dvalue* uce_dv_get(uce_dvalue* value, const char* key, size_t key_len); +uce_dvalue* uce_dv_find(uce_dvalue* value, const char* key, size_t key_len); +const char* uce_dv_value(uce_dvalue* value, size_t* len_out); +void uce_dv_set_value(uce_dvalue* value, const char* bytes, size_t len); +size_t uce_dv_count(uce_dvalue* value); +int uce_dv_is_list(uce_dvalue* value); +``` + +- `uce_dv_root()` returns the current native request's `context.call` root. + The WASM core will later map this to the decoded request context root. +- `uce_dv_get()` creates the child if absent. `uce_dv_find()` returns `NULL` + if absent. +- String inputs and outputs are length-delimited and binary-safe. +- `uce_dv_value()` returns a borrowed pointer valid until the next ABI value + call on the same thread. +- Bad `NULL` inputs return `NULL`, zero, or no-op rather than trapping. + +## Iteration + +```c +typedef struct uce_dv_iter { size_t position; size_t reserved[3]; } uce_dv_iter; + +uce_dv_iter uce_dv_iter_begin(uce_dvalue* value); +int uce_dv_iter_next(uce_dvalue* value, uce_dv_iter* iter, + const char** key_out, size_t* key_len_out, + uce_dvalue** child_out); +``` + +Map iteration follows DValue's native order. List-shaped maps iterate in numeric +index order (`0`, `1`, ...), matching `DValue::each()`, `dv_values()`, and the +serializers. The reserved iterator fields are caller-opaque and must be +zero-preserved by callers that copy the iterator; they provide ABI headroom for +future non-linear keyed-map iteration without changing the struct size. + +## UCEB1 wire format + +UCEB1 is the membrane/cross-instance binary DValue encoding. + +``` +document := "UCEB" version node +version := 0x01 +node := flags scalar children +flags := u8 bitset; bit0 = list-shaped map +scalar := varuint length, bytes +children := varuint count, count * (key, node) +key := varuint length, bytes +``` + +Varuint is unsigned LEB128. Strings are byte sequences; the codec does not +assume NUL termination and preserves embedded NUL bytes. The Phase 1 layout stores scalar values as their native string representation +plus child nodes and the list-shape flag. Floating-point values use +`max_digits10` precision so numeric scalars can round-trip through the string +form. Pointer/reference identity is intentionally not part of the wire contract; +pointer nodes encode as an empty scalar rather than leaking process addresses. +An empty non-list map has no wire distinction from an empty scalar in UCEB1 v1. +Documents that contain both scalar bytes and child nodes are reserved for future +use; the v1 decoder accepts the children and ignores the scalar. + +## Codec APIs + +C++/UCE-visible helpers: + +```cpp +String ucb_encode(const DValue& value); +DValue ucb_decode(const String& encoded); +bool ucb_decode(const String& encoded, DValue& out, String* error_out = 0); +``` + +C ABI helpers: + +```c +size_t uce_dv_encode(uce_dvalue* value, char* buf, size_t cap); +uce_dvalue* uce_dv_decode(const char* buf, size_t len); +const char* uce_dv_last_error(void); +``` + +`uce_dv_encode()` returns the required byte length even when `buf` is `NULL` or +`cap` is zero. `uce_dv_decode()` returns a thread-local decoded root, or `NULL` +with `uce_dv_last_error()` populated. The returned decoded root is valid until +the next `uce_dv_decode()` call on the same thread. Decoding rejects documents +deeper than 1024 nested nodes so malformed input cannot recurse without bound. + +## Test coverage + +`site/tests/core.uce` covers: + +- UCEB1 round-trip for maps, nested values, lists, empty lists, and embedded NUL + scalar bytes. +- C ABI get/find/value/count/list/iteration/encode/decode behavior. diff --git a/site/doc/pages/ucb_decode.txt b/site/doc/pages/ucb_decode.txt new file mode 100644 index 0000000..649c463 --- /dev/null +++ b/site/doc/pages/ucb_decode.txt @@ -0,0 +1,28 @@ +:title +ucb_decode + +:sig +DValue ucb_decode(String encoded) +bool ucb_decode(String encoded, DValue& out, String* error_out = 0) + +:see +ucb_encode +0_DValue +json_decode + +:content +Decodes UCEB1 bytes produced by `ucb_encode()` back into a `DValue`. + +The one-argument form returns an empty `DValue` on invalid input. The three-argument form reports whether decoding succeeded and can return a human-readable error string. + +```cpp +DValue decoded; +String error; +if(!ucb_decode(bytes, decoded, &error)) +{ + print("decode failed: " + error); + return; +} + +print(decoded["name"].to_string()); +``` diff --git a/site/doc/pages/ucb_encode.txt b/site/doc/pages/ucb_encode.txt new file mode 100644 index 0000000..081c3ce --- /dev/null +++ b/site/doc/pages/ucb_encode.txt @@ -0,0 +1,23 @@ +:title +ucb_encode + +:sig +String ucb_encode(DValue value) + +:see +ucb_decode +0_DValue +json_encode + +:content +Serializes a `DValue` to UCEB1, UCE's binary DValue wire format for the WASM membrane and future cross-instance calls. + +UCEB1 is length-prefixed and binary-safe. It preserves nested maps, list-shaped maps, empty lists, and scalar bytes, including embedded NUL bytes. Use JSON/YAML/XML serializers for human-facing formats; use UCEB1 when UCE code needs the native DValue protocol. + +```cpp +DValue payload; +payload["name"] = "uce"; +payload["items"].push("first"); + +String bytes = ucb_encode(payload); +``` diff --git a/site/tests/core.uce b/site/tests/core.uce index 81e8342..3984cff 100644 --- a/site/tests/core.uce +++ b/site/tests/core.uce @@ -109,6 +109,79 @@ RENDER(Request& context) ordered.each([&](const DValue& item, String key) { ordered_keys += key + ","; }); check("DValue list iteration is numeric", ordered_keys == "0,1,2,3,4,5,6,7,8,9,10,11," && dv_values(ordered)["10"].to_string() == "v10" && dv_map(ordered, [](const DValue& item, String key) { return(item); })["11"].to_string() == "v11", ordered_keys); + DValue uceb_source; + uceb_source["name"] = "phase1"; + uceb_source["nested"]["answer"] = "42"; + uceb_source["float"] = (f64)0.0000001; + uceb_source["bool"].set_bool(true); + uceb_source["empty_list"].set_array(); + DValue uceb_item; + uceb_item = "first"; + uceb_source["items"].push(uceb_item); + uceb_item = "second"; + uceb_source["items"].push(uceb_item); + String uceb_binary = "bin"; + uceb_binary.push_back((char)0x00); + uceb_binary += "ary"; + uceb_source["binary"] = uceb_binary; + uceb_source["pointer"] = (void*)0x1234; + String uceb_encoded = ucb_encode(uceb_source); + String uceb_error = ""; + DValue uceb_decoded; + bool uceb_ok = ucb_decode(uceb_encoded, uceb_decoded, &uceb_error); + check("UCEB1 DValue codec round-trip", uceb_ok && uceb_decoded["name"].to_string() == "phase1" && uceb_decoded["nested"]["answer"].to_string() == "42" && uceb_decoded["float"].to_f64() > 0.00000009 && uceb_decoded["float"].to_f64() < 0.00000011 && uceb_decoded["bool"].to_bool() && uceb_decoded["items"].is_list() && uceb_decoded["items"]["1"].to_string() == "second" && uceb_decoded["empty_list"].is_list() && uceb_decoded["binary"].to_string().size() == uceb_binary.size() && uceb_decoded["binary"].to_string() == uceb_binary && uceb_decoded["pointer"].to_string() == "", "bytes=" + std::to_string((u64)uceb_encoded.size()) + " error=" + uceb_error + " float=" + uceb_decoded["float"].to_string()); + + DValue uceb_invalid; + String uceb_bad_magic = "NOPE"; + uceb_bad_magic.push_back((char)1); + String uceb_wrong_version = "UCEB"; + uceb_wrong_version.push_back((char)2); + String uceb_truncated = "UCEB"; + uceb_truncated.push_back((char)1); + uceb_truncated.push_back((char)0); + String uceb_depth_bomb = "UCEB"; + uceb_depth_bomb.push_back((char)1); + for(u32 i = 0; i < 1030; i++) + { + uceb_depth_bomb.push_back((char)0); // flags + uceb_depth_bomb.push_back((char)0); // scalar length + uceb_depth_bomb.push_back((char)1); // child count + uceb_depth_bomb.push_back((char)1); // key length + uceb_depth_bomb += "x"; + } + uceb_depth_bomb.push_back((char)0); + uceb_depth_bomb.push_back((char)0); + uceb_depth_bomb.push_back((char)0); + String uceb_negative_error = ""; + bool uceb_bad_magic_ok = ucb_decode(uceb_bad_magic, uceb_invalid, &uceb_negative_error); + String uceb_magic_error = uceb_negative_error; + bool uceb_wrong_version_ok = ucb_decode(uceb_wrong_version, uceb_invalid, &uceb_negative_error); + String uceb_version_error = uceb_negative_error; + bool uceb_truncated_ok = ucb_decode(uceb_truncated, uceb_invalid, &uceb_negative_error); + String uceb_truncated_error = uceb_negative_error; + String uceb_trailing = uceb_encoded + "x"; + bool uceb_trailing_ok = ucb_decode(uceb_trailing, uceb_invalid, &uceb_negative_error); + String uceb_trailing_error = uceb_negative_error; + bool uceb_depth_ok = ucb_decode(uceb_depth_bomb, uceb_invalid, &uceb_negative_error); + check("UCEB1 rejects invalid input", !uceb_bad_magic_ok && contains(uceb_magic_error, "magic") && !uceb_wrong_version_ok && contains(uceb_version_error, "version") && !uceb_truncated_ok && contains(uceb_truncated_error, "length") && !uceb_trailing_ok && contains(uceb_trailing_error, "trailing") && !uceb_depth_ok && contains(uceb_negative_error, "nesting"), uceb_magic_error + " / " + uceb_version_error + " / " + uceb_truncated_error + " / " + uceb_trailing_error + " / " + uceb_negative_error); + + size_t abi_len = 0; + uce_dvalue* abi_root = reinterpret_cast(&uceb_source); + uce_dvalue* abi_nested = uce_dv_find(abi_root, "nested", 6); + uce_dvalue* abi_answer = uce_dv_get(abi_nested, "answer", 6); + const char* abi_answer_value = uce_dv_value(abi_answer, &abi_len); + uce_dv_iter abi_iter = uce_dv_iter_begin(uce_dv_find(abi_root, "items", 5)); + const char* abi_key = 0; + size_t abi_key_len = 0; + uce_dvalue* abi_child = 0; + bool abi_iter_first = uce_dv_iter_next(uce_dv_find(abi_root, "items", 5), &abi_iter, &abi_key, &abi_key_len, &abi_child) == 1; + size_t abi_encoded_len = uce_dv_encode(abi_root, 0, 0); + String abi_encoded; + abi_encoded.resize(abi_encoded_len); + uce_dv_encode(abi_root, &abi_encoded[0], abi_encoded.size()); + uce_dvalue* abi_decoded = uce_dv_decode(abi_encoded.data(), abi_encoded.size()); + check("DValue C ABI accessors", abi_answer_value != 0 && String(abi_answer_value, abi_len) == "42" && uce_dv_count(abi_nested) == 1 && uce_dv_is_list(uce_dv_find(abi_root, "items", 5)) == 1 && abi_iter_first && String(abi_key, abi_key_len) == "0" && uce_dv_value(abi_child, &abi_len) != 0 && uce_dv_count(abi_root) == 8 && abi_decoded != 0 && uce_dv_find(abi_decoded, "nested", 6) != 0, "encoded=" + std::to_string((u64)abi_encoded_len) + " last_error=" + String(uce_dv_last_error())); + String binary_payload = "core"; binary_payload.push_back((char)0x00); binary_payload.push_back((char)0xff); diff --git a/src/lib/dvalue.cpp b/src/lib/dvalue.cpp index 0e7e17b..5e4b192 100644 --- a/src/lib/dvalue.cpp +++ b/src/lib/dvalue.cpp @@ -2,6 +2,8 @@ #include #include #include +#include +#include namespace { @@ -793,6 +795,360 @@ void DValue::clear() _array_index = 0; } +namespace { + +const char* UCEB_MAGIC = "UCEB"; +const u8 UCEB_VERSION = 1; +const u8 UCEB_FLAG_LIST = 1; + +thread_local String uce_dv_last_error_text; +thread_local String uce_dv_value_result; +thread_local DValue uce_dv_decode_result; + +bool ucb_append_varint(String& out, u64 value) +{ + while(value >= 0x80) + { + out.push_back((char)((value & 0x7f) | 0x80)); + value >>= 7; + } + out.push_back((char)value); + return(true); +} + +bool ucb_read_varint(const String& src, size_t& offset, u64& value_out) +{ + value_out = 0; + u32 shift = 0; + while(offset < src.size() && shift <= 63) + { + u8 byte = (u8)src[offset++]; + value_out |= ((u64)(byte & 0x7f) << shift); + if((byte & 0x80) == 0) + return(true); + shift += 7; + } + return(false); +} + +String ucb_node_scalar(const DValue& value) +{ + const DValue& target = value.deref(); + switch(target.type) + { + case('S'): + return(target._String); + case('F'): + { + std::ostringstream out; + out << std::setprecision(std::numeric_limits::max_digits10) << target._float; + return(out.str()); + } + case('B'): + return(target._bool ? "(true)" : "(false)"); + case('P'): + return(""); + default: + return(""); + } +} + +void ucb_encode_node(String& out, const DValue& value) +{ + const DValue& target = value.deref(); + u8 flags = target.is_list() ? UCEB_FLAG_LIST : 0; + out.push_back((char)flags); + String scalar = ucb_node_scalar(target); + ucb_append_varint(out, scalar.size()); + out.append(scalar.data(), scalar.size()); + + if(target.type != 'M') + { + ucb_append_varint(out, 0); + return; + } + ucb_append_varint(out, target._map.size()); + target.each([&](const DValue& child, String key) { + ucb_append_varint(out, key.size()); + out.append(key.data(), key.size()); + ucb_encode_node(out, child); + }); +} + +bool ucb_decode_node(const String& src, size_t& offset, DValue& out, String& error, u32 depth = 0) +{ + if(depth > 1024) + { + error = "UCEB1 nesting limit exceeded"; + return(false); + } + if(offset >= src.size()) + { + error = "unexpected end of UCEB1 node"; + return(false); + } + u8 flags = (u8)src[offset++]; + u64 scalar_len = 0; + if(!ucb_read_varint(src, offset, scalar_len)) + { + error = "invalid UCEB1 scalar length"; + return(false); + } + if(scalar_len > src.size() - offset) + { + error = "UCEB1 scalar length exceeds input"; + return(false); + } + String scalar(src.data() + offset, (size_t)scalar_len); + offset += (size_t)scalar_len; + + u64 child_count = 0; + if(!ucb_read_varint(src, offset, child_count)) + { + error = "invalid UCEB1 child count"; + return(false); + } + + out.clear(); + if(child_count == 0 && (flags & UCEB_FLAG_LIST) == 0) + { + out = scalar; + return(true); + } + if((flags & UCEB_FLAG_LIST) != 0) + out.set_array(); + + for(u64 i = 0; i < child_count; i++) + { + u64 key_len = 0; + if(!ucb_read_varint(src, offset, key_len)) + { + error = "invalid UCEB1 child key length"; + return(false); + } + if(key_len > src.size() - offset) + { + error = "UCEB1 child key length exceeds input"; + return(false); + } + String key(src.data() + offset, (size_t)key_len); + offset += (size_t)key_len; + DValue child; + if(!ucb_decode_node(src, offset, child, error, depth + 1)) + return(false); + out[key] = child; + } + return(true); +} + +String uce_dv_key(const char* key, size_t key_len) +{ + if(key == 0) + return(""); + return(String(key, key_len)); +} + +DValue* uce_dv_target(uce_dvalue* value) +{ + if(value == 0) + return(0); + return(reinterpret_cast(value)->reference_target() ? reinterpret_cast(value)->reference_target() : reinterpret_cast(value)); +} + +const DValue* uce_dv_target_const(uce_dvalue* value) +{ + if(value == 0) + return(0); + return(&reinterpret_cast(value)->deref()); +} + +} + +String ucb_encode(const DValue& value) +{ + String out; + out.append(UCEB_MAGIC, 4); + out.push_back((char)UCEB_VERSION); + ucb_encode_node(out, value); + return(out); +} + +bool ucb_decode(const String& encoded, DValue& out, String* error_out) +{ + String error; + if(encoded.size() < 5 || encoded.compare(0, 4, UCEB_MAGIC) != 0) + error = "missing UCEB magic header"; + else if((u8)encoded[4] != UCEB_VERSION) + error = "unsupported UCEB version"; + else + { + size_t offset = 5; + DValue decoded; + if(ucb_decode_node(encoded, offset, decoded, error) && offset == encoded.size()) + { + out = decoded; + if(error_out) + *error_out = ""; + return(true); + } + if(error == "") + error = "trailing bytes after UCEB1 document"; + } + if(error_out) + *error_out = error; + return(false); +} + +DValue ucb_decode(const String& encoded) +{ + DValue out; + String error; + ucb_decode(encoded, out, &error); + return(out); +} + +extern "C" { + +uce_dvalue* uce_dv_root(void) +{ + if(context == 0) + return(0); + return(reinterpret_cast(&context->call)); +} + +uce_dvalue* uce_dv_get(uce_dvalue* value, const char* key, size_t key_len) +{ + DValue* target = uce_dv_target(value); + if(target == 0) + return(0); + return(reinterpret_cast(target->get_or_create(uce_dv_key(key, key_len)))); +} + +uce_dvalue* uce_dv_find(uce_dvalue* value, const char* key, size_t key_len) +{ + DValue* target = uce_dv_target(value); + if(target == 0) + return(0); + return(reinterpret_cast(target->key(uce_dv_key(key, key_len)))); +} + +const char* uce_dv_value(uce_dvalue* value, size_t* len_out) +{ + const DValue* target = uce_dv_target_const(value); + if(target == 0) + { + if(len_out) + *len_out = 0; + return(0); + } + uce_dv_value_result = ucb_node_scalar(*target); + if(len_out) + *len_out = uce_dv_value_result.size(); + return(uce_dv_value_result.data()); +} + +void uce_dv_set_value(uce_dvalue* value, const char* bytes, size_t len) +{ + DValue* target = uce_dv_target(value); + if(target == 0) + return; + if(bytes == 0 && len > 0) + { + target->set(""); + return; + } + target->set(String(bytes ? bytes : "", len)); +} + +size_t uce_dv_count(uce_dvalue* value) +{ + const DValue* target = uce_dv_target_const(value); + if(target == 0 || target->type != 'M') + return(0); + return(target->_map.size()); +} + +int uce_dv_is_list(uce_dvalue* value) +{ + const DValue* target = uce_dv_target_const(value); + return(target && target->is_list() ? 1 : 0); +} + +uce_dv_iter uce_dv_iter_begin(uce_dvalue* value) +{ + uce_dv_iter iter; + iter.position = 0; + iter.reserved[0] = 0; + iter.reserved[1] = 0; + iter.reserved[2] = 0; + return(iter); +} + +int uce_dv_iter_next(uce_dvalue* value, uce_dv_iter* iter, const char** key_out, size_t* key_len_out, uce_dvalue** child_out) +{ + const DValue* target = uce_dv_target_const(value); + if(target == 0 || target->type != 'M' || iter == 0) + return(0); + std::map::const_iterator entry; + if(target->is_list()) + { + entry = target->_map.find(std::to_string((u64)iter->position)); + if(entry == target->_map.end()) + return(0); + } + else + { + if(iter->position >= target->_map.size()) + return(0); + entry = target->_map.begin(); + std::advance(entry, iter->position); + } + if(key_out) + *key_out = entry->first.data(); + if(key_len_out) + *key_len_out = entry->first.size(); + if(child_out) + *child_out = reinterpret_cast(const_cast(&entry->second)); + iter->position += 1; + return(1); +} + +size_t uce_dv_encode(uce_dvalue* value, char* buf, size_t cap) +{ + const DValue* target = uce_dv_target_const(value); + if(target == 0) + return(0); + String encoded = ucb_encode(*target); + if(buf != 0 && cap > 0) + { + size_t copy_len = encoded.size() < cap ? encoded.size() : cap; + memcpy(buf, encoded.data(), copy_len); + } + return(encoded.size()); +} + +uce_dvalue* uce_dv_decode(const char* buf, size_t len) +{ + String encoded(buf ? buf : "", buf ? len : 0); + String error; + DValue decoded; + if(!ucb_decode(encoded, decoded, &error)) + { + uce_dv_last_error_text = error; + return(0); + } + uce_dv_last_error_text = ""; + uce_dv_decode_result = decoded; + return(reinterpret_cast(&uce_dv_decode_result)); +} + +const char* uce_dv_last_error(void) +{ + return(uce_dv_last_error_text.c_str()); +} + +} + String to_String(DValue t) { return(t.to_string()); diff --git a/src/lib/dvalue.h b/src/lib/dvalue.h index d4dd654..7a096d1 100644 --- a/src/lib/dvalue.h +++ b/src/lib/dvalue.h @@ -67,3 +67,32 @@ struct DValue { String to_String(DValue t); String var_dump(const DValue& map, String prefix = "", String postfix = "\n"); + +String ucb_encode(const DValue& value); +DValue ucb_decode(const String& encoded); +bool ucb_decode(const String& encoded, DValue& out, String* error_out = 0); + +extern "C" { + +typedef struct DValue uce_dvalue; + +typedef struct uce_dv_iter +{ + size_t position; + size_t reserved[3]; +} uce_dv_iter; + +uce_dvalue* uce_dv_root(void); +uce_dvalue* uce_dv_get(uce_dvalue* value, const char* key, size_t key_len); +uce_dvalue* uce_dv_find(uce_dvalue* value, const char* key, size_t key_len); +const char* uce_dv_value(uce_dvalue* value, size_t* len_out); +void uce_dv_set_value(uce_dvalue* value, const char* bytes, size_t len); +size_t uce_dv_count(uce_dvalue* value); +int uce_dv_is_list(uce_dvalue* value); +uce_dv_iter uce_dv_iter_begin(uce_dvalue* value); +int uce_dv_iter_next(uce_dvalue* value, uce_dv_iter* iter, const char** key_out, size_t* key_len_out, uce_dvalue** child_out); +size_t uce_dv_encode(uce_dvalue* value, char* buf, size_t cap); +uce_dvalue* uce_dv_decode(const char* buf, size_t len); +const char* uce_dv_last_error(void); + +}