fix(file source) Fix a data corruption bug with multi-char delimiters #24028

lfrancke · 2025-10-20T13:14:11Z

Summary

Fix problem where multi-chars delimiters fail to be parsed if they happen right at a buffer boundary.

Vector configuration

See https://github.com/lfrancke/vector-repro-24027 for a reproduction repository

How did you test this PR?

The repro repo contains a test case which I used.
In addition I added unit tests for 1-5 char delimiters.

Change Type

Bug fix
New feature
Non-functional (chore, refactoring, docs)
Performance

Is this a breaking change?

Yes
No

Does this PR include user facing changes?

Yes. Please add a changelog fragment based on our guidelines.
No. A maintainer will apply the no-changelog label to this PR.

References

Closes: Multi-byte line delimiters split across buffer boundaries cause log event merging #24027

Notes

Please read our Vector contributor resources.
Do not hesitate to use @vectordotdev/vector to reach out to us regarding this PR.
Some CI checks run only after we manually approve them.
- We recommend adding a pre-push hook, please see this template.
- Alternatively, we recommend running the following locally before pushing to the remote branch:
  - make fmt
  - make check-clippy (if there are failures it's possible some of them can be fixed with make clippy-fix)
  - make test
After a review is requested, please avoid force pushes to help us review incrementally.
- Feel free to push as many commits as you want. They will be squashed into one before merging.
- For example, you can run git merge origin master and git push.
If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
run make build-licenses to regenerate the license inventory and commit the changes (if any). More details here.

…ppen right at a buffer boundary.

thomasqueirozb · 2025-10-20T16:34:53Z

Hi @lfrancke thanks for your contribution! Since this is something that alters Vector behavior it is considered a user facing change (I edited the PR description already). Could you please add a changelog? Thanks!

Also, your changes seem sound to me but I still need to review them more throughly. I will take a closer look soon

lfrancke · 2025-10-20T17:25:29Z

Will do! Thanks.
I see that I left two of my debugging statements in the test as well. I'll remove those too.

lfrancke · 2025-10-20T19:49:48Z

I pushed the changelog and removed the debug statements. It's ready for review I believe.

NOTE: I removed async/await parts from the original patch as that comes after 0.49.0 ```sh pushd $(cargo patchable checkout vector 0.49.0) git remote add lfrancke https://github.com/lfrancke/vector git fetch lfrancke git cherry-pick 3ce729073f23631dd7b5525be640b5fa15af0223 and git cherry-pick --continue git commit --amend popd cargo patchable export vector 0.49.0 ```

* chore(vector): Init patchable * chore(stackable-devel): Make a special variant for Vector so that a different rust toolchain can be selected * chore(stackable-devel): Add note about moving the version to boil-config.toml once renovate can check there (for consistency) * chore(nix): Add rust and cargo dependencies Otherwise cargo can't be found ``` error: the 'cargo' binary, normally provided by the 'cargo' component, is not applicable to the '1.89.0-x86_64-unknown-linux-gnu' toolchain ``` * chore(vector): Build from source (based on ubi9-rust-builder) NOTE: The ubi9-rust-builder could not be used as it contains `ONBUILD` steps which we need to run after patchable does it's thing. Also it is specifically designed for operators and their layout (under `rust/` and using workspaces). * chore(nix): Remove unused image-tools * chore(issue_template/vector): Update instructions for version bumps * fix(vector): Cherry pick unmerged patch from vectordotdev/vector#24028 NOTE: I removed async/await parts from the original patch as that comes after 0.49.0 ```sh pushd $(cargo patchable checkout vector 0.49.0) git remote add lfrancke https://github.com/lfrancke/vector git fetch lfrancke git cherry-pick 3ce729073f23631dd7b5525be640b5fa15af0223 and git cherry-pick --continue git commit --amend popd cargo patchable export vector 0.49.0 ``` * chore(vector): Add maintainer label This seems to be added to other images, so I'm just copying that. * chore: Update changelog * Apply suggestions from code review Co-authored-by: Techassi <[email protected]> * chore(vector): Remove unused upload script * chore(vector): Remove old comments, add new todo --------- Co-authored-by: Techassi <[email protected]>

lfrancke · 2025-11-03T08:14:37Z

@thomasqueirozb a quick ping. Considering that it corrupts data I hope a ping is fine here.

thomasqueirozb

Thanks for the contribution, this fix is very welcome! Sorry for the delay

thomasqueirozb · 2025-11-14T21:26:08Z

lib/file-source-common/src/buffer.rs

+                        // Check if the end matches a prefix of the delimiter.
+                        // We iterate from longest to shortest prefix and break on first match.
+                        // Performance: For typical 2-byte delimiters (CRLF), this is 1 iteration.
+                        // For longer delimiters, this runs O(delim_len) times but only occurs
+                        // at buffer boundaries (~every 8KB), making the impact negligible.


If I'm reading this correctly this means that this will only run more than once if the delimiter is >8kb or if we fail to fetch the whole delimiter this iteration (maybe due to some io issue). Is this correct?

Instead of answering here I tried to add more comments: 0254302 (#24028)

I had to reread my code as well and it's only been a month. I hope these new explanations help.

One thing that I'm not sure about - but I'd be happy if we could leave it out of scope for now - if someone were to use a delimiter itself that is larger than 8kb...... I didn't think that through.

thomasqueirozb · 2025-11-14T21:27:42Z

lib/file-source-common/src/buffer.rs

+    /// 1. Creates test data with delimiters positioned to split at buffer boundaries
+    /// 2. Tests multiple iterations to ensure state tracking works correctly
+    /// 3. Verifies all lines are correctly separated without merging
+    async fn test_delimiter_boundary_split_helper(delimiter: &[u8], num_lines: usize) {


Does this work if we add a #[cfg(test)] so that this isn't compiled when outside of tests?

Absolutely! Thanks.

Addressed in 6fc45e6 (#24028)

thomasqueirozb · 2025-11-14T21:29:39Z

lib/file-source-common/src/buffer.rs

+    #[tokio::test]
+    async fn test_single_byte_delimiter_boundary() {
+        // Test single-byte delimiter (should work without any special handling)
+        test_delimiter_boundary_split_helper(b"\n", 5).await;
+    }
+
+    #[tokio::test]
+    async fn test_two_byte_delimiter_boundary() {
+        // Test two-byte delimiter (CRLF case)
+        test_delimiter_boundary_split_helper(b"\r\n", 5).await;
+    }
+
+    #[tokio::test]
+    async fn test_three_byte_delimiter_boundary() {
+        test_delimiter_boundary_split_helper(b"|||", 5).await;
+    }
+
+    #[tokio::test]
+    async fn test_four_byte_delimiter_boundary() {
+        test_delimiter_boundary_split_helper(b"<|>|", 5).await;
+    }
+
+    #[tokio::test]
+    async fn test_five_byte_delimiter_boundary() {
+        test_delimiter_boundary_split_helper(b"<<>>>", 5).await;
+    }


These tests are nice. Are we able to build something similar by creating a test for the file source? I guess it shouldn't be that hard given the existing repro repo. If we create a file with the same data as in the repo we should be able to assert the data is correct (and also that the test should fail on master today).

I added something based on the existing test that was already there. I hope that's what you meant. It passes for me with my patch and fails for me without.

changelog.d/24027_file_source_multi_char_delimiter.fix.md

Co-authored-by: Thomas <[email protected]>

Fix problem where multi-chars delimiters fail to be parsed if they ha…

3ce7290

…ppen right at a buffer boundary.

lfrancke requested a review from a team as a code owner October 20, 2025 13:14

lfrancke mentioned this pull request Oct 20, 2025

Multi-byte line delimiters split across buffer boundaries cause log event merging #24027

Open

lfrancke added 2 commits October 20, 2025 21:44

Add changelog entry and remove debug println statements

cf9ccf3

Merge branch 'master' into fix/multi-char-delimiter

85bbe23

NickLarsenNZ mentioned this pull request Oct 28, 2025

Apply the **Vector** fix to the Vector image layer. stackabletech/issues#789

Closed

5 tasks

lfrancke added 2 commits November 4, 2025 22:27

Merge branch 'master' into fix/multi-char-delimiter

a2cb741

Merge branch 'master' into fix/multi-char-delimiter

8dfbed0

pront assigned thomasqueirozb Nov 14, 2025

thomasqueirozb reviewed Nov 14, 2025

View reviewed changes

thomasqueirozb added the meta: awaiting author Pull requests that are awaiting their author. label Nov 17, 2025

Update changelog.d/24027_file_source_multi_char_delimiter.fix.md

72374a5

Co-authored-by: Thomas <[email protected]>

github-actions bot removed the meta: awaiting author Pull requests that are awaiting their author. label Nov 20, 2025

lfrancke added 3 commits November 20, 2025 11:53

Add #[cfg(test)] to test method

6fc45e6

Expand comments

0254302

Add a regression/integration test to file source

f4d6884

github-actions bot added the domain: sources Anything related to the Vector's sources label Nov 20, 2025

lfrancke requested a review from thomasqueirozb November 20, 2025 12:12

fix(file source) Fix a data corruption bug with multi-char delimiters #24028

Are you sure you want to change the base?

fix(file source) Fix a data corruption bug with multi-char delimiters #24028

Conversation

lfrancke commented Oct 20, 2025 • edited by thomasqueirozb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Vector configuration

How did you test this PR?

Change Type

Is this a breaking change?

Does this PR include user facing changes?

References

Notes

Uh oh!

thomasqueirozb commented Oct 20, 2025

Uh oh!

lfrancke commented Oct 20, 2025

Uh oh!

lfrancke commented Oct 20, 2025

Uh oh!

lfrancke commented Nov 3, 2025

Uh oh!

thomasqueirozb left a comment

Choose a reason for hiding this comment

Uh oh!

thomasqueirozb Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

lfrancke Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

lfrancke Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

thomasqueirozb Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

lfrancke Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

thomasqueirozb Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

lfrancke Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lfrancke commented Oct 20, 2025 •

edited by thomasqueirozb

Loading