-
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
Adding depth check in doc parser for deep nested document #5199
Adding depth check in doc parser for deep nested document #5199
Conversation
Gradle Check (Jenkins) Run Completed with:
|
c2b1c23
to
da33086
Compare
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
da33086
to
174883a
Compare
Gradle Check (Jenkins) Run Completed with:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add to CHANGELOG. We'll also need to document this.
How does this alter the default behavior? If I understand correctly now the mapping depth limit is now enforced - what is its default value?
server/src/main/java/org/opensearch/index/mapper/DocumentParser.java
Outdated
Show resolved
Hide resolved
174883a
to
ae7253f
Compare
Gradle Check (Jenkins) Run Completed with:
|
server/src/main/java/org/opensearch/index/mapper/DocumentParser.java
Outdated
Show resolved
Hide resolved
ae7253f
to
f9b61d7
Compare
server/src/main/java/org/opensearch/index/mapper/DocumentParser.java
Outdated
Show resolved
Hide resolved
Gradle Check (Jenkins) Run Completed with:
|
Codecov Report
@@ Coverage Diff @@
## main #5199 +/- ##
============================================
+ Coverage 70.98% 71.01% +0.02%
+ Complexity 58669 58655 -14
============================================
Files 4763 4764 +1
Lines 279945 280010 +65
Branches 40418 40425 +7
============================================
+ Hits 198731 198843 +112
+ Misses 64988 64858 -130
- Partials 16226 16309 +83
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will leave this to @reta since he had some concerns.
f9b61d7
to
997e763
Compare
Gradle Check (Jenkins) Run Completed with:
|
997e763
to
2c6c8b9
Compare
Gradle Check (Jenkins) Run Completed with:
|
@nkumar04 could you rebase please? |
2c6c8b9
to
d45b0eb
Compare
d45b0eb
to
ee3d1ec
Compare
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
done. |
Looks like this needs another rebase, sorry. I am traveling, will be slow to reply. Hope other maintainers can help merge. |
Fixing the issue (opensearch-project#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com>
Fixing the issue (opensearch-project#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com>
Fixing the issue (opensearch-project#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com>
Fixing the issue (opensearch-project#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com>
ee3d1ec
to
c7b3897
Compare
Gradle Check (Jenkins) Run Completed with:
|
I'm assuming this is safe to backport, as it is a fix to a stack overflow label. I'm adding the backport label now but we'll also have to fix up the changelog entry and move it to the |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-5199-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 950b86ae697d95db537e7809f2675ebdf2b0dc10
# Push it to GitHub
git push --set-upstream origin backport/backport-5199-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x Then, create a pull request where the |
…-project#5199) * Adding depth check in doc parser for deep nested document Fixing the issue (opensearch-project#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com> (cherry picked from commit 950b86a)
…-project#5199) * Adding depth check in doc parser for deep nested document Fixing the issue (opensearch-project#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com> (cherry picked from commit 950b86a)
…-project#5199) * Adding depth check in doc parser for deep nested document Fixing the issue (opensearch-project#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com> (cherry picked from commit 950b86a)
…-project#5199) * Adding depth check in doc parser for deep nested document Fixing the issue (opensearch-project#5195) Signed-off-by: Nikhil Kumar <nikhkumn@amazon.com> Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Description
Added a depth level check within the document parser context. The depth level is incremented for each nested object parsing and decremented when parsing is done. If the depth level crosses a threshold, document parser throws a parsing exception and an error is returned in the response, stating the reason of the failure
Issues Resolved
Closes issue (#5195)
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.