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.

Sign up for GitHub

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

Jump to bottom

[Backport 1.3] Bugfix to guard against stack overflow errors caused by very large reg-ex input #16101

Merged
merged 3 commits into from
Oct 10, 2024
Merged

[Backport 1.3] Bugfix to guard against stack overflow errors caused by very large reg-ex input #16101

merged 3 commits into from
Oct 10, 2024

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

@opensearch-trigger-bot opensearch-trigger-bot bot commented Sep 26, 2024 •
edited by dbwiddis
Loading

Backport 566ebfa from #2810.

This was backported to 1.x in #8663 but never made it to 1.3. Encountered in the wild today, 58 nodes dropped at once, whee!

@github-actions
Bugfix to guard against stack overflow errors caused by very large re…
fabde0c 
…g-ex input (#2810)

* Bugfix to guard against stack overflow errors caused by very large reg-ex input

This change fixes a code path that did not properly impose the index-level max_regex_length limit. Therefore, it was possibly to provide ar arbitrarily large string as the include/exclude reg-ex value under search aggregations. This exposed the underlying node to crashes from a StackOverflowError, due to how the Lucene RegExp class processes strings using stack frames.

Signed-off-by: Kartik Ganesh <[email protected]>

* Adding integration tests for large string RegEx

Signed-off-by: Kartik Ganesh <[email protected]>

* Spotless

Signed-off-by: Kartik Ganesh <[email protected]>
(cherry picked from commit 566ebfa)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions GitHub Actions
Copy link
Contributor

❌ Gradle check result for fabde0c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions GitHub Actions
Copy link
Contributor

❌ Gradle check result for 653d12f: ABORTED

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions GitHub Actions
Copy link
Contributor

✅ Gradle check result for f86d44e: SUCCESS

@dbwiddis
Change log
7eca19d 
Signed-off-by: Daniel Widdis <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

❕ Gradle check result for 7eca19d: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.index.ShardIndexingPressureSettingsIT.testShardIndexingPressureEnforcedEnabledDisabledSetting

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@opensearch-ci-bot opensearch-ci-bot mentioned this pull request Sep 27, 2024
Open
@dblock
Copy link
Member

dblock commented Sep 27, 2024

Gradle assemble is failing, do we need to backport something else to this branch @dbwiddis ?

@dbwiddis
Copy link
Member

dbwiddis commented Sep 28, 2024 •
edited
Loading

Gradle assemble is failing, do we need to backport something else to this branch @dbwiddis ?

It's only failing on macOS. Apparently we don't have docker there (at least on 1.3?)

image

Interestingly, it showed able to merge before I added the changelog commit.

@dbwiddis
Copy link
Member

dbwiddis commented Sep 28, 2024 •
edited
Loading

Ah, I see, the GHA doesn't like the changed name to macos-13:

        if: runner.os == 'macos'

EDIT: oops. that was matrix.os. runner.os is apparently macOS and is case sensitivve.

@github-actions GitHub Actions
Copy link
Contributor

✅ Gradle check result for d6a157c: SUCCESS

@dbwiddis
Copy link
Member

Well, that didn't fix it. The action on main and 2.x looks completely different, 2.x even allows fail on error and says docker is unstable.

@dbwiddis dbwiddis mentioned this pull request Oct 2, 2024
Closed
@dbwiddis dbwiddis force-pushed the backport/backport-2810-to-1.3 branch from d6a157c to 7eca19d Compare October 10, 2024 19:23
@github-actions GitHub Actions
Copy link
Contributor

✅ Gradle check result for 7eca19d: SUCCESS

@dbwiddis
Copy link
Member

Thanks @reta for fixing the assemble workflow in #16222. I was just coming back to investigate that.

As @dblock has previously approved and all tests now pass, merging.

@dbwiddis dbwiddis merged commit 2428278 into 1.3 Oct 10, 2024
29 of 30 checks passed
@dbwiddis dbwiddis deleted the backport/backport-2810-to-1.3 branch October 10, 2024 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbwiddis dbwiddis dbwiddis approved these changes

@dblock dblock dblock approved these changes

@anasalkouz anasalkouz Awaiting requested review from anasalkouz anasalkouz is a code owner

@andrross andrross Awaiting requested review from andrross andrross is a code owner

@ashking94 ashking94 Awaiting requested review from ashking94 ashking94 is a code owner

@Bukhtawar Bukhtawar Awaiting requested review from Bukhtawar Bukhtawar is a code owner

@CEHENKLE CEHENKLE Awaiting requested review from CEHENKLE CEHENKLE is a code owner

@gbbafna gbbafna Awaiting requested review from gbbafna gbbafna is a code owner

@jed326 jed326 Awaiting requested review from jed326 jed326 is a code owner

@kotwanikunal kotwanikunal Awaiting requested review from kotwanikunal kotwanikunal is a code owner

@mch2 mch2 Awaiting requested review from mch2 mch2 is a code owner

@msfroh msfroh Awaiting requested review from msfroh msfroh is a code owner

@nknize nknize Awaiting requested review from nknize nknize is a code owner

@owaiskazi19 owaiskazi19 Awaiting requested review from owaiskazi19 owaiskazi19 is a code owner

@reta reta Awaiting requested review from reta reta is a code owner

@Rishikesh1159 Rishikesh1159 Awaiting requested review from Rishikesh1159 Rishikesh1159 is a code owner

@sachinpkale sachinpkale Awaiting requested review from sachinpkale sachinpkale is a code owner

@saratvemulapalli saratvemulapalli Awaiting requested review from saratvemulapalli saratvemulapalli is a code owner

@shwetathareja shwetathareja Awaiting requested review from shwetathareja shwetathareja is a code owner

@sohami sohami Awaiting requested review from sohami sohami is a code owner

@VachaShah VachaShah Awaiting requested review from VachaShah VachaShah is a code owner

@peternied peternied Awaiting requested review from peternied

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dblock @dbwiddis