Skip to content

Conversation

@NikolaMilosa
Copy link
Contributor

@NikolaMilosa NikolaMilosa commented May 20, 2025

Deserializing incoming chunks as utf8_lossy breaks the encoding of journald protocol because there are non-utf8 bytes for the length of the message.

@NikolaMilosa NikolaMilosa requested a review from a team as a code owner May 20, 2025 11:51
@NikolaMilosa NikolaMilosa enabled auto-merge (squash) May 20, 2025 11:51
@NikolaMilosa NikolaMilosa merged commit 9d2bb47 into main May 20, 2025
9 checks passed
@NikolaMilosa NikolaMilosa deleted the nim-patching-log-fetcher branch May 20, 2025 11:52
@sasa-tomic sasa-tomic requested a review from Copilot May 20, 2025 11:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the buffering logic in the log-fetcher by switching from a String to a Vec for accumulating chunks, ensuring that non‑UTF8 bytes used in the journald protocol are preserved. Key changes include:

  • Replacing the String buffer with a Vec and updating related operations.
  • Adjusting the logic to find journal entry boundaries using byte slices.
  • Updating error logging to handle entries missing the __CURSOR field and adding tests for binary journal fields.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
rs/log-fetcher/src/main.rs Updated buffering logic to use Vec and modified error handling for missing __CURSOR.
rs/log-fetcher/src/journald_parser.rs Added a new test ensuring that binary journal fields are correctly processed.

Current chunk: {}\n\n\\
",
map,
std::str::from_utf8(&leftover_buffer).unwrap(),
Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

Using unwrap here may cause a panic if leftover_buffer contains non-UTF8 bytes. Consider using std::str::from_utf8_lossy to safely display the data.

Suggested change
std::str::from_utf8(&leftover_buffer).unwrap(),
std::str::from_utf8_lossy(&leftover_buffer),

Copilot uses AI. Check for mistakes.
",
map,
std::str::from_utf8(&leftover_buffer).unwrap(),
std::str::from_utf8(&chunk).unwrap()
Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

Using unwrap here may cause a panic if chunk contains non-UTF8 bytes. Consider using std::str::from_utf8_lossy to safely convert the bytes for logging.

Suggested change
std::str::from_utf8(&chunk).unwrap()
std::str::from_utf8_lossy(&chunk)

Copilot uses AI. Check for mistakes.
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.

2 participants