Skip to content

Fix JacksonEvent delete for fields with slashes in field names#5962

Open
jkbedi wants to merge 1 commit into
opensearch-project:mainfrom
jkbedi:ocsf-eks
Open

Fix JacksonEvent delete for fields with slashes in field names#5962
jkbedi wants to merge 1 commit into
opensearch-project:mainfrom
jkbedi:ocsf-eks

Conversation

@jkbedi

@jkbedi jkbedi commented Aug 5, 2025

Copy link
Copy Markdown

Description

This change fixes a bug in JacksonEvent.delete() where fields containing forward slashes in their names could not be properly deleted when using JSON Pointer escaped keys.

Problem: When attempting to delete a field like {"A/B": "allow"} using the JSON Pointer escaped key "A~1B", the deletion would fail because ObjectNode.remove() was looking for a field literally named "A~1B" instead of the actual field name "A/B".

Solution: Modified the delete method to unescape JSON Pointer field names by converting ~1 back to / before calling ObjectNode.remove(). This allows the ObjectNode to find and remove the correct field.

Testing: Added testDelete_withSlashInFieldName_jsonPointerEscaping() test that verifies fields with forward slashes in their names can be successfully deleted using JSON Pointer escaped keys.

Issues Resolved

Resolves #5953

Check List

  • [Y ] New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
  • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

}

if (!baseNode.isMissingNode()) {
((ObjectNode) baseNode).remove(leafKey);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this issue only with leafkey? What if the key is "A/B/C/D" where "B/C" is a single key inside A and with child D?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, I also wonder if this is general enough.

Relatedly, this is in the delete() call. Is this not a problem for put and get as well? And it would be better to fix this in the EventKey would result in better performance as well because you won't need to call String::replace for every delete operation.

@dlvenable

Copy link
Copy Markdown
Member

@jkbedi , Thank you for the contribution. We have a few questions about the implementation.

Also, your DCO is failing. Please see:

https://github.com/opensearch-project/data-prepper/blob/main/CONTRIBUTING.md

And you will need to rebase your commits so that all have the DCO Sign-Off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Delete entries and any processor that uses delete does not work when field name has '/'

3 participants