mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 08:09:27 +00:00
Hengfei Yang
663e2f75da
## Problem
`api_fuzz_tests` (schemathesis) intermittently reported a 500 on
`POST /api/{org_id}/streams/{stream_name}` with the body
`failed to parse header value`.
That string is not ours — it is the `http` crate's `InvalidHeaderValue`
display. The chain:
- ~20 sites return `(StatusCode, [(ERROR_HEADER, format!(...))],
Json(...))`,
where the message interpolates user-supplied names (org, stream, **field
type**).
- The failing payload carried `"type": "v^0K…"`. `HeaderValue` accepts
only `b >= 32 && b != 127 || b == b'\t'`, so the control byte makes
header
construction fail.
- axum's `TryIntoHeaderError` then **discards the whole response** and
substitutes `(500, inner.to_string())`.
So the handler's real 400 and its JSON body never reached the client.
The
status code was not chosen by the handler at all — axum overwrote it.
The org name was never the trigger; it only had to exist so the request
got
past the auth check. The field type was the poison.
A second, quieter bug sits in the same place: non-ascii bytes *are*
accepted
into a header, but `HeaderValue::to_str()` only reads back `32..127`,
and the
audit middleware reads this header with exactly `to_str().ok()`. Any
error
mentioning a non-ascii stream name was being silently dropped from the
audit
log.
## Approach
Route every `X-Error-Message` write through one chokepoint in
`common::meta::http`:
- `error_header_value()` escapes anything outside visible ascii via
`char::escape_default`, which fixes both failure modes at once — the
header
always builds, and it always reads back.
- `HttpResponse::error_with_header()` sets the header and the JSON body
from a
single message, replacing the hand-built tuples.
Most sites were formatting the identical message twice (once for the
header,
once for the body), so this removes that duplication as well.
`enrichment_table` additionally had four
`HeaderValue::from_str(..).unwrap()`
calls — outright panics, not 500s, on the same class of input. Those are
gone.
Other `HeaderValue` construction sites (router `user_email`,
`x-api-node`)
already guard with `unwrap_or_else` and are untouched.
## Organization identifier validation
Investigating how such an org existed at all: `create_org` validates the
display name and assigns a uuid identifier, but
`core::organization::check_and_create_org` takes the identifier
**verbatim from
the URL path** with no validation. It is reachable from any
root-authenticated
ingestion write (`ZO_CREATE_ORG_THROUGH_INGESTION`, default true), which
is how
the fuzzer provisioned it.
`validate_auto_created_org_id` (ascii alphanumeric plus `-` and `_`) now
guards
both auto-create entry points. It runs **after** the existing-org
lookup, so it
only gates new creation: no deployment loses an org it already has, and
SSO
auto-provisioning from IdP group names keeps working for normal
identifiers.
## Verification
Against a local server with the CI payload:
- stream create → `400 {"code":400,"message":"invalid data type:
v^0K"}`
instead of the 500
- ingestion to the exotic org → 404; `autoorg-1` still auto-creates and
ingests
- valid create, duplicate create, settings update, settings on a missing
stream,
delete, enrichment upload → correct codes, no panics
Two edits sit under `#[cfg(feature = "cloud")]` and cannot be compiled
here:
swapping in `Cargo.toml.openobserve` fails inside o2-enterprise itself,
whose
`main` has not yet picked up the `get_orm_client_rw` split from #13927.
They
were verified by temporarily un-gating them and stubbing the cloud-only
predicate.