Skip to content

fix(jsonwrap): handle potential null keys - #22326

Merged
stelfrag merged 3 commits into
netdata:masterfrom
stelfrag:fix_jsonwrap_null_deref
Apr 30, 2026
Merged

fix(jsonwrap): handle potential null keys#22326
stelfrag merged 3 commits into
netdata:masterfrom
stelfrag:fix_jsonwrap_null_deref

Conversation

@stelfrag

@stelfrag stelfrag commented Apr 30, 2026 •
edited by cubic-dev-ai Bot
Loading

Copy link
Copy Markdown
Collaborator
Summary
  • Fallback to id and check for NULL to avoid NULL dereference 
    SIGSEGV / SEGV_MAPERR / 0x0
Fatal Error: SIGSEGV / SEGV_MAPERR / 0x0
query_target_summary_dimensions_v12.cold (jsonwrap-summary-dimensions.c:148)
rrdr_json_wrapper_begin2.cold (jsonwrap-v2.c:414)

Summary by cubic

Fixes a crash in the JSON summary dimensions formatter by falling back to metric ID when the name is missing and skipping entries with no valid key. Adds dev-mode error logging for visibility and prevents SIGSEGV in v2 JSON wrapping.

  • Bug Fixes
    • Fallback to rrdmetric_acquired_id(rma) when rrdmetric_acquired_name(rma) is null or empty.
    • Skip the dimension when both name and id are missing in query_target_summary_dimensions_v12.
    • Log a dev-mode error when skipping a dimension with no valid key.

Written for commit 2ae137e. Summary will update on new commits. Review in cubic

@stelfrag
stelfrag marked this pull request as ready for review April 30, 2026 09:24
@stelfrag
stelfrag requested a review from thiagoftsm as a code owner April 30, 2026 09:24
Copilot AI review requested due to automatic review settings April 30, 2026 09:24
@stelfrag
stelfrag requested a review from vkalintiris as a code owner April 30, 2026 09:24
@stelfrag
stelfrag marked this pull request as draft April 30, 2026 09:24
Copilot AI reviewed Apr 30, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Prevents a reported SIGSEGV in the JSON wrapper summary-dimensions path by hardening key selection and handling dictionary insertion failures.

Changes:

  • Falls back from an empty/null metric name to the metric id when selecting the dictionary key.
  • Skips processing when dictionary_set() fails (e.g., OOM) to avoid dereferencing a null entry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/web/api/formatters/jsonwrap-summary-dimensions.c Outdated
Comment thread src/web/api/formatters/jsonwrap-summary-dimensions.c
Comment thread src/web/api/formatters/jsonwrap-summary-dimensions.c Outdated
@stelfrag
stelfrag requested a review from Copilot April 30, 2026 09:54
@stelfrag
stelfrag marked this pull request as ready for review April 30, 2026 10:01
Copilot AI reviewed Apr 30, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/web/api/formatters/jsonwrap-summary-dimensions.c
Comment thread src/web/api/formatters/jsonwrap-summary-dimensions.c
@stelfrag stelfrag changed the title fix(jsonwrap): handle potential null keys and dictionary set failures fix(jsonwrap): handle potential null keys Apr 30, 2026
@sonarqubecloud

sonarqubecloud Bot commented Apr 30, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

thiagoftsm
thiagoftsm approved these changes Apr 30, 2026
View reviewed changes

@thiagoftsm thiagoftsm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found after more or less one hour running. LGTM!

@stelfrag
stelfrag merged commit 645355e into netdata:master Apr 30, 2026
157 checks passed
@stelfrag
stelfrag deleted the fix_jsonwrap_null_deref branch April 30, 2026 14:08
@stelfrag stelfrag mentioned this pull request Jun 22, 2026
Merged
Ferroin pushed a commit that referenced this pull request Jul 15, 2026
@stelfrag @Ferroin
fix(jsonwrap): handle potential null keys (#22326)
c7fa6fb 
* fix(jsonwrap): handle potential null keys and dictionary set failures

* address comments

* fix(jsonwrap): Add dev mode error message logging

(cherry picked from commit 645355e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@thiagoftsm thiagoftsm thiagoftsm approved these changes
@vkalintiris vkalintiris Awaiting requested review from vkalintiris vkalintiris is a code owner

Assignees

No one assigned

Labels

area/web

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stelfrag @thiagoftsm