Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search: Validate script query is run with a single script #29304

Merged
merged 1 commit into from
Mar 30, 2018

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Mar 29, 2018

The parsing code for script query currently silently skips by any tokens
it does not know about within its parsing loop. The only token it does
not catch is an array, which means passing multiple scripts in via an
array will cause the last script to be parsed, silently dropping
the others. This commit adds validation that arrays are not seen while
parsing.

The parsing code for script query currently silently skips by any tokens
it does not know about within its parsing loop. The only token it does
not catch is an array, which means pasing multiple scripts in via an
array will cause the last script to be parsed and one, silently dropping
the others. This commit adds validation that arrays are not seen while
parsing.
@rjernst rjernst added >bug :Search/Search Search-related issues that do not fall into other categories v7.0.0 v6.3.0 labels Mar 29, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@rjernst
Copy link
Member Author

rjernst commented Mar 29, 2018

@mayya-sharipova mayya-sharipova self-requested a review March 29, 2018 23:34
Copy link
Contributor

@mayya-sharipova mayya-sharipova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM, tested with another script, works well

@rjernst rjernst merged commit 54f8f81 into elastic:master Mar 30, 2018
@rjernst rjernst deleted the script_query_parsing branch March 30, 2018 05:10
rjernst added a commit that referenced this pull request Mar 30, 2018
The parsing code for script query currently silently skips by any tokens
it does not know about within its parsing loop. The only token it does
not catch is an array, which means pasing multiple scripts in via an
array will cause the last script to be parsed and one, silently dropping
the others. This commit adds validation that arrays are not seen while
parsing.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 3, 2018
* master: (80 commits)
  Remove HTTP max content length leniency (elastic#29337)
  Begin moving XContent to a separate lib/artifact (elastic#29300)
  Java versions for ci (elastic#29320)
  Minor cleanup in the InternalEngine (elastic#29241)
  Clarify expectations of false positives/negatives (elastic#27964)
  Update docs on vertex ordering (elastic#27963)
  Revert "REST high-level client: add support for Indices Update Settings API (elastic#28892)" (elastic#29323)
  [test] remove Streamable serde assertions (elastic#29307)
  Improve query string docs (elastic#28882)
  fix query string example for boolean query (elastic#28881)
  Resolve unchecked cast warnings introduced with elastic#28892
  REST high-level client: add support for Indices Update Settings API (elastic#28892)
  Search: Validate script query is run with a single script (elastic#29304)
  [DOCS] Added info on WGS-84. Closes issue elastic#3590 (elastic#29305)
  Increase timeout on Netty client latch for tests
  Build: Use branch specific refspec sysprop for bwc builds (elastic#29299)
  TEST: trim unsafe commits before opening engine
  Move trimming unsafe commits from engine ctor to store (elastic#29260)
  Fix incorrect geohash for lat 90, lon 180 (elastic#29256)
  Do not load global state when deleting a snapshot (elastic#29278)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v6.3.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants