-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Avoid infinite loop in flat_object parsing #15985
Avoid infinite loop in flat_object parsing #15985
Conversation
1e1e7fe
to
73e7e84
Compare
@kkewwei -- would you mind taking a look at this change? |
server/src/main/java/org/opensearch/common/xcontent/JsonToStringXContentParser.java
Show resolved
Hide resolved
❌ Gradle check result for 73e7e84: 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? |
We had logic in flat_object parsing that would: 1. Try parsing a flat object field that is not an object or null. 2. Would see an END_ARRAY token, ignore it, and not advance the parser. Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token. Signed-off-by: Michael Froh <froh@amazon.com>
The removed code does not actually seem to affect the logic. Also, I want to be 100% sure that every call to parseToken is guaranteed to call parser.nextToken() at some point. Signed-off-by: Michael Froh <froh@amazon.com>
Thanks for the reminder, @kkewwei! Signed-off-by: Michael Froh <froh@amazon.com>
4ee50a1
to
dff498d
Compare
❌ Gradle check result for 4ee50a1: 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? |
❌ Gradle check result for dff498d: 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? |
The test fails on MixedClusterClientYamlTestSuiteIT because 2.x still has the infinite loop until backport. Signed-off-by: Michael Froh <froh@amazon.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15985 +/- ##
============================================
- Coverage 71.90% 71.90% -0.01%
+ Complexity 64392 64290 -102
============================================
Files 5278 5280 +2
Lines 300877 300864 -13
Branches 43478 43473 -5
============================================
- Hits 216351 216337 -14
+ Misses 66747 66736 -11
- Partials 17779 17791 +12 ☔ View full report in Codecov by Sentry. |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-15985-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 05dab3b7eb54a361af3583a322f0a748d6412836
# Push it to GitHub
git push --set-upstream origin backport/backport-15985-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x Then, create a pull request where the |
* Avoid infinite loop in flat_object parsing We had logic in flat_object parsing that would: 1. Try parsing a flat object field that is not an object or null. 2. Would see an END_ARRAY token, ignore it, and not advance the parser. Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token. Signed-off-by: Michael Froh <froh@amazon.com> * Remove some unused code and add more tests The removed code does not actually seem to affect the logic. Also, I want to be 100% sure that every call to parseToken is guaranteed to call parser.nextToken() at some point. Signed-off-by: Michael Froh <froh@amazon.com> * Remove unused parameter from parseToken Thanks for the reminder, @kkewwei! Signed-off-by: Michael Froh <froh@amazon.com> * Add skip for newly-added test The test fails on MixedClusterClientYamlTestSuiteIT because 2.x still has the infinite loop until backport. Signed-off-by: Michael Froh <froh@amazon.com> --------- Signed-off-by: Michael Froh <froh@amazon.com> (cherry picked from commit 05dab3b)
Manual backport PR: #16026 |
* Avoid infinite loop in flat_object parsing We had logic in flat_object parsing that would: 1. Try parsing a flat object field that is not an object or null. 2. Would see an END_ARRAY token, ignore it, and not advance the parser. Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token. Signed-off-by: Michael Froh <froh@amazon.com> * Remove some unused code and add more tests The removed code does not actually seem to affect the logic. Also, I want to be 100% sure that every call to parseToken is guaranteed to call parser.nextToken() at some point. Signed-off-by: Michael Froh <froh@amazon.com> * Remove unused parameter from parseToken Thanks for the reminder, @kkewwei! Signed-off-by: Michael Froh <froh@amazon.com> * Add skip for newly-added test The test fails on MixedClusterClientYamlTestSuiteIT because 2.x still has the infinite loop until backport. Signed-off-by: Michael Froh <froh@amazon.com> --------- Signed-off-by: Michael Froh <froh@amazon.com> (cherry picked from commit 05dab3b)
* Avoid infinite loop in flat_object parsing We had logic in flat_object parsing that would: 1. Try parsing a flat object field that is not an object or null. 2. Would see an END_ARRAY token, ignore it, and not advance the parser. Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token. Signed-off-by: Michael Froh <froh@amazon.com> * Remove some unused code and add more tests The removed code does not actually seem to affect the logic. Also, I want to be 100% sure that every call to parseToken is guaranteed to call parser.nextToken() at some point. Signed-off-by: Michael Froh <froh@amazon.com> * Remove unused parameter from parseToken Thanks for the reminder, @kkewwei! Signed-off-by: Michael Froh <froh@amazon.com> * Add skip for newly-added test The test fails on MixedClusterClientYamlTestSuiteIT because 2.x still has the infinite loop until backport. Signed-off-by: Michael Froh <froh@amazon.com> --------- Signed-off-by: Michael Froh <froh@amazon.com>
* Avoid infinite loop in flat_object parsing We had logic in flat_object parsing that would: 1. Try parsing a flat object field that is not an object or null. 2. Would see an END_ARRAY token, ignore it, and not advance the parser. Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token. Signed-off-by: Michael Froh <froh@amazon.com> * Remove some unused code and add more tests The removed code does not actually seem to affect the logic. Also, I want to be 100% sure that every call to parseToken is guaranteed to call parser.nextToken() at some point. Signed-off-by: Michael Froh <froh@amazon.com> * Remove unused parameter from parseToken Thanks for the reminder, @kkewwei! Signed-off-by: Michael Froh <froh@amazon.com> * Add skip for newly-added test The test fails on MixedClusterClientYamlTestSuiteIT because 2.x still has the infinite loop until backport. Signed-off-by: Michael Froh <froh@amazon.com> --------- Signed-off-by: Michael Froh <froh@amazon.com>
* Avoid infinite loop in flat_object parsing We had logic in flat_object parsing that would: 1. Try parsing a flat object field that is not an object or null. 2. Would see an END_ARRAY token, ignore it, and not advance the parser. Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token. Signed-off-by: Michael Froh <froh@amazon.com> * Remove some unused code and add more tests The removed code does not actually seem to affect the logic. Also, I want to be 100% sure that every call to parseToken is guaranteed to call parser.nextToken() at some point. Signed-off-by: Michael Froh <froh@amazon.com> * Remove unused parameter from parseToken Thanks for the reminder, @kkewwei! Signed-off-by: Michael Froh <froh@amazon.com> * Add skip for newly-added test The test fails on MixedClusterClientYamlTestSuiteIT because 2.x still has the infinite loop until backport. Signed-off-by: Michael Froh <froh@amazon.com> --------- Signed-off-by: Michael Froh <froh@amazon.com>
* Avoid infinite loop in flat_object parsing We had logic in flat_object parsing that would: 1. Try parsing a flat object field that is not an object or null. 2. Would see an END_ARRAY token, ignore it, and not advance the parser. Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token. Signed-off-by: Michael Froh <froh@amazon.com> * Remove some unused code and add more tests The removed code does not actually seem to affect the logic. Also, I want to be 100% sure that every call to parseToken is guaranteed to call parser.nextToken() at some point. Signed-off-by: Michael Froh <froh@amazon.com> * Remove unused parameter from parseToken Thanks for the reminder, @kkewwei! Signed-off-by: Michael Froh <froh@amazon.com> * Add skip for newly-added test The test fails on MixedClusterClientYamlTestSuiteIT because 2.x still has the infinite loop until backport. Signed-off-by: Michael Froh <froh@amazon.com> --------- Signed-off-by: Michael Froh <froh@amazon.com>
* Avoid infinite loop in flat_object parsing We had logic in flat_object parsing that would: 1. Try parsing a flat object field that is not an object or null. 2. Would see an END_ARRAY token, ignore it, and not advance the parser. Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token. Signed-off-by: Michael Froh <froh@amazon.com> * Remove some unused code and add more tests The removed code does not actually seem to affect the logic. Also, I want to be 100% sure that every call to parseToken is guaranteed to call parser.nextToken() at some point. Signed-off-by: Michael Froh <froh@amazon.com> * Remove unused parameter from parseToken Thanks for the reminder, @kkewwei! Signed-off-by: Michael Froh <froh@amazon.com> * Add skip for newly-added test The test fails on MixedClusterClientYamlTestSuiteIT because 2.x still has the infinite loop until backport. Signed-off-by: Michael Froh <froh@amazon.com> --------- Signed-off-by: Michael Froh <froh@amazon.com>
Description
We had logic in flat_object parsing that would:
Combined, this would create a scenario where passing an array of strings for a flat_object would parse the string values, then loop infinitely on the END_ARRAY token.
Related Issues
Resolves #15982
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.