8000 fix(api): refuse a regex include/exclude on a non-string terms field by amartya1523 · Pull Request #3008 · xerj-org/xerj · GitHub
[go: up one dir, main page]
Skip to content

fix(api): refuse a regex include/exclude on a non-string terms field - #3008

Open
amartya1523 wants to merge 2 commits into
xerj-org:mainfrom
amartya1523:fix/terms-regex-include-ip
Open

fix(api): refuse a regex include/exclude on a non-string terms field#3008
amartya1523 wants to merge 2 commits into
xerj-org:mainfrom
amartya1523:fix/terms-regex-include-ip

Conversation

@amartya1523
@amartya1523 amartya1523 commented Nov 6, 2026
Copy link
Copy Markdown
Contributor

AI-assisted change, run and reviewed by @amartya1523.

What this changes, and why

terms, rare_terms and significant_terms accept include / exclude in three forms:

  • an array of exact values,
  • a {partition, num_partitions} object,
  • a string, which is a regular expression.

ES only applies a regular expression to terms that are strings. On a field whose values aren't (IncludeExclude#isRegexBased with a non-RAW values-source format: ip, numbers, dates, booleans), it refuses the request. That's a 400 since 8.12:

Aggregation [ip_terms] cannot support regular expression style include/exclude settings as they can only be applied to string fields. Use an array of values for include/exclude clauses

XERJ answered 200 in all three aggregations:

  • terms matched the pattern against the rendered values, so "exclude": "127.*" on an ip field happened to drop 127.0.0.1.
  • rare_terms and significant_terms only read arrays and ignored the string, so the filter silently did nothing.

Either way the caller got buckets that didn't mean what the request said. Three catch-lenient.txt lines are this case, the last step of each "IP test":

  • aggregations/terms.yml · IP test
  • aggregations/rare_terms.yml · IP test
  • aggregations/sig_terms.yml · IP test

Fix (es_compat.rs): new find_regex_include_exclude_on_non_string_field, chained after find_unsupported_metric_agg_field in search_impl, at the same point (after the field-alias rewrite, before execution) and with the same rules:

  • Aggregations: terms, rare_terms and significant_terms, at any depth. The error names the aggregation that carries the pattern, as ES does. significant_text is left alone because it reads text, and its YAML cases use string patterns on text fields.
  • Regex style: include or exclude is a JSON string. Arrays and partition objects are allowed on every type, as in ES.
  • Non-string types: ip; long, integer, short, byte, double, float, half_float, scaled_float, unsigned_long; date, date_nanos; boolean. Multi-fields resolve through fields (name.raw is a keyword and stays allowed).
  • Every target index: the field is refused only when it has such a type in every target index. With mixed mappings ES fails some shards and answers a partial result, which this path doesn't model, so it keeps today's answer. An unmapped field is never refused.
  • Envelope: the existing search_shard_validation_error: 400 search_phase_execution_exception with an illegal_argument_exception root cause.

Evidence

New test module regex_include_exclude_tests (es_compat.rs), run against main first:

# main 34bf35b05
test es_compat::regex_include_exclude_tests::other_include_exclude_forms_still_answer ... ok
test es_compat::regex_include_exclude_tests::regex_include_exclude_on_non_string_fields_is_a_400 ... FAILED
  terms {"field":"ip","exclude":"127.*"} -> {"took":0, ..., "aggregations":{"ip_terms": ...   (200)

# this branch
test es_compat::regex_include_exclude_tests::regex_include_exclude_on_non_string_fields_is_a_400 ... ok
test es_compat::regex_include_exclude_tests::other_include_exclude_forms_still_answer ... ok
  • Refused (asserts the 400, the outer type, the root-cause type and ES's exact reason):
    • terms, rare_terms and significant_terms with "exclude": "127.*" on an ip
    • "include": "1.*" on an integer
    • "exclude": "2020.*" on a date
    • a pattern on an ip inside a sub-aggregation, which is reported as Aggregation [inner] ...
  • Still answers:
    • array include/exclude on an ip (terms, rare_terms)
    • a partition object on an integer
    • a regex on a keyword and on a keyword multi-field
    • a regex on an unmapped field
    • /t,u where u maps the field as a long
    • significant_text with a string include

Checks

  • cargo fmt -p xerj-api -- --check
  • cargo test --profile quick -p xerj-api --lib: 432 passed, 0 failed
  • cargo clippy --profile quick -p xerj-api --lib --tests: no warnings in the changed code (the local toolchain is newer than CI's 1.97.1 and flags unrelated existing code elsewhere)
  • YAML: the three existing "IP test" steps now enforce it; their catch-lenient lines are deleted. Across the whole suite, the only other string include/exclude values are significant_text / significant_texts on text fields, which this doesn't touch. The partition cases in terms.yml (keyword and integer) are objects, which stay allowed.
  • Docs: none describe the old behaviour.

Not run locally:

  • The ES-YAML suite against a live node, --release builds and the integration binaries. CI runs these.

Provenance

  • Written by: AI-assisted, run and reviewed by @amartya1523
  • Verified: the test outputs above, on macOS 26 (aarch64), quick profile, based on main 34bf35b05.
  • Assumed:
    • The message text is pinned by the rare_terms and sig_terms YAML regexes; terms.yml only asserts bad_request.
    • Before 8.12 ES raised this as a 500; the 400 here follows 8.12+.
    • version fields aren't in the refused list (I'm not sure of their values-source format in ES), so they keep today's answer.
    • _msearch and the template paths don't go through search_impl, so they aren't covered.

@amartya1523
fix(api): refuse a regex include/exclude on a non-string terms field
a68c63c 
terms, rare_terms and significant_terms take include/exclude as an
array of values, a partition object, or a string, which is a regular
expression. ES applies a regular expression to the terms as strings, so
on an ip, numeric, date or boolean field it answers 400 with
"Aggregation [name] cannot support regular expression style
include/exclude settings as they can only be applied to string fields.
Use an array of values for include/exclude clauses". XERJ ran the
pattern anyway: terms matched it against the rendered values and
rare_terms / significant_terms ignored it.

The check sits beside the non-numeric metric check in search_impl and
follows its rule: the field is refused only when it has such a type in
every target index.

Retires the three "IP test" catch-lenient entries (terms, rare_terms,
sig_terms).
@amartya1523
test(api): pin the regex include/exclude refusal
d2a7484 
terms, rare_terms and significant_terms on ip, a regex on an integer and a date, and a sub-aggregation are a 400 with ES's reason; arrays, partitions, string fields, multi-fields, unmapped fields, mixed targets and significant_text still answer.
@cla-bot cla-bot Bot added the cla-signed label Nov 6, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@amartya1523
0