Skip to content

fix(jsonparser): Fix possible JSON log line corruption caused by json parser on query path #18056

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

Merged
merged 3 commits into from
Jun 11, 2025

Conversation

chaudum
Copy link
Contributor

@chaudum chaudum commented Jun 11, 2025

What this PR does / why we need it:

This PR fixes two bugs in the json parser:

  1. A bug that caused the original log line to be mutated by the json parser when the extracted field was a nested field and collided with an already existing label name, such as in line {"app":{"id": 123, "name": "bar"}} and label app_name="foo".

  2. A bug that caused a wrong json path for the extracted field in the same conditions as 1, such as []string{"app", "name_extracted"} for app_name, instead of []string{"app", "name"}.

Which issue(s) this PR fixes:
Fixes #17267

@chaudum chaudum requested a review from a team as a code owner June 11, 2025 07:39
chaudum added 3 commits June 11, 2025 09:54
… log line

The json parser mutated the extracted unsafe string which can cause an
in-memory log entry on the ingester to be modified leading to a
malformed JSON string.

Signed-off-by: Christian Haudum <[email protected]>
The jsonpath for nested field that would result in duplicate labels and
therefore have the `_extracted` suffix, was incorrectly adding the
suffix to the last element of the json path.

Signed-off-by: Christian Haudum <[email protected]>
@chaudum chaudum force-pushed the chaudum/jsonparser-corruption branch from be9ee22 to 3246924 Compare June 11, 2025 07:54
Copy link
Contributor

@ashwanthgoli ashwanthgoli left a comment

Choose a reason for hiding this comment

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

lgtm!

@chaudum chaudum added type/bug Somehing is not working as expected backport k258 labels Jun 11, 2025
@chaudum chaudum merged commit 153bbbc into main Jun 11, 2025
67 checks passed
@chaudum chaudum deleted the chaudum/jsonparser-corruption branch June 11, 2025 08:11
loki-gh-app bot pushed a commit that referenced this pull request Jun 11, 2025
…n` parser on query path (#18056)

This PR fixes two bugs in the json parser:

1. A bug that caused the original log line to be mutated by the json parser when the extracted field was a nested field and collided with an already existing label name, such as in line `{"app":{"id": 123, "name": "bar"}}` and label `app_name="foo"`.

2. A bug that caused a wrong json path for the extracted field in the same conditions as 1, such as `[]string{"app", "name_extracted"}` for `app_name`, instead of `[]string{"app", "name"}`.

Signed-off-by: Christian Haudum <[email protected]>
(cherry picked from commit 153bbbc)
loki-gh-app bot pushed a commit that referenced this pull request Jun 11, 2025
…n` parser on query path (#18056)

This PR fixes two bugs in the json parser:

1. A bug that caused the original log line to be mutated by the json parser when the extracted field was a nested field and collided with an already existing label name, such as in line `{"app":{"id": 123, "name": "bar"}}` and label `app_name="foo"`.

2. A bug that caused a wrong json path for the extracted field in the same conditions as 1, such as `[]string{"app", "name_extracted"}` for `app_name`, instead of `[]string{"app", "name"}`.

Signed-off-by: Christian Haudum <[email protected]>
(cherry picked from commit 153bbbc)
@loki-gh-app
Copy link
Contributor

loki-gh-app bot commented Jun 11, 2025

The backport to release-3.4.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-18056-to-release-3.4.x origin/release-3.4.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 153bbbcf8a1b8793dc12fe4539e536773c4aa459

When the conflicts are resolved, stage and commit the changes:

git add . && git cherry-pick --continue

If you have the GitHub CLI installed:

# Push the branch to GitHub:
git push --set-upstream origin backport-18056-to-release-3.4.x
# Create the PR body template
PR_BODY=$(gh pr view 18056 --json body --template 'Backport 153bbbcf8a1b8793dc12fe4539e536773c4aa459 from #18056{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title 'fix(jsonparser): Fix possible JSON log line corruption caused by `json` parser on query path (backport release-3.4.x)' --body-file - --label 'size/M' --label 'type/bug' --label 'backport' --base release-3.4.x --milestone release-3.4.x --web

Or, if you don't have the GitHub CLI installed (we recommend you install it!):

# Push the branch to GitHub:
git push --set-upstream origin backport-18056-to-release-3.4.x

# Create a pull request where the `base` branch is `release-3.4.x` and the `compare`/`head` branch is `backport-18056-to-release-3.4.x`.

# Remove the local backport branch
git switch main
git branch -D backport-18056-to-release-3.4.x

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

Successfully merging this pull request may close these issues.

[Querier] Some JSON appears corrupted by label detection
2 participants