Fix dissect leading delimiter byte offset for multi-byte characters

[winklemad]

Description

DissectParser.parse seeds the value cursor with leadingDelimiter.length() — a char count — but uses it to index into the input byte array (input = inputString.getBytes(UTF_8)). When the leading delimiter contains a non-ASCII character its char length is smaller than its byte length, so the first value starts partway into the delimiter's bytes and comes back corrupted. Nothing is thrown — it silently returns the wrong value.

byte[] input = inputString.getBytes(StandardCharsets.UTF_8); // byte domain
...
int i = leadingDelimiter.length();   // char count used as an index into input[]
int valueStart = i;

Example — pattern »%{a}, input »foo (» = U+00BB = 2 bytes):

• leadingDelimiter.length() is 1, so valueStart = 1 — one byte into the 2-byte ».
• the first value becomes copyOfRange(input, 1, …) = [BB, 66, 6F, 6F], which decodes to "�foo".
• result is {a="�foo"}; expected {a="foo"}.

Root cause. The between-key delimiters in the same method are already handled in the byte domain — delimiter = dissectPair.getDelimiter().getBytes(UTF_8), indexed against input.length. Only the leading delimiter measured its offset with String.length(). testLogstashSpecs already relies on multi-byte between-key delimiters (», €) working, so multi-byte delimiters are a supported case — the leading position was just overlooked.

Fix. Measure the leading delimiter in bytes, matching the rest of the method:

int i = leadingDelimiter.getBytes(StandardCharsets.UTF_8).length;

One line; StandardCharsets is already imported. This is the only place in parse where a char length indexed the byte array — lines 137/221/222 operate on Strings in the char domain and are correct. Reachable via the dissect ingest processor with any pattern whose leading delimiter contains a non-ASCII character.

Test. Added testLeadingDelimiterMultiByte next to the existing testLeadingDelimiter, covering 2-byte (»), 3-byte (€, 子) and 4-byte / surrogate-pair (😀) leading delimiters, single and multiple keys, a repeated delimiter, and a multi-byte value. It fails before the fix (Expected: "foo" but: was "�foo") and passes after. The :libs:opensearch-dissect test suite plus precommit (spotless, checkstyle, forbiddenApis, licenseHeaders) are green locally.

Related Issues

No linked issue — found by reading the code.

Check List

• Functionality includes testing.
• Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 7f400dd)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 7f400dd

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Align guard check with byte-based parsing The guard still uses String.length() (char units), which can reject valid inputs whose char length equals the delimiter's char length but whose byte length differs, or slice a surrogate pair. Since parsing is byte-based, compare using the input's byte array against the leading delimiter's bytes for consistency with leadingDelimiterByteLength. libs/dissect/src/main/java/org/opensearch/dissect/DissectParser.java [223-226] if (inputString != null - && inputString.length() > leadingDelimiter.length() + && inputString.length() >= leadingDelimiter.length() && leadingDelimiter.equals(inputString.substring(0, leadingDelimiter.length()))) { byte[] input = inputString.getBytes(StandardCharsets.UTF_8); + if (input.length < leadingDelimiterByteLength) { + return null; + } Suggestion importance[1-10]: 3 __ Why: The suggestion raises a reasonable edge case about char vs byte length in the guard, but the proposed improved_code changes > to >= (altering semantics unrelated to the byte issue) and adds a redundant check, without truly aligning the guard to byte comparison. Marginal correctness improvement with questionable implementation.	Low

---

Previous suggestions

Suggestions up to commit 7f23336

Category	Suggestion	Impact
General	Cache leading delimiter byte length Avoid re-encoding leadingDelimiter to UTF-8 bytes just to get its length, since this allocates a throwaway byte array on every parse call. Cache the byte length once (e.g., store leadingDelimiterBytesLength as a field alongside leadingDelimiter) or reuse the byte array to reduce per-parse allocations on a hot path. libs/dissect/src/main/java/org/opensearch/dissect/DissectParser.java [228-229] // start dissection after the first delimiter, measured in bytes to stay aligned with the input byte[] -int i = leadingDelimiter.getBytes(StandardCharsets.UTF_8).length; +int i = leadingDelimiterBytesLength; Suggestion importance[1-10]: 5 __ Why: Valid performance optimization to avoid repeated UTF-8 encoding of leadingDelimiter on every parse call. The impact is modest since the delimiter is typically small, but caching it as a field is a reasonable improvement on a hot path.	Low

Suggestions up to commit 54ac7cf

Category	Suggestion	Impact
General	Cache leading delimiter byte length Recomputing the UTF-8 byte length on every parse call allocates a throwaway byte array per invocation. Since leadingDelimiter is set once at construction, cache its byte length as a field (e.g., leadingDelimiterByteLength) to avoid repeated encoding work on hot paths. libs/dissect/src/main/java/org/opensearch/dissect/DissectParser.java [228-229] // start dissection after the first delimiter, measured in bytes to stay aligned with the input byte[] -int i = leadingDelimiter.getBytes(StandardCharsets.UTF_8).length; +int i = leadingDelimiterByteLength; Suggestion importance[1-10]: 5 __ Why: Valid minor performance improvement to avoid allocating a byte array on each parse invocation, since leadingDelimiter is fixed at construction. Impact is modest but reasonable for a hot path.	Low

[github-actions]

Persistent review updated to latest commit 7f23336

[github-actions]

Persistent review updated to latest commit 7f400dd

[github-actions]

✅ Gradle check result for 7f400dd: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.51%. Comparing base (d603ae1) to head (7f400dd).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22444      +/-   ##
============================================
+ Coverage     73.46%   73.51%   +0.04%     
- Complexity    76477    76555      +78     
============================================
  Files          6104     6104              
  Lines        346568   346569       +1     
  Branches      49883    49883              
============================================
+ Hits         254598   254766     +168     
+ Misses        71693    71526     -167     
  Partials      20277    20277              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.