-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(file source) Fix a data corruption bug with multi-char delimiters #24028
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
base: master
Are you sure you want to change the base?
Conversation
…ppen right at a buffer boundary.
|
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 |
|
Will do! Thanks. |
|
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 ```
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 <sascha.lautenschlaeger@stackable.tech> * chore(vector): Remove unused upload script * chore(vector): Remove old comments, add new todo --------- Co-authored-by: Techassi <sascha.lautenschlaeger@stackable.tech>
|
@thomasqueirozb a quick ping. Considering that it corrupts data I hope a ping is fine here. |
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
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Notes
@vectordotdev/vectorto reach out to us regarding this PR.pre-pushhook, please see this template.make fmtmake check-clippy(if there are failures it's possible some of them can be fixed withmake clippy-fix)make testgit merge origin masterandgit push.Cargo.lock), pleaserun
make build-licensesto regenerate the license inventory and commit the changes (if any). More details here.