Repository navigation
fix(api): validate index-creation, mapping, alias and template requests like ES - #1870
Merged
Merged
Conversation
…ts like ES Motivation: 17 `catch-lenient.txt` lines (xerj-org#1583) are index-admin requests that Elasticsearch 8.13 refuses with a 400 and XERJ accepted with a 200, leaving the bad state behind: a create-index body whose `mappings`, `properties`, `settings` or `aliases` is not an object, an empty field name, `soft_deletes.enabled: false`, a `lookup` index with more than one shard, a negative `ignore_above` (mapping or `index.mapping` setting), `bbq_*` vector options on byte/bit vectors or with `index: false`, alias values of the wrong type, an alias named like an index or with forbidden characters, unknown alias body fields, a legacy template without `index_patterns`, and `GET /_foo`. Root cause: `create_index` validated only the index name and field types; `put_alias`, `put_legacy_template` and `get_index_inner` validated nothing of this. Fix (es_compat.rs), all before anything is created or attached: - `create_index_body_refusal`: section shapes, soft_deletes, lookup shards, `index.mapping.ignore_above`, alias value types; settings read in every spelling ES accepts (`create_setting`). - `empty_field_name_refusal` (create_index and put_mapping): `mapper_parsing_exception` caused by `illegal_argument_exception`. - `validate_properties`: negative `ignore_above` on keyword / flattened / wildcard; dense_vector `index_options` with `index: false`, and `bbq_*` on byte/bit (bfloat16 stays allowed). - `put_alias`: forbidden characters, a name that is a concrete index (an existing alias of that name can still be re-put), unknown body fields, non-scalar routing, wrong value types. - `put_legacy_template`: `index patterns are missing`. - `get_index_inner`: a `_`-prefixed name other than `_all` is `invalid_index_name_exception` (a leading `-` is an exclusion, left alone). - `es_bad_request`: a 400 in ES's error shape for refusals whose type or `caused_by` no `XerjError` variant renders. Evidence: - Full ES-YAML suite against a node built from this commit: 0 failed, and exactly these 17 catch-lenient entries reported stale; with them deleted and enforced: `1402 passed · 0 failed · 10 skipped`, catch-lenient 151 -> 134.
|
Thanks for this contribution — we'd like to merge it. Before we can, we need a signed Contributor License Agreement from: @amartya1523 It is a one-time thing per contributor, not per pull request. Read CLA.md, then open a small pull request adding your GitHub username to Once it is merged, comment |
Owner
|
@cla-bot check |
|
Re-checking the CLA status for every commit author on this pull request… |
Owner
|
Xerj Merger made the fix: index-admin requests that Elasticsearch 8.13 refuses are now refused by XERJ too, before anything is created: malformed create-index bodies, empty field names, soft_deletes disabled, lookup-mode indices with more than one shard, negative ignore_above, invalid bbq dense_vector options, bad alias bodies and alias names that collide with an index, legacy templates without index_patterns, and GET /_foo. That burns 17 lines off catch-lenient.txt (151 to 134). Squash-merged on head 6d1e1c6 after Build + Test passed. Thanks @amartya1523! |
This was referenced Oct 20, 2026
amartya1523
added a commit
to amartya1523/xerj
that referenced
this pull request
Oct 20, 2026
Motivation: 20 `catch-lenient.txt` lines (xerj-org#1583) are aggregation requests that Elasticsearch 8.13 refuses with a 400 at parse time and XERJ answered 200, ignoring the bad value or returning an empty or defaulted result: a negative cardinality `precision_threshold` or unknown `execution_hint`, `filters` with no filters, a non-object `meta`, an `ip_prefix` `prefix_length` out of range, a non-positive `median_absolute_deviation` `compression`, unknown tdigest execution hints, `percentile_ranks` without usable `values`, a `terms` with neither `field` nor `script`, `random_sampler` misuse, and `time_series` under `global` or next to an aggregation incompatible with time-series execution. Root cause: None of these parameters or placements were validated; the executors treat each malformed shape as "use a default" or "nothing to return". Fix: `find_invalid_agg_params` (es_compat.rs), a request-time walk chained after `find_invalid_agg_order_path` (xerj-org#1283) at its four call sites (`_search`, `_msearch`, `_search/template`, `_msearch/template`), with ES 8.13's messages. Only checks that need no mapping are made: e.g. `avg` on a keyword field (which needs the field type) is left alone. Evidence: - Full ES-YAML suite against a node built from this commit: 0 failed, and exactly 20 catch-lenient entries reported stale; with them deleted and enforced: `1404 passed · 0 failed · 10 skipped` on current main (after xerj-org#1854 and xerj-org#1870), catch-lenient 128 -> 108. - Unit tests pin the valid shapes the suite relies on (the five cardinality hints, `default`/`high_accuracy`, random_sampler 0.1 and 1.0, time_series with date_histogram, terms with only a script, an incompatible type in a request without time_series) and each message.
xerj-org
pushed a commit
that referenced
this pull request
Oct 20, 2026
#1861) Motivation: 20 `catch-lenient.txt` lines (#1583) are aggregation requests that Elasticsearch 8.13 refuses with a 400 at parse time and XERJ answered 200, ignoring the bad value or returning an empty or defaulted result: a negative cardinality `precision_threshold` or unknown `execution_hint`, `filters` with no filters, a non-object `meta`, an `ip_prefix` `prefix_length` out of range, a non-positive `median_absolute_deviation` `compression`, unknown tdigest execution hints, `percentile_ranks` without usable `values`, a `terms` with neither `field` nor `script`, `random_sampler` misuse, and `time_series` under `global` or next to an aggregation incompatible with time-series execution. Root cause: None of these parameters or placements were validated; the executors treat each malformed shape as "use a default" or "nothing to return". Fix: `find_invalid_agg_params` (es_compat.rs), a request-time walk chained after `find_invalid_agg_order_path` (#1283) at its four call sites (`_search`, `_msearch`, `_search/template`, `_msearch/template`), with ES 8.13's messages. Only checks that need no mapping are made: e.g. `avg` on a keyword field (which needs the field type) is left alone. Evidence: - Full ES-YAML suite against a node built from this commit: 0 failed, and exactly 20 catch-lenient entries reported stale; with them deleted and enforced: `1404 passed · 0 failed · 10 skipped` on current main (after #1854 and #1870), catch-lenient 128 -> 108. - Unit tests pin the valid shapes the suite relies on (the five cardinality hints, `default`/`high_accuracy`, random_sampler 0.1 and 1.0, time_series with date_histogram, terms with only a script, an incompatible type in a request without time_series) and each message.
3 of 7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI-assisted change, run and reviewed by @amartya1523.
Burns down 17 lines of the
catch-lenient.txtratchet from #1583 (151 → 134). These are index-admin requests that Elasticsearch 8.13 refuses with a 400 and XERJ accepted with a 200, leaving the bad state behind (an index with an empty-named field, an alias named like an index, …).Independent of my other catch-lenient PRs (#1824, #1854, #1859, #1861): different handlers. Only
catch-lenient.txtoverlaps, so the later merge needs a trivial rebase there.What this changes, and why
All checks run before anything is created or attached, so a refusal leaves nothing behind.
mappings/settings/aliasesis not an object, ormappings.propertiesis not a map""(create index, put mapping)mapper_parsing_exceptioncaused byillegal_argument_exception"field name cannot be an empty string"soft_deletes.enabled: false(any spelling)index.mode: lookupwithnumber_of_shards≠ 1ignore_above< 0 on keyword / flattened / wildcardindex.mapping.ignore_above< 0bbq_*withelement_typebyte/bit, orindex_optionswithindex: falseis_write_index: "true",routing: truePUT /i/_alias/test_*, or an alias named like an existing indexinvalid_alias_name_exceptionindex_patternsaction_request_validation_exception"Validation Failed: 1: index patterns are missing;"GET /_fooinvalid_index_name_exception(was 404)Care taken against false positives:
Engine::get_indexalso resolves aliases and is not used alone for this check.GET /<index>: only a leading_(other than_all) is refused; a leading-in a list is an exclusion.put_aliasbody and create-indexaliasesspec in the YAML corpus (filter,routing,is_write_index); all remain accepted.es_bad_requestbuilds ES-shaped 400s for refusals whosetypeorcaused_bynoXerjErrorvariant renders.Evidence
Full ES-YAML suite against a node built from this commit. The runner reported exactly these 17 entries stale, with 0 failed:
indices.create/10_basic.yml×5: invalid mappings, without soft deletes, lookup index with two shards, poorly formatted request, unsupported value typeindices.get/10_basic.yml×1indices.put_alias/10_basic.yml×3indices.put_mapping/10_basic.yml×1indices.put_template/10_basic.yml×1 (without index_patterns)search/550_ignore_above_invalid.yml×446_knn_search_bbq_ivf.yml×2 (both copies)With them deleted and enforced:
Checks
cargo fmt --all -- --check(exit 0)cargo build --release -p xerj-server -p es-yaml-runner[limits] disk_flood_stage_percent = 0because this machine's disk is above the 95% flood stage.Not run:
cargo test -p xerj-api(release/fat-LTO test binaries do not fit this machine's free disk; CI runs them) and clippy.Left in the list:
indices.put_template/10_basic.yml · Put template create. The runner does not forward thecreateparameter (tests/es-compat-yaml/src/main.rs), so it needs a runner change first.Provenance
bad_request) there, and I did not compare those strings against a live ES.