diff --git a/src/analysis/exact_summaries.jl b/src/analysis/exact_summaries.jl index 75fd6d61..a02985a3 100644 --- a/src/analysis/exact_summaries.jl +++ b/src/analysis/exact_summaries.jl @@ -260,7 +260,9 @@ function summary_to_display(s::ExactSummary; digits::Integer=s.policy.round_digi continue end for (feature, proportion) in zip(x.features, x.proportions) - println(io, " ", feature, ": ", to_display(exact_value(proportion); digits)) + rendered = isnothing(proportion) ? "undefined (not observed)" : + to_display(exact_value(proportion); digits) + println(io, " ", feature, ": ", rendered) end end end diff --git a/src/server/routes/validated_statistics.jl b/src/server/routes/validated_statistics.jl index ce214c7d..fb14d584 100644 --- a/src/server/routes/validated_statistics.jl +++ b/src/server/routes/validated_statistics.jl @@ -79,6 +79,135 @@ function _statistics_policy(body) round_digits=_policy_integer(raw, :round_digits, 6)) end +# JSON3 collapses an integral float token (1.0, 1e0) to an integer when it +# parses, so the token form of `counts` entries can only be checked on the +# raw request text. This scanner walks the top-level object to the `counts` +# member and rejects any numeric token there that is not a plain integer +# token. Strings are skipped properly; their contents are validated by +# _json_exact_count after parsing. +function _reject_float_count_tokens(raw::AbstractString) + text = String(raw) + bytes = codeunits(text) + n = length(bytes) + pos = Ref(1) + at_end() = pos[] > n + peek() = at_end() ? 0x00 : bytes[pos[]] + function skip_ws() + while !at_end() && bytes[pos[]] in (0x20, 0x09, 0x0a, 0x0d) + pos[] += 1 + end + end + function parse_string() + # At the opening quote; returns the decoded string. + pos[] += 1 + out = IOBuffer() + while !at_end() + byte = bytes[pos[]] + pos[] += 1 + if byte == 0x22 # " + return String(take!(out)) + elseif byte == 0x5c # backslash + at_end() && break + esc = bytes[pos[]] + pos[] += 1 + if esc == 0x75 && pos[] + 3 <= n # \uXXXX + hex = text[pos[]:pos[] + 3] + code = tryparse(UInt32, hex; base=16) + isnothing(code) || print(out, Char(code)) + pos[] += 4 + else + print(out, Char(esc)) + end + else + write(out, byte) + end + end + String(take!(out)) + end + function skip_value() + skip_ws() + byte = peek() + if byte == 0x22 + parse_string() + elseif byte in (0x7b, 0x5b) # { [ + depth = 0 + while !at_end() + current = peek() + if current == 0x22 + parse_string() + continue + end + pos[] += 1 + if current in (0x7b, 0x5b) + depth += 1 + elseif current in (0x7d, 0x5d) # } ] + depth -= 1 + depth == 0 && break + end + end + else + while !at_end() && !(bytes[pos[]] in (0x2c, 0x7d, 0x5d, 0x20, 0x09, 0x0a, 0x0d)) + pos[] += 1 + end + end + end + function scan_counts_value() + # At the first byte of the counts value: check every numeric token + # until the value ends, skipping strings and nesting. + depth = 0 + while !at_end() + byte = peek() + if byte == 0x22 + parse_string() + continue + elseif byte in (0x7b, 0x5b) + depth += 1 + pos[] += 1 + elseif byte in (0x7d, 0x5d) + depth -= 1 + pos[] += 1 + depth <= 0 && break + elseif byte == 0x2d || (0x30 <= byte <= 0x39) # - or digit + start = pos[] + while !at_end() && (bytes[pos[]] in (0x2d, 0x2b, 0x2e, 0x65, 0x45) || + 0x30 <= bytes[pos[]] <= 0x39) + pos[] += 1 + end + token = text[start:pos[] - 1] + occursin(r"^-?[0-9]+$", token) || throw(UnsupportedRepresentationError( + "integer JSON tokens in counts", token, + "counts; floating-point JSON tokens are not exact input, " * + "even when their value is integral")) + depth == 0 && break + else + pos[] += 1 + byte == 0x2c && depth == 0 && break # comma ends a scalar value + end + end + end + + skip_ws() + peek() == 0x7b || return nothing # not an object; _statistics_body refuses it + pos[] += 1 + while true + skip_ws() + peek() == 0x22 || return nothing + key = parse_string() + skip_ws() + peek() == 0x3a || return nothing # : + pos[] += 1 + skip_ws() + if key == "counts" + scan_counts_value() + return nothing + end + skip_value() + skip_ws() + peek() == 0x2c || return nothing + pos[] += 1 + end +end + function _statistics_labels(raw, name::String) raw isa AbstractVector || throw(ArgumentError("$name must be an array of strings")) all(x -> x isa AbstractString, raw) || throw(ArgumentError("$name must contain only strings")) @@ -95,8 +224,9 @@ function _json_exact_count(value, feature::String, sample::String) "feature '$feature', sample '$sample'; use a canonical whole-number string")) end end - # JSON3 represents a decimal/exponent token as a float even when its value - # is mathematically integral. Reject the representation, not just 1.5. + # A non-integral float token (1.5) reaches here as a float and is refused. + # Integral float tokens (1.0, 1e0) are collapsed to integers by JSON3, so + # _reject_float_count_tokens refuses those on the raw text before parsing. (value isa Integer && !(value isa Bool)) || throw(UnsupportedRepresentationError( "an integer JSON token or whole-number string", repr(value), "feature '$feature', sample '$sample'; floating-point JSON is not exact input")) @@ -218,31 +348,45 @@ function _validated_duckdb_counts(con, table::String, columns::Vector{String}; "COALESCE(NULLIF(CAST($(_sql_identifier(taxon)) AS VARCHAR), ''), 'Unassigned')" end count_columns = join([_sql_identifier(c) for c in samples], ", ") + # Rows sharing a feature (e.g. several sequences in one genus) are summed + # here, in BigInt, not in SQL: DuckDB refuses bare count columns beside + # GROUP BY, and SUM over HUGEINT counts would overflow exactly the values + # this path exists to keep exact. ORDER BY keeps features sorted and each + # feature's rows adjacent. sql = "SELECT $feature_expression AS feature, $count_columns FROM $(_sql_identifier(table)) " * - "$where_clause GROUP BY $feature_expression ORDER BY feature LIMIT $(MAX_CELLS + 1)" + "$where_clause ORDER BY feature LIMIT $(MAX_CELLS + 1)" frame = DataFrame(DBInterface.execute(con, sql, where_params)) - nrow(frame) <= MAX_CELLS || throw(ResourceLimitError("features", MAX_CELLS, nrow(frame))) + nrow(frame) <= MAX_CELLS || throw(ResourceLimitError("rows", MAX_CELLS, nrow(frame))) nrow(frame) > 0 || throw(ArgumentError("no rows remain after filtering")) - nrow(frame) * length(samples) <= MAX_CELLS || throw(ResourceLimitError( - "table cells", MAX_CELLS, nrow(frame) * length(samples))) - features = Vector{String}(undef, nrow(frame)) - counts = Matrix{BigInt}(undef, nrow(frame), length(samples)) + features = String[] + feature_rows = Vector{BigInt}[] for (i, row) in enumerate(eachrow(frame)) ismissing(row.feature) && throw(UnsupportedRepresentationError( "a non-missing feature label", "missing", "validated run statistics row $i")) - features[i] = string(row.feature) + feature = string(row.feature) + if isempty(features) || features[end] != feature + push!(features, feature) + push!(feature_rows, zeros(BigInt, length(samples))) + end + target = feature_rows[end] for (j, sample) in enumerate(samples) value = row[Symbol(sample)] ismissing(value) && throw(UnsupportedRepresentationError( - "a non-missing integer count", "NULL", "feature '$(features[i])', sample '$sample'")) + "a non-missing integer count", "NULL", "feature '$feature', sample '$sample'")) (value isa Integer && !(value isa Bool)) || throw(UnsupportedRepresentationError( - "an integer count", repr(value), "feature '$(features[i])', sample '$sample'")) + "an integer count", repr(value), "feature '$feature', sample '$sample'")) value >= 0 || throw(ArgumentError( - "negative count $value at feature '$(features[i])', sample '$sample'")) - counts[i, j] = BigInt(value) # every addition later is exact BigInt + "negative count $value at feature '$feature', sample '$sample'")) + target[j] += BigInt(value) # every addition is exact BigInt end end + counts = Matrix{BigInt}(undef, length(features), length(samples)) + for (i, feature_row) in enumerate(feature_rows) + counts[i, :] = feature_row + end + length(features) * length(samples) <= MAX_CELLS || throw(ResourceLimitError( + "table cells", MAX_CELLS, length(features) * length(samples))) ExactSummaries.validate_coordinates(counts, samples, features) counts, samples, features end @@ -281,6 +425,7 @@ end @post "/api/v1/statistics/summaries" function(req) _statistics_guard() do body = _statistics_body(req) + _reject_float_count_tokens(String(req.body)) policy = _statistics_policy(body) counts, labels, features, groups = _direct_count_input(body) result = _run_statistics(counts, labels, features, groups, policy) diff --git a/src/server/server.jl b/src/server/server.jl index ebf5abb6..eca368a0 100644 --- a/src/server/server.jl +++ b/src/server/server.jl @@ -118,7 +118,6 @@ module Server include(joinpath(@__DIR__, "routes", "phylogeny.jl")) include(joinpath(@__DIR__, "routes", "cladistics.jl")) include(joinpath(@__DIR__, "routes", "clade_cumulus.jl")) - include(joinpath(@__DIR__, "routes", "cladistics.jl")) ## Bad request middleware # Helpers deep in a route (e.g. `_req_group`) throw this to reject a request. diff --git a/test/unit/test_exact_summaries.jl b/test/unit/test_exact_summaries.jl index e1ace22d..098667c8 100644 --- a/test/unit/test_exact_summaries.jl +++ b/test/unit/test_exact_summaries.jl @@ -14,7 +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) - @test s.samples[2].proportions == Union{Rational{BigInt},Nothing}[big(1), nothing, nothing] + # 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] @test length(s.groups) == 2 @test s.groups[1].label == "g1" @test s.groups[2].counts == BigInt[2, 0, 0] @@ -28,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] === nothing + @test storage["samples"][2]["proportions"][2] == "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_validated_statistics.jl b/test/unit/test_validated_statistics.jl index c19bee2c..7a7465d2 100644 --- a/test/unit/test_validated_statistics.jl +++ b/test/unit/test_validated_statistics.jl @@ -92,16 +92,20 @@ end @testset "Numeric JSON representations fail closed" begin _vs_with_root() do _ - for bad in (1.0, 1.5, -1, 9007199254740993) + for bad in (1.5, -1, 9007199254740993) payload = merge(_VS_EXACT_BODY, Dict("persist" => false, "counts" => [[bad, 1], [1, 0], [1, 0]])) response = _vs_post("/api/v1/statistics/summaries", payload) @test response.status == 422 @test JSON3.read(String(response.body)).error == "invalid_numeric_input" end - # Exponent notation is a float token even though its value is integral. - raw = """{"numeric_policy":{"mode":"exact_counts"},"sample_labels":["A"],"feature_labels":["f"],"counts":[[1e0]]}""" - @test _vs_post("/api/v1/statistics/summaries", raw; raw=true).status == 422 + # Decimal and exponent notation are float tokens even though their + # values are integral. These go through raw bodies: a Dict round-trip + # cannot represent the token distinction (JSON3.write(1.0) is "1"). + for token in ("1.0", "1e0") + raw = """{"numeric_policy":{"mode":"exact_counts"},"sample_labels":["A"],"feature_labels":["f"],"counts":[[$token]]}""" + @test _vs_post("/api/v1/statistics/summaries", raw; raw=true).status == 422 + 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