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

Fix class cast exception in BucketMetricsPipeline path traversal #30632

Merged

Conversation

polyfractal
Copy link
Contributor

When processing a top-level sibling pipeline, we destructively sublist the path by assigning back onto the same variable. But if aggs are specified such:

A. Multi-bucket agg in the first entry of our internal list
B. Regular agg as the immediate child of the multi-bucket in A
C. Regular agg with the same name as B at the top level, listed as the second entry in our internal list
D. Finally, a pipeline agg with the path down to B

We'll get class cast exception. The first agg will sublist the path from [A,B] to [B], and then when we loop around to check agg C, the sublisted path [B] matches the name of C and it fails.

The fix is simple: we just need to store the sublist in a new object so that the old path remains valid for the rest of the aggs in the loop

Closes #30608

/cc @colings86

When processing a top-level sibling pipeline, we destructively sublist
the path by assigning back onto the same variable.  But if aggs are
specified such:

A. Multi-bucket agg in the first entry of our internal list
B. Regular agg as the immediate child of the multi-bucket in A
C. Regular agg with the same name as B at the top level, listed as the
   second entry in our internal list
D. Finally, a pipeline agg with the path down to B

We'll get class cast exception.  The first agg will sublist the path
from [A,B] to [B], and then when we loop around to check agg C,
the sublisted path [B] matches the name of C and it fails.

The fix is simple: we just need to store the sublist in a new object
so that the old path remains valid for the rest of the aggs in the loop

Closes elastic#30608
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

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

LGTM

@polyfractal polyfractal merged commit cd1ed90 into elastic:master May 16, 2018
polyfractal added a commit that referenced this pull request May 16, 2018
When processing a top-level sibling pipeline, we destructively sublist
the path by assigning back onto the same variable.  But if aggs are
specified such:

A. Multi-bucket agg in the first entry of our internal list
B. Regular agg as the immediate child of the multi-bucket in A
C. Regular agg with the same name as B at the top level, listed as the
   second entry in our internal list
D. Finally, a pipeline agg with the path down to B

We'll get class cast exception.  The first agg will sublist the path
from [A,B] to [B], and then when we loop around to check agg C,
the sublisted path [B] matches the name of C and it fails.

The fix is simple: we just need to store the sublist in a new object
so that the old path remains valid for the rest of the aggs in the loop

Closes #30608
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 16, 2018
…ngs-to-true

* elastic/master: (34 commits)
  Test: increase search logging for LicensingTests
  Adjust serialization version in IndicesOptions
  [TEST] Fix compilation
  Remove version argument in RangeFieldType (elastic#30411)
  Remove unused DirectoryUtils class. (elastic#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534)
  Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594)
  Removes AwaitsFix on IndicesOptionsTests
  Template upgrades should happen in a system context (elastic#30621)
  Fix bug in BucketMetrics path traversal (elastic#30632)
  Fixes IndiceOptionsTests to serialise correctly (elastic#30644)
  S3 repo plugin populate SettingsFilter (elastic#30652)
  mute IndicesOptionsTests.testSerialization
  Rest High Level client: Add List Tasks (elastic#29546)
  Refactors ClientHelper to combine header logic (elastic#30620)
  [ML] Wait for ML indices in rolling upgrade tests (elastic#30615)
  Watcher: Ensure secrets integration tests also run triggered watch (elastic#30478)
  Move allocation awareness attributes to list setting (elastic#30626)
  [Docs] Update code snippet in has-child-query.asciidoc (elastic#30510)
  Replace custom reloadable Key/TrustManager (elastic#30509)
  ...
martijnvg added a commit that referenced this pull request May 17, 2018
* es/master: (74 commits)
  Preserve REST client auth despite 401 response (#30558)
  [test] packaging: add windows boxes (#30402)
  Make xpack modules instead of a meta plugin (#30589)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (#30646)
  Reindex: Fixed typo in assertion failure message (#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  [DOCS] Reorganizes RBAC documentation
  SQL: Remove dependency for server's version from JDBC driver (#30631)
  Test: increase search logging for LicensingTests
  Adjust serialization version in IndicesOptions
  [TEST] Fix compilation
  Remove version argument in RangeFieldType (#30411)
  Remove unused DirectoryUtils class. (#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (#30534)
  Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (#29594)
  Removes AwaitsFix on IndicesOptionsTests
  Template upgrades should happen in a system context (#30621)
  Fix bug in BucketMetrics path traversal (#30632)
  Fixes IndiceOptionsTests to serialise correctly (#30644)
  ...
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request May 17, 2018
* es/ccr: (75 commits)
  Preserve REST client auth despite 401 response (elastic#30558)
  [test] packaging: add windows boxes (elastic#30402)
  Make xpack modules instead of a meta plugin (elastic#30589)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (elastic#30646)
  Reindex: Fixed typo in assertion failure message (elastic#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  [DOCS] Reorganizes RBAC documentation
  SQL: Remove dependency for server's version from JDBC driver (elastic#30631)
  Test: increase search logging for LicensingTests
  Adjust serialization version in IndicesOptions
  [TEST] Fix compilation
  Remove version argument in RangeFieldType (elastic#30411)
  Remove unused DirectoryUtils class. (elastic#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534)
  Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594)
  Removes AwaitsFix on IndicesOptionsTests
  Template upgrades should happen in a system context (elastic#30621)
  Fix bug in BucketMetrics path traversal (elastic#30632)
  Fixes IndiceOptionsTests to serialise correctly (elastic#30644)
  ...
ywelsch pushed a commit to ywelsch/elasticsearch that referenced this pull request May 23, 2018
When processing a top-level sibling pipeline, we destructively sublist
the path by assigning back onto the same variable.  But if aggs are
specified such:

A. Multi-bucket agg in the first entry of our internal list
B. Regular agg as the immediate child of the multi-bucket in A
C. Regular agg with the same name as B at the top level, listed as the
   second entry in our internal list
D. Finally, a pipeline agg with the path down to B

We'll get class cast exception.  The first agg will sublist the path
from [A,B] to [B], and then when we loop around to check agg C,
the sublisted path [B] matches the name of C and it fails.

The fix is simple: we just need to store the sublist in a new object
so that the old path remains valid for the rest of the aggs in the loop

Closes elastic#30608
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bucket path name resolution fails with siblings and child aggregations with the same name
3 participants