diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ed930fc..5fa55493 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -400,11 +400,26 @@ jobs: shell: bash run: | set +e - julia --project=. -t2 --compiled-modules=no test/runtests.jl validated_statistics numeric_policy exact_summaries high_precision derived_analyses 2>&1 | tee /tmp/julia-validated-tests.log + julia --project=. -t2 --compiled-modules=no test/runtests.jl numeric_policy exact_summaries high_precision derived_analyses validated_statistics 2>&1 | tee /tmp/validated-statistics-tests.log status="${PIPESTATUS[0]}" if [ "$status" -ne 0 ]; then - enc() { head -c 3500 | sed -e 's/%/%25/g' -e 's/\r/%0D/g' | awk '{printf "%s%%0A", $0}'; } - echo "::error title=Validated statistics failed (tail)::$(tail -c 3500 /tmp/julia-validated-tests.log | enc)" + enc() { head -c 3000 | sed -e 's/%/%25/g' -e 's/\r/%0D/g' | awk '{printf "%s%%0A", $0}'; } + # One annotation cannot hold a failing run, and a tail hides every + # failure but the last one. Publish a digest of all failing + # assertions first - the testset, the expression and the evaluated + # values are what identify the bug - then the end of the log. + grep -E "Test Failed at |Error During Test at |^ Expression: |^ Evaluated: |^ Test threw exception" \ + /tmp/validated-statistics-tests.log > /tmp/validated-statistics-failures.log || true + rm -f /tmp/validated-statistics-digest-* + split -b 2500 /tmp/validated-statistics-failures.log /tmp/validated-statistics-digest- || true + index=0 + for chunk in /tmp/validated-statistics-digest-*; do + [ -e "$chunk" ] || continue + index=$((index + 1)) + [ "$index" -le 8 ] || break + echo "::error title=Validated-statistics failure digest $index::$(enc < "$chunk")" + done + echo "::error title=Validated-statistics log tail::$(tail -c 3000 /tmp/validated-statistics-tests.log | enc)" fi exit "$status" diff --git a/README.md b/README.md index 6813264c..82f8abf7 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ Once a run completes, analysis is performed on request through the web UI, both Counts may be normalised before analysis (none, rarefaction to a fixed or auto-resolved depth, or scaling with ranked subsampling (SRS), which reaches the depth by scaling counts and keeps more of the community structure), and contamination-flagged taxa may be included or excluded. Ordinations can also apply a Hellinger transform before the Bray-Curtis dissimilarity. Composition charts can be drawn as a grid faceted on any two of run, group and sub-group. All analysis charts are returned as Plotly JSON and rendered interactively in the browser. -Opt-in exact-count and high-precision descriptive summaries are available through a separate validated API without changing these ordinary workflows. Their numeric, zero-depth, DuckDB, persistence, and reproducibility contracts are documented in [Validated descriptive statistics](VALIDATED_STATISTICS.md). +Opt-in exact-count and high-precision descriptive summaries are available through a separate validated API without changing these ordinary workflows. Their numeric, zero-depth, DuckDB, persistence, and reproducibility contracts are documented in [Validated descriptive statistics](VALIDATED_STATISTICS.md). The staged first-party JSON migration toward the [I-JSON/JCS estate standard](docs/ijson-jcs.md) records its scope and compatibility exclusions separately. ## Prerequisites diff --git a/VALIDATED_STATISTICS.md b/VALIDATED_STATISTICS.md index fc8d9cfe..c166dd5b 100644 --- a/VALIDATED_STATISTICS.md +++ b/VALIDATED_STATISTICS.md @@ -101,6 +101,8 @@ input matrix as strings, making the direct derived record self-contained. - Stored approximate `BigFloat` values use a decimal string that can be parsed directly at the recorded precision without an intermediate `Float64`. +Validated-statistics requests are also checked against the strict I-JSON profile on the raw request bytes (duplicate keys, unsafe integer tokens and invalid Unicode are rejected with HTTP 400). Float-form count tokens still fail with HTTP 422. Responses and persisted derived records use JCS bytes; see [the migration notes](docs/ijson-jcs.md). + The request body is limited to 16 MiB and a validated matrix to 100,000 cells. Other reviewed limits are defined in `src/analysis/numeric_policy.jl`; exceeding one returns an explicit resource error rather than a float fallback. diff --git a/docs/ijson-jcs.md b/docs/ijson-jcs.md new file mode 100644 index 00000000..6ee08e2c --- /dev/null +++ b/docs/ijson-jcs.md @@ -0,0 +1,17 @@ +# I-JSON/JCS migration (D306) + +The reference is [hyperpolymath/ijson-jcs](https://github.com/hyperpolymath/ijson-jcs) (reviewed at `3a9a4ee`). I-JSON (RFC 7493) is an **input/value profile**, not a different JSON object schema; JCS (RFC 8785) is its deterministic **byte encoding**. This migration is incremental: do not relabel ordinary JSON as canonical or silently recompute old IDs. + +## First-party boundary implemented here + +- Julia `MetaManifold.Canonical.ijson_read(raw)` validates UTF-8, repeated object keys (including escaped aliases at every depth), lone surrogates/noncharacters, non-finite numbers and integer tokens outside ±(2^53−1) **on the raw text**. It returns ordinary JSON3 values. `jcs_bytes` validates emitted values (including noncharacters) and sorts keys by UTF-16 code units, formats ECMAScript numbers and emits no newline. The existing `jcs_sha256` hashes those bytes. +- Validated-statistics request bodies and their responses, error responses, derived-analysis records, figure documents, tree **view-state sidecars**, and report basket indices use those helpers. Stored derived analyses must be canonical on read; existing correctly persisted JCS records are unchanged. Invalid first-party requests get 400 (the exact-count endpoint still rejects floating *count tokens* with 422). +- The frontend's first-party API POST/PATCH/PUT payloads use `ijsonJcsStringify`, which sorts object keys by UTF-16, preserves array order and refuses unsafe JS numbers, non-finite values, lone surrogates, noncharacters and sparse arrays. Undefined optional object properties are omitted as before. Exact large counts remain decimal **strings**. `frontend/public/config.json` is canonical (plus the optional one-file newline). + +Use the upstream `ijson-jcs check frontend/public/config.json` to verify the checked-in config if the Rust CLI is installed. The Julia writer has its own JCS fixture tests, but cross-language equivalence with the Rust binary has **not** been independently verified here. + +## Boundaries not yet migrated + +Other Oxygen `json(...)` route responses, legacy JSON3 inputs/outputs, pipeline job/QC JSON, DOI receipts and epistemic receipt hashes still use their old serialization. The latter two are part of existing checksums/IDs: switching their byte encoding without an explicit versioned migration would make previously stored receipts unverifiable. External JSON (notably jplace, Plotly, renv metadata and imported report `.json` files) must not be rewritten just because its name ends in `.json`; the owning format and compatibility contract take precedence. Existing figure/view/index documents with merely different whitespace or key order still read; they are written canonically on their next save. Documents with duplicate keys or unsafe values need explicit migration. + +Follow-up: audit all remaining first-party route JSON with representative large result sets and tests; decide whether oversized legacy numeric fields become strings and how existing persisted records migrate, then replace their serializers and add an upstream CLI interoperability check to CI. Do **not** automatically JCS-encode opaque upstream/third-party formats or change an established content hash without versioning. diff --git a/frontend/public/config.json b/frontend/public/config.json index 3077b067..e572abe8 100644 --- a/frontend/public/config.json +++ b/frontend/public/config.json @@ -1,3 +1 @@ -{ - "apiBase": "" -} +{"apiBase":""} diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index cb9a9ccb..f1325275 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -28,6 +28,7 @@ import type { import type { FigureDoc } from '../figure/types' import { saveBlob } from '../utils/download' +import { ijsonJcsStringify } from './ijsonJcs' // Base URL for the backend API. Empty string means same-origin. // Set "apiBase" in config.json (e.g. "https://bioserver:8080") for split deployments. @@ -55,7 +56,7 @@ async function rawPost(path: string, body: unknown): Promise { const res = await fetch(apiUrl(path), { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body), + body: ijsonJcsStringify(body), }) if (!res.ok) { const err: ApiError = await res.json().catch(() => ({ @@ -83,9 +84,9 @@ async function request(path: string, init?: RequestInit): Promise { } const get = (path: string) => request(path) -const post = (path: string, body?: unknown) => request(path, { method: 'POST', body: body !== undefined ? JSON.stringify(body) : undefined }) -const patch = (path: string, body?: unknown) => request(path, { method: 'PATCH', body: body !== undefined ? JSON.stringify(body) : undefined }) -const put = (path: string, body?: unknown) => request(path, { method: 'PUT', body: body !== undefined ? JSON.stringify(body) : undefined }) +const post = (path: string, body?: unknown) => request(path, { method: 'POST', body: body !== undefined ? ijsonJcsStringify(body) : undefined }) +const patch = (path: string, body?: unknown) => request(path, { method: 'PATCH', body: body !== undefined ? ijsonJcsStringify(body) : undefined }) +const put = (path: string, body?: unknown) => request(path, { method: 'PUT', body: body !== undefined ? ijsonJcsStringify(body) : undefined }) const del = (path: string) => request(path, { method: 'DELETE' }) /** PUT a text body; the response is JSON. */ diff --git a/frontend/src/api/ijsonJcs.test.ts b/frontend/src/api/ijsonJcs.test.ts new file mode 100644 index 00000000..b33184f5 --- /dev/null +++ b/frontend/src/api/ijsonJcs.test.ts @@ -0,0 +1,30 @@ +import { expect, test } from 'bun:test' +import { ijsonJcsStringify as jcs } from './ijsonJcs' + +test('JCS sorts by UTF-16 code units recursively and preserves array order', () => { + expect(jcs({ b: 1, a: { z: 1, x: [3, 2] }, 'A': 4, '😀': 5 })) + .toBe('{"a":{"x":[3,2],"z":1},"b":1,"😀":5,"A":4}') + // Numeric-like keys are sorted too, unlike JSON.stringify's object enumeration. + expect(jcs({ '10': 1, '2': 2 })).toBe('{"10":1,"2":2}') + expect(jcs({ optional: undefined, exact: '9007199254740992' })).toBe('{"exact":"9007199254740992"}') +}) + +test('ECMAScript number formatting and escaped controls match JCS', () => { + expect(jcs({ a: -0, b: 1e-7, c: 1e-6, d: '\n' })) + .toBe('{"a":0,"b":1e-7,"c":0.000001,"d":"\\n"}') +}) + +test('I-JSON rejects unsafe and non-finite numbers, noncharacters and lone surrogates', () => { + for (const value of [Number.MAX_SAFE_INTEGER + 1, NaN, Infinity, -Infinity]) { + expect(() => jcs({ count: value })).toThrow() + } + for (const text of ['\ud800', '\udc00', '\ufdd0', '\uffff', '\u{1ffff}']) { + expect(() => jcs({ text })).toThrow() + expect(() => jcs({ [text]: 1 })).toThrow() + } + expect(() => jcs({ a: [undefined] })).toThrow() + expect(() => jcs(new Array(1))).toThrow() + const cycle: { self?: unknown } = {} + cycle.self = cycle + expect(() => jcs(cycle)).toThrow() +}) diff --git a/frontend/src/api/ijsonJcs.ts b/frontend/src/api/ijsonJcs.ts new file mode 100644 index 00000000..e2c39463 --- /dev/null +++ b/frontend/src/api/ijsonJcs.ts @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// First-party API JSON: I-JSON values in RFC 8785 canonical form. JS string +// ordering is UTF-16 code-unit ordering, as required by JCS (unlike UTF-8). +// This deliberately doesn't canonicalise external jplace/Plotly documents. + +function validString(value: string): void { + for (let i = 0; i < value.length; i++) { + const code = value.codePointAt(i)! + if (code > 0xffff) i++ // a valid UTF-16 surrogate pair + else if (code >= 0xd800 && code <= 0xdfff) throw new TypeError('I-JSON refuses unpaired surrogates') + if ((code >= 0xfdd0 && code <= 0xfdef) || (code & 0xfffe) === 0xfffe) { + throw new TypeError('I-JSON refuses Unicode noncharacters') + } + } +} + +// JCS sorts UTF-16 code units, NOT human-language collation (localeCompare). +const compareUtf16 = (a: string, b: string): number => a < b ? -1 : a > b ? 1 : 0 + +/** Canonicalise a JSON-shaped API payload; reject values that cannot round-trip. */ +export function ijsonJcsStringify(value: unknown): string { + const active = new Set() + function encode(input: unknown): string { + if (input === null) return 'null' + if (typeof input === 'string') { validString(input); return JSON.stringify(input) } + if (typeof input === 'boolean') return String(input) + if (typeof input === 'number') { + if (!Number.isFinite(input) || (Number.isInteger(input) && !Number.isSafeInteger(input))) { + throw new TypeError('I-JSON refuses non-finite or unsafe numbers; use a decimal string for exact counts') + } + return JSON.stringify(input) + } + if (typeof input !== 'object') throw new TypeError(`Not an I-JSON value: ${typeof input}`) + if (active.has(input)) throw new TypeError('Cyclic JSON value') + if (!Array.isArray(input) && Object.getPrototypeOf(input) !== Object.prototype && Object.getPrototypeOf(input) !== null) { + throw new TypeError('I-JSON objects must be plain objects') + } + active.add(input) + try { + if (Array.isArray(input)) { + return `[${Array.from({ length: input.length }, (_, i) => { + if (!(i in input)) throw new TypeError('I-JSON refuses sparse arrays') + return encode(input[i]) + }).join(',')}]` + } + const object = input as Record + // Omit undefined optional properties just as JSON.stringify does, but + // never convert an undefined array element into null silently. + return `{${Object.keys(object).sort(compareUtf16).filter(key => object[key] !== undefined).map(key => { + validString(key) + return `${JSON.stringify(key)}:${encode(object[key])}` + }).join(',')}}` + } finally { + active.delete(input) + } + } + return encode(value) +} diff --git a/src/analysis/derived_analyses.jl b/src/analysis/derived_analyses.jl index 7d22a164..3ef09107 100644 --- a/src/analysis/derived_analyses.jl +++ b/src/analysis/derived_analyses.jl @@ -7,7 +7,7 @@ module DerivedAnalyses using JSON3 -using ..Canonical: jcs_bytes, jcs_sha256 +using ..Canonical: jcs_bytes, jcs_sha256, ijson_read export DerivedAnalysisError, derived_analysis_id, persist_derived_analysis, load_derived_analysis, MAX_DERIVED_BYTES @@ -43,13 +43,15 @@ function _validate_bytes(bytes::Vector{UInt8}) length(bytes) <= MAX_DERIVED_BYTES || throw(DerivedAnalysisError( "invalid_record", "derived analysis exceeds the $MAX_DERIVED_BYTES-byte read limit")) record = try - JSON3.read(bytes) + ijson_read(bytes) catch err throw(DerivedAnalysisError("invalid_record", "derived analysis is not valid JSON: $(sprint(showerror, err))")) end (record isa JSON3.Object || record isa AbstractDict) || throw(DerivedAnalysisError( "invalid_record", "derived analysis root must be a JSON object")) + jcs_bytes(record) == bytes || throw(DerivedAnalysisError( + "invalid_record", "derived analysis is not canonical I-JSON/JCS")) _get(record, "schema") == SCHEMA || throw(DerivedAnalysisError( "invalid_record", "derived analysis has an unsupported schema")) id = _get(record, "analysis_id") diff --git a/src/canonical.jl b/src/canonical.jl index 3da806a1..eeb35911 100644 --- a/src/canonical.jl +++ b/src/canonical.jl @@ -29,9 +29,9 @@ module Canonical -using SHA +using SHA, JSON3 -export jcs_bytes, jcs_sha256 +export jcs_bytes, jcs_sha256, ijson_read # The I-JSON exact-integer range. const SAFE_INT_MAX = 2^53 - 1 @@ -86,8 +86,18 @@ end _write!(io, ::AbstractFloat) = throw(ArgumentError( "canonical JSON supports only Float16, Float32 and Float64")) +# I-JSON disallows Unicode noncharacters (including the last two code points +# of every plane), whether they occur in values or property names. +function _valid_ijson_string(s::AbstractString) + isvalid(s) || return false + all(s) do c + cp = UInt32(c) + !(0xfdd0 <= cp <= 0xfdef || (cp & 0xfffe) == 0xfffe) + end +end + function _write!(io, s::AbstractString) - isvalid(s) || throw(ArgumentError("canonical JSON refuses invalid Unicode")) + _valid_ijson_string(s) || throw(ArgumentError("canonical JSON refuses invalid I-JSON Unicode")) print(io, '"') for c in s _write_char!(io, c) @@ -135,7 +145,7 @@ function _write!(io, d::AbstractDict) key isa Union{AbstractString,Symbol} || throw(ArgumentError( "canonical JSON object keys must be strings or symbols")) name = String(key) - isvalid(name) || throw(ArgumentError("canonical JSON refuses invalid Unicode keys")) + _valid_ijson_string(name) || throw(ArgumentError("canonical JSON refuses invalid I-JSON Unicode keys")) name in names && throw(ArgumentError("canonical JSON refuses duplicate property names")) push!(names, name) push!(entries, name => value) @@ -218,4 +228,133 @@ function _shortest_digits(x::Float64) digits, n end +""" + ijson_read(raw) -> JSON3.Object / JSON3.Array / scalar + +Parse a JSON document in the strict I-JSON mode of hyperpolymath/ijson-jcs. +Inspect the original tokens *before* relying on JSON3's parsed object: parsed +objects can lose duplicate names, and integral-looking floats/oversized integer +tokens can be normalised. The return value retains JSON3's usual types so +existing callers need not change their field access. +""" +function ijson_read(raw::Union{AbstractString,AbstractVector{UInt8}}) + text = raw isa AbstractString ? String(raw) : String(copy(raw)) + isvalid(text) || throw(ArgumentError("I-JSON requires UTF-8")) + # JSON3 parses the value; the raw-token walk also checks grammar and + # trailing content, which JSON3 may otherwise ignore. + parsed = try + JSON3.read(text) + catch err + throw(ArgumentError("invalid JSON: $(sprint(showerror, err))")) + end + bytes = codeunits(text) + n = length(bytes) + pos = Ref(1) + peek() = pos[] <= n ? bytes[pos[]] : 0x00 + function whitespace!() + while peek() in (0x20, 0x09, 0x0a, 0x0d) && pos[] <= n + pos[] += 1 + end + end + function string!() + start = pos[] + pos[] += 1 # opening quote + while pos[] <= n + c = bytes[pos[]] + pos[] += 1 + c == 0x22 && break + if c == 0x5c + # JSON3 checked the escape grammar; additionally reject lone + # surrogate escapes even if a parser replaces them with U+FFFD. + if bytes[pos[]] == 0x75 + high = parse(UInt32, String(bytes[pos[]+1:pos[]+4]); base=16) + if 0xd800 <= high <= 0xdbff + pos[] + 10 <= n && bytes[pos[]+5] == 0x5c && + bytes[pos[]+6] == 0x75 || throw(ArgumentError( + "I-JSON refuses an unpaired high surrogate")) + low = parse(UInt32, String(bytes[pos[]+7:pos[]+10]); base=16) + 0xdc00 <= low <= 0xdfff || throw(ArgumentError( + "I-JSON refuses an unpaired high surrogate")) + pos[] += 11 + continue + elseif 0xdc00 <= high <= 0xdfff + throw(ArgumentError("I-JSON refuses an unpaired low surrogate")) + end + pos[] += 5 + else + pos[] += 1 + end + end + end + value = String(JSON3.read(String(bytes[start:pos[]-1]))) + _valid_ijson_string(value) || throw(ArgumentError("I-JSON refuses Unicode noncharacters")) + value + end + function value!() + whitespace!() + if peek() == 0x7b # object + pos[] += 1 + whitespace!() + seen = Set{String}() + while peek() != 0x7d + key = string!() + key in seen && throw(ArgumentError("I-JSON refuses duplicate object key $(repr(key))")) + push!(seen, key) + whitespace!() + pos[] += 1 # colon (JSON3 already verified the grammar) + value!() + whitespace!() + peek() == 0x2c || break + pos[] += 1 + whitespace!() + end + pos[] += 1 # closing brace + elseif peek() == 0x5b # array + pos[] += 1 + whitespace!() + while peek() != 0x5d + value!() + whitespace!() + peek() == 0x2c || break + pos[] += 1 + whitespace!() + end + pos[] += 1 # closing bracket + elseif peek() == 0x22 + string!() + else + start = pos[] + while pos[] <= n && !(peek() in (0x2c, 0x5d, 0x7d, 0x20, 0x09, 0x0a, 0x0d)) + pos[] += 1 + end + token = String(bytes[start:pos[]-1]) + if occursin(r"^-?(0|[1-9][0-9]*)$", token) + # Bound the token before parsing: an HTTP body could contain + # millions of digits, but a safe integer has at most 16. + length(token) <= 17 && + -SAFE_INT_MAX <= parse(BigInt, token) <= SAFE_INT_MAX || + throw(ArgumentError("I-JSON refuses an unsafe integer token $token")) + elseif occursin(r"^-?(0|[1-9][0-9]*)\.[0-9]+([eE][+-]?[0-9]+)?$|^-?(0|[1-9][0-9]*)[eE][+-]?[0-9]+$", token) + x = tryparse(Float64, token) + (!isnothing(x) && isfinite(x)) || throw(ArgumentError( + "I-JSON refuses a non-finite number $token")) + # Exponent/fraction input may stringify as an integer in + # JCS. Never accept a value whose canonical token would be + # rejected as an unsafe integer on the next read. + canonical = String(jcs_bytes(x)) + if occursin(r"^-?(0|[1-9][0-9]*)$", canonical) && + !(abs(x) <= SAFE_INT_MAX) + throw(ArgumentError("I-JSON refuses a number canonicalizing to an unsafe integer token $canonical")) + end + elseif !(token in ("true", "false", "null")) + throw(ArgumentError("invalid JSON token $token")) + end + end + end + value!() + whitespace!() + pos[] > n || throw(ArgumentError("invalid JSON: trailing content")) + parsed +end + end # module diff --git a/src/server/routes/figures.jl b/src/server/routes/figures.jl index 94aeb804..3808ebdf 100644 --- a/src/server/routes/figures.jl +++ b/src/server/routes/figures.jl @@ -26,21 +26,21 @@ function _figure_request_error(study::String, id::Union{String,Nothing}=nothing) end function _read_figure(study::String, id::String) - try JSON3.read(read(_figure_path(study, id), String)) catch; nothing end + try ijson_read(read(_figure_path(study, id))) catch; nothing end end @get "/api/v1/studies/{study}/figures" function(req, study::String) err = _figure_request_error(study) isnothing(err) || return err dir = _figures_dir(study) - isdir(dir) || return json([]) + isdir(dir) || return jcs_json([]) ids = [splitext(f)[1] for f in readdir(dir) if endswith(f, ".json") && _valid_figure_id(splitext(f)[1])] list = map(ids) do id doc = _read_figure(study, id) (; id, title = doc isa JSON3.Object ? string(get(doc, :title, id)) : id, modified = string(unix2datetime(mtime(_figure_path(study, id))))) end - json(sort(list; by = x -> x.modified, rev=true)) + jcs_json(sort(list; by = x -> x.modified, rev=true)) end @get "/api/v1/studies/{study}/figures/{id}" function(req, study::String, id::String) @@ -64,12 +64,12 @@ end function _save_figure(req, study::String, id::String) length(req.body) <= _FIGURE_MAX_BYTES || return json_error(413, "too_large", "Figure documents are limited to 2 MB") - doc = try JSON3.read(String(req.body)) catch; nothing end + doc = try ijson_read(req.body) catch; nothing end doc isa JSON3.Object || return json_error(400, "invalid_body", "Body must be a JSON object") out = Dict{String,Any}(String(k) => v for (k, v) in doc) out["id"] = id - _write_atomic(_figure_path(study, id), JSON3.write(out)) - json(out) + _write_atomic(_figure_path(study, id), String(jcs_bytes(out))) + jcs_json(out) end # Rendered copies are named after the figure's title and end in _{id}.png. @@ -92,7 +92,7 @@ _figure_rasters(study::String, id::String) = write(tmp, bytes) foreach(f -> f == path || rm(f; force=true), _figure_rasters(study, id)) mv(tmp, path; force=true) - json((; file=basename(path))) + jcs_json((; file=basename(path))) end @delete "/api/v1/studies/{study}/figures/{id}" function(req, study::String, id::String) @@ -100,5 +100,5 @@ end isnothing(err) || return err foreach(f -> rm(f; force=true), _figure_rasters(study, id)) rm(_figure_path(study, id); force=true) - json((; deleted=id)) + jcs_json((; deleted=id)) end diff --git a/src/server/routes/report.jl b/src/server/routes/report.jl index 73ac45a6..b1959738 100644 --- a/src/server/routes/report.jl +++ b/src/server/routes/report.jl @@ -19,14 +19,14 @@ _report_index(study::String) = joinpath(_report_dir(study), "items.json") function _report_items(study::String)::Vector{Dict{String,Any}} path = _report_index(study) isfile(path) || return Dict{String,Any}[] - raw = try JSON3.read(read(path, String)) catch; return Dict{String,Any}[] end + raw = try ijson_read(read(path)) catch; return Dict{String,Any}[] end [Dict{String,Any}(String(k) => v for (k, v) in item) for item in raw] end function _save_report_items(study::String, items) mkpath(_report_dir(study)) tmp = _report_index(study) * ".tmp" - write(tmp, JSON3.write(items)) + write(tmp, jcs_bytes(items)) mv(tmp, _report_index(study); force=true) end @@ -50,7 +50,7 @@ end @get "/api/v1/studies/{study}/report" function(req, study::String) err = _report_study_error(study) isnothing(err) || return err - json(_report_items(study)) + jcs_json(_report_items(study)) end # The file is the raw request body; kind, title and extension come as query parameters. @@ -64,6 +64,14 @@ end ext in _REPORT_EXTENSIONS || return json_error(400, "invalid_ext", "ext must be one of $(join(_REPORT_EXTENSIONS, ", "))") title = strip(get(q, "title", "")) isempty(title) && (title = "Untitled $kind") + # Reject invalid I-JSON Unicode before writing the upload: an invalid + # title would otherwise make the index unsavable and orphan the file. + try + jcs_bytes(title) + catch e + e isa ArgumentError || rethrow() + return json_error(400, "invalid_title", "Title contains invalid Unicode") + end bytes = req.body isempty(bytes) && return json_error(400, "empty", "The item has no content") length(bytes) <= _REPORT_MAX_BYTES || return json_error(413, "too_large", "Report items are limited to 50 MB") @@ -75,13 +83,13 @@ end item = Dict{String,Any}("id" => id, "kind" => kind, "title" => title, "file" => file, "added" => string(now(UTC))) _report_edit(items -> push!(items, item), study) - json(item) + jcs_json(item) end @patch "/api/v1/studies/{study}/report/{id}" function(req, study::String, id::String) err = _report_study_error(study) isnothing(err) || return err - body = try JSON3.read(String(req.body)) catch; nothing end + body = try ijson_read(req.body) catch; nothing end title = body isa JSON3.Object ? get(body, :title, nothing) : nothing title isa AbstractString && !isempty(strip(title)) || return json_error(400, "invalid_title", "Body must include a non-empty 'title'") @@ -91,13 +99,13 @@ end items[i]["title"] = strip(title) items[i] end - isnothing(found) ? json_error(404, "not_found", "No report item '$id'") : json(found) + isnothing(found) ? json_error(404, "not_found", "No report item '$id'") : jcs_json(found) end @put "/api/v1/studies/{study}/report/order" function(req, study::String) err = _report_study_error(study) isnothing(err) || return err - body = try JSON3.read(String(req.body)) catch; nothing end + body = try ijson_read(req.body) catch; nothing end ids = body isa JSON3.Object ? get(body, :ids, nothing) : nothing ids isa AbstractVector || return json_error(400, "invalid_body", "Body must include 'ids'") ordered = _report_edit(study) do items @@ -105,7 +113,7 @@ end sort!(items; by = it -> get(rank, it["id"], typemax(Int))) copy(items) end - json(ordered) + jcs_json(ordered) end @delete "/api/v1/studies/{study}/report/{id}" function(req, study::String, id::String) @@ -118,7 +126,7 @@ end end isnothing(removed) && return json_error(404, "not_found", "No report item '$id'") rm(joinpath(_report_dir(study), removed["file"]); force=true) - json((; deleted=id)) + jcs_json((; deleted=id)) end const _REPORT_MIME = Dict(".svg" => "image/svg+xml", ".png" => "image/png", ".json" => "application/json", diff --git a/src/server/routes/trees.jl b/src/server/routes/trees.jl index 2d1cd348..d7edfb94 100644 --- a/src/server/routes/trees.jl +++ b/src/server/routes/trees.jl @@ -81,7 +81,7 @@ end @get "/api/v1/studies/{study}/trees" function(req, study::String) err = _tree_request_error(study) isnothing(err) || return err - json(_tree_listing(study)) + jcs_json(_tree_listing(study)) end @get "/api/v1/studies/{study}/trees/{file}" function(req, study::String, file::String) @@ -91,8 +91,8 @@ end isfile(path) || return json_error(404, "not_found", "Tree '$file' not found") content = read(path, String) vpath = _tree_view_path(study, file) - view = isfile(vpath) ? try JSON3.read(read(vpath, String)) catch; nothing end : nothing - json((; file, + view = isfile(vpath) ? try ijson_read(read(vpath)) catch; nothing end : nothing + jcs_json((; file, format = _tree_format(file), sha256 = bytes2hex(sha256(content)), modified = string(unix2datetime(stat(path).mtime)), @@ -103,7 +103,7 @@ end @post "/api/v1/studies/{study}/trees" function(req, study::String) err = _tree_request_error(study) isnothing(err) || return err - body = try JSON3.read(String(req.body)) catch + body = try ijson_read(req.body) catch return json_error(400, "invalid_body", "Expected a JSON body") end file = string(get(body, :file, "")) @@ -119,7 +119,7 @@ end return json_error(409, "exists", "Tree '$file' already exists") end _write_atomic(path, String(content)) - json((; file, format = _tree_format(file))) + jcs_json((; file, format = _tree_format(file))) end @put "/api/v1/studies/{study}/trees/{file}/view" function(req, study::String, file::String) @@ -129,12 +129,12 @@ end return json_error(404, "not_found", "Tree '$file' not found") text = String(req.body) sizeof(text) <= _VIEW_MAX_BYTES || return json_error(413, "too_large", "View state is limited to 5 MB") - doc = try JSON3.read(text) catch + doc = try ijson_read(text) catch return json_error(400, "invalid_body", "Expected a JSON object") end doc isa JSON3.Object || return json_error(400, "invalid_body", "Expected a JSON object") - _write_atomic(_tree_view_path(study, file), text) - json((; saved = true)) + _write_atomic(_tree_view_path(study, file), String(jcs_bytes(doc))) + jcs_json((; saved = true)) end @delete "/api/v1/studies/{study}/trees/{file}" function(req, study::String, file::String) @@ -145,5 +145,5 @@ end rm(path) vpath = _tree_view_path(study, file) isfile(vpath) && rm(vpath) - json((; deleted = file)) + jcs_json((; deleted = file)) end diff --git a/src/server/routes/validated_statistics.jl b/src/server/routes/validated_statistics.jl index 047302ba..56e797ee 100644 --- a/src/server/routes/validated_statistics.jl +++ b/src/server/routes/validated_statistics.jl @@ -41,12 +41,8 @@ end function _statistics_body(req; reject_noninteger_tokens::Bool=false) length(req.body) <= _STATISTICS_REQUEST_LIMIT || throw(ResourceLimitError( "request bytes", _STATISTICS_REQUEST_LIMIT, length(req.body))) - source = String(req.body) - parsed = try - JSON3.read(source) - catch - throw(ArgumentError("request body is not valid JSON")) - end + source = String(copy(req.body)) + parsed = ijson_read(source) parsed isa JSON3.Object || throw(ArgumentError("request body must be a JSON object")) reject_noninteger_tokens && _reject_noninteger_json_tokens(source) parsed diff --git a/src/server/server.jl b/src/server/server.jl index eca368a0..93e0717a 100644 --- a/src/server/server.jl +++ b/src/server/server.jl @@ -14,6 +14,7 @@ module Server using Oxygen, HTTP, JSON3, YAML, Logging using PrecompileTools: @setup_workload, @compile_workload + using ..Canonical: ijson_read, jcs_bytes using ..PipelineTypes, ..PipelineLog, ..Config using ..Databases, ..DuckDBStore, ..Validation using ..Tools, ..TaxonomyTableTools, ..ProjectSetup @@ -63,11 +64,22 @@ module Server using .JobQueue ## Shared helpers (available to all included route files) + function jcs_json(payload; status::Int=200) + HTTP.Response(status, ["Content-Type" => "application/json"], body=jcs_bytes(payload)) + end + function json_error(status::Int, code::String, message::String; detail=nothing) body = isnothing(detail) ? (; error=code, message) : (; error=code, message, detail) - HTTP.Response(status, - ["Content-Type" => "application/json"], - body = JSON3.write(body)) + bytes = try + jcs_bytes(body) + catch e + e isa ArgumentError || rethrow() + # Error messages can interpolate untrusted path/query segments. + # If they contain invalid I-JSON Unicode, still return the intended + # HTTP status rather than turning a client error into a server error. + jcs_bytes((; error=code, message="Invalid request")) + end + HTTP.Response(status, ["Content-Type" => "application/json"], body=bytes) end ## Route registration diff --git a/test/unit/test_canonical.jl b/test/unit/test_canonical.jl index 064317f7..3be5ae09 100644 --- a/test/unit/test_canonical.jl +++ b/test/unit/test_canonical.jl @@ -5,7 +5,7 @@ # ECMAScript JSON.stringify and UTF-16 key order. Rust fixture generation # provenance has not been reproduced in this audit. using MetaManifold -using MetaManifold.Canonical: jcs_bytes, jcs_sha256 +using MetaManifold.Canonical: jcs_bytes, jcs_sha256, ijson_read using JSON3, SHA, Test @testset "canonical JSON (JCS)" begin @@ -96,3 +96,33 @@ end cases = readlines(joinpath(@__DIR__, "..", "fixtures", "jcs_oracle.jsonl")) @test JSON3.read(cases[5]).input[2] isa AbstractFloat end + +@testset "strict I-JSON input and JCS output (ijson-jcs)" begin + for input in ("{\"a\":1,\"a\":2}", + "{\"a\":1,\"\\u0061\":2}", + "{\"nested\":[{\"b\":0,\"b\":1}]}", + "{\"n\":9007199254740992}", + "{\"n\":9.007199254740992e15}", + "{\"n\":-9.007199254740992e15}", + "{\"n\":2.4499999999999998e16}", + "[-9007199254740992]", "[1e400]", "{\"key\":\"\\ud800\"}", + "{\"key\":\"\\udc00\"}", + "{\"\\ufdd0\":0}", "[\"\\uFFFF\"]", + "01", "1.", "-", "[1] x", "{\"a\":1}{\"a\":2}") + @test_throws ArgumentError ijson_read(input) + end + @test_throws ArgumentError ijson_read(UInt8[0xff]) + @test_throws ArgumentError jcs_bytes(Dict("bad" => "\ufdd0")) + @test_throws ArgumentError jcs_bytes(Dict("\U0001ffff" => 1)) + for input in ("{\"a\":1,\"b\":[true,null]}", + "{\"emoji\":\"\\ud83d\\ude00\"}", + "{\"exact\":\"9007199254740992\"}", + "{\"float\":1e-7}") + @test ijson_read(String(jcs_bytes(ijson_read(input)))) isa JSON3.Object + end + @test ijson_read("{\"n\":1e21}").n == 1e21 # JCS retains exponent notation + @test ijson_read("{\"n\":9e15}").n == 9e15 # within exact range + @test String(jcs_bytes(ijson_read("{\"b\":2,\"a\":1}"))) == "{\"a\":1,\"b\":2}" + @test ijson_read(Vector{UInt8}(codeunits("{\"a\":1}"))) isa JSON3.Object + @test ijson_read("[1] \n") isa JSON3.Array +end diff --git a/test/unit/test_clade_cumulus.jl b/test/unit/test_clade_cumulus.jl index 3b887de9..3876d41c 100644 --- a/test/unit/test_clade_cumulus.jl +++ b/test/unit/test_clade_cumulus.jl @@ -417,7 +417,25 @@ echo "(MMTIP_000001:0.05,(MMTIP_000002:0.03,MMTIP_000003:0.04)0.910:0.12,MMTIP_0 @test v_allow.proposal_file !== nothing @test isfile(joinpath(trees_dir, ".clade_cumulus", String(v_allow.proposal_file))) @test v_allow.proposed_root.cumulative_count ≈ 110.0 atol = 1e-9 - @test Float64(v_allow.proposed_root.children[1].subtree_pd) > before_target_pd + # Subtree PD includes each active edge exactly once, including the + # target's own incoming edge (main's node-local PD convention). + function proposed_node(u, id) + u.id == id && return u + for child in u.children + found = proposed_node(child, id) + found !== nothing && return found + end + nothing + end + proposed_target = proposed_node(v_allow.proposed_root, high_clade.id) + @test proposed_target !== nothing + if proposed_target !== nothing + expected_pd = max(0.0, Float64(proposed_target.branch_length)) + + sum((Float64(ch.subtree_pd) for ch in proposed_target.children + if ch.cumulative_count > 0); init=0.0) + @test Float64(proposed_target.subtree_pd) ≈ expected_pd atol = 1e-10 + @test Float64(proposed_target.subtree_pd) != before_target_pd + end # Re-saving under a different threshold writes a distinct immutable sidecar file r_allow2 = _cc_req("POST", "/api/v1/studies/StudyCC/runs/run1/clade-cumulus/validate-move", JSON3.write(Dict( diff --git a/test/unit/test_derived_analyses.jl b/test/unit/test_derived_analyses.jl index 1f2a2bd8..dc06d3a3 100644 --- a/test/unit/test_derived_analyses.jl +++ b/test/unit/test_derived_analyses.jl @@ -74,5 +74,10 @@ end # The verified original is still available after the failed conflicting use. @test load_derived_analysis(directory, id)["analysis_id"] == id @test object["analysis_id"] == id + + # A record with the right semantic ID but noncanonical bytes is not a + # valid content-addressed I-JSON/JCS document. + write(joinpath(tampered, "$id.json"), vcat(UInt8[0x20], read(joinpath(directory, "$id.json")))) + @test_throws DerivedAnalysisError load_derived_analysis(tampered, id) end end diff --git a/test/unit/test_exact_summaries.jl b/test/unit/test_exact_summaries.jl index 098667c8..d78f23e3 100644 --- a/test/unit/test_exact_summaries.jl +++ b/test/unit/test_exact_summaries.jl @@ -14,10 +14,10 @@ const EXACT_FEATURES = ["f1", "f2", "f3"] @test !s.approximate @test s.samples[1].total == 3 @test s.samples[1].proportions == fill(big(1)//3, 3) - # A zero count in a sample with a positive total has an exact proportion - # of 0 (the independent Python fractions reference below agrees); only a - # zero total leaves proportions undefined. - @test s.samples[2].proportions == Union{Rational{BigInt},Nothing}[big(1)//1, big(0)//1, big(0)//1] + # Sample B has [2, 0, 0] reads: a zero count in a sample that has reads is a + # defined proportion of zero. Only a zero *total* leaves proportions + # undefined, and this sample's total is 2. + @test s.samples[2].proportions == Union{Rational{BigInt},Nothing}[big(1), big(0), big(0)] @test length(s.groups) == 2 @test s.groups[1].label == "g1" @test s.groups[2].counts == BigInt[2, 0, 0] @@ -31,7 +31,7 @@ const EXACT_FEATURES = ["f1", "f2", "f3"] @test storage["input_status"] == "exact" @test storage["samples"][1]["proportions"] == ["1/3", "1/3", "1/3"] @test storage["samples"][1]["total"] == 3 - @test storage["samples"][2]["proportions"][2] == "0/1" + @test storage["samples"][2]["proportions"] == ["1/1", "0/1", "0/1"] @test length(jcs_sha256(storage)) == 64 display = summary_to_display(s; digits=8) @test occursin("exact", display) diff --git a/test/unit/test_report_funnel.jl b/test/unit/test_report_funnel.jl index 6dc69c44..d6ef004e 100644 --- a/test/unit/test_report_funnel.jl +++ b/test/unit/test_report_funnel.jl @@ -3,6 +3,7 @@ using MetaManifold SV = MetaManifold.Server using JSON3, YAML +using MetaManifold.Canonical: jcs_bytes _rf_request(method, path, body="", headers=["Content-Type" => "application/json"]) = SV.Oxygen.internalrequest(SV.HTTP.Request(method, path, headers, body)) @@ -18,10 +19,18 @@ _rf_request(method, path, body="", headers=["Content-Type" => "application/json" base = "/api/v1/studies/StudyR/report" @test isempty(JSON3.read(String(_rf_request("GET", base).body))) svg = "" + @test _rf_request("POST", "$base?kind=figure&title=%EF%B7%90&ext=.svg", svg, + ["Content-Type" => "application/octet-stream"]).status == 400 + @test !isdir(joinpath(tmp, "projects", "StudyR", "report")) r = _rf_request("POST", "$base?kind=figure&title=Alpha&ext=.svg", svg, ["Content-Type" => "application/octet-stream"]) @test r.status == 200 - a = JSON3.read(String(r.body)) + # String(Vector{UInt8}) may take ownership and empty the input vector. + # Keep the response bytes intact for the canonical-byte assertion. + a = JSON3.read(String(copy(r.body))) + @test r.body == jcs_bytes(a) + @test read(joinpath(tmp, "projects", "StudyR", "report", "items.json")) == + jcs_bytes([a]) r = _rf_request("POST", "$base?kind=table&title=Tables&ext=.csv", "a,b\n1,2\n", ["Content-Type" => "application/octet-stream"]) b = JSON3.read(String(r.body)) @@ -132,11 +141,15 @@ end @test isempty(JSON3.read(String(_rf_request("GET", base).body))) r = _rf_request("POST", base, JSON3.write(Dict("title" => "Fig 1", "groups" => []))) @test r.status == 200 + @test r.body == jcs_bytes(JSON3.read(String(copy(r.body)))) id = JSON3.read(String(r.body)).id @test occursin(r"^[0-9a-f]{8}$", id) r = _rf_request("PUT", "$base/$id", JSON3.write(Dict("id" => "ffffffff", "title" => "Renamed"))) @test JSON3.read(String(r.body)).id == id @test JSON3.read(String(_rf_request("GET", "$base/$id").body)).title == "Renamed" + @test read(joinpath(tmp, "projects", "StudyF", "figures", "$id.json")) == + jcs_bytes(JSON3.read(String(_rf_request("GET", "$base/$id").body))) + @test _rf_request("PUT", "$base/$id", "{\"title\":\"x\",\"title\":\"y\"}").status == 400 @test only(JSON3.read(String(_rf_request("GET", base).body))).title == "Renamed" @test _rf_request("PUT", "$base/$id", "[1,2]").status == 400 @test _rf_request("GET", "$base/nothex12").status == 400 diff --git a/test/unit/test_server_security.jl b/test/unit/test_server_security.jl index b108de6e..eee51fc7 100644 --- a/test/unit/test_server_security.jl +++ b/test/unit/test_server_security.jl @@ -163,6 +163,15 @@ end @test _ss_served("/api/v1/studies").status == 200 end + @testset "Invalid Unicode in client errors keeps its HTTP status" begin + r = SV.json_error(404, "study_not_found", "Study '\ufdd0' not found") + @test r.status == 404 + @test JSON3.read(String(copy(r.body))).message == "Invalid request" + r = SV.json_error(400, "bad_request", "Invalid group", detail="\U0001ffff") + @test r.status == 400 + @test JSON3.read(String(copy(r.body))).error == "bad_request" + end + @testset "Report uploads are served inert" begin host = "Host" => "127.0.0.1:8080" base = "/api/v1/studies/StudyS/report" diff --git a/test/unit/test_trees.jl b/test/unit/test_trees.jl index 61428469..89053ccc 100644 --- a/test/unit/test_trees.jl +++ b/test/unit/test_trees.jl @@ -2,6 +2,7 @@ using MetaManifold SV = MetaManifold.Server using JSON3 +using MetaManifold.Canonical: jcs_bytes function _tr_request(method::String, path::String, body=nothing) payload = isnothing(body) ? "" : body isa AbstractString ? body : JSON3.write(body) @@ -50,6 +51,8 @@ _tr_json(res) = JSON3.read(String(res.body)) view = Dict("version" => 1, "ops" => [Dict("op" => "collapse", "clade" => "abc")]) @test _tr_request("PUT", "$base/t.jplace/view", view).status == 200 + @test read(joinpath(tmp, "projects", "StudyT", "trees", ".views", "t.jplace.json")) == jcs_bytes(view) + @test _tr_request("PUT", "$base/t.jplace/view", "{\"ops\":[],\"ops\":[]}").status == 400 @test _tr_request("PUT", "$base/t.jplace/view", "[1,2]").status == 400 @test _tr_request("PUT", "$base/missing.nwk/view", view).status == 404 got = _tr_json(_tr_request("GET", "$base/t.jplace")) diff --git a/test/unit/test_validated_statistics.jl b/test/unit/test_validated_statistics.jl index a196f5a9..ea64cbcd 100644 --- a/test/unit/test_validated_statistics.jl +++ b/test/unit/test_validated_statistics.jl @@ -92,7 +92,7 @@ end @testset "Numeric JSON representations fail closed" begin _vs_with_root() do _ - for bad in (1.5, -1, 9007199254740993) + for bad in (1.5, -1) payload = merge(_VS_EXACT_BODY, Dict("persist" => false, "counts" => [[bad, 1], [1, 0], [1, 0]])) response = _vs_post("/api/v1/statistics/summaries", payload) @@ -113,6 +113,17 @@ end @test _vs_post("/api/v1/statistics/summaries", Vector{UInt8}(codeunits(integer_raw)); raw=true).status == 200 end + + # Fractional input stays invalid as well, even when it is nearly 1. + fractional = """{"numeric_policy":{"mode":"exact_counts"},"sample_labels":["A"],"feature_labels":["f"],"counts":[[1.0000000001]]}""" + @test _vs_post("/api/v1/statistics/summaries", fractional; raw=true).status == 422 + # Strict I-JSON is checked on the raw body, before JSON3 can collapse + # duplicate keys or normalise an oversized integer token. Unlike a + # negative count, an oversized JSON number is invalid at parse time. + for raw in ("""{"numeric_policy":{"mode":"exact_counts","mode":"high_precision"},"sample_labels":["A"],"feature_labels":["f"],"counts":[[1]]}""", + """{"numeric_policy":{"mode":"exact_counts"},"sample_labels":["A"],"feature_labels":["f"],"counts":[[9007199254740992]]}""") + @test _vs_post("/api/v1/statistics/summaries", raw; raw=true).status == 400 + end negative_string = merge(_VS_EXACT_BODY, Dict("persist" => false, "counts" => [["-1", 1], [1, 0], [1, 0]])) @test _vs_post("/api/v1/statistics/summaries", negative_string).status == 422