Skip to content

Fix nil pointer dereference in toComDetails when port parsing fails#491

Open
sebrandon1 wants to merge 1 commit into
openshift-kni:mainfrom
sebrandon1:fix-nil-deref-parseComDetail
Open

Fix nil pointer dereference in toComDetails when port parsing fails#491
sebrandon1 wants to merge 1 commit into
openshift-kni:mainfrom
sebrandon1:fix-nil-deref-parseComDetail

Conversation

@sebrandon1

@sebrandon1 sebrandon1 commented May 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • parseComDetail() returns nil when strconv.Atoi fails on the port string, but toComDetails() dereferences the result at *cd without a nil check, causing a panic
  • Added a nil guard to skip unparseable ss entries — the debug log inside parseComDetail already records the failure

Test plan

  • Existing e2e tests pass (commatrix-e2e-tests-1of2, 2of2)
  • Unit tests pass
  • Lint passes

@openshift-ci openshift-ci Bot requested review from oribon and yuvalk May 4, 2026 18:03
@sebrandon1

sebrandon1 commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

/retest commatrix-e2e-tests-1of2

@sebrandon1

sebrandon1 commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

/retest commatrix-e2e-tests-2of2

@sebrandon1

sebrandon1 commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

/retest

10 similar comments
@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1

sebrandon1 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@aabughosh

aabughosh commented May 6, 2026

Copy link
Copy Markdown
Collaborator

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 6, 2026
@oribon

oribon commented May 13, 2026

Copy link
Copy Markdown
Member

/approve

@openshift-ci

openshift-ci Bot commented May 13, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oribon, sebrandon1

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2026
@sebrandon1

sebrandon1 commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@sebrandon1

sebrandon1 commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@sebrandon1
Fix nil pointer dereference in toComDetails when port parsing fails
d30e9af 
parseComDetail returns nil when strconv.Atoi fails on the port string,
but the caller dereferences the result without a nil check, causing a
panic. Add a nil guard to skip unparseable ss entries.
@sebrandon1 sebrandon1 force-pushed the fix-nil-deref-parseComDetail branch from d2643d0 to d30e9af Compare May 19, 2026 20:50
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label May 19, 2026
@openshift-ci

openshift-ci Bot commented May 19, 2026

Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@coderabbitai

coderabbitai Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 20e33c0b-729a-45bc-8c85-68fd30b2cb02

📥 Commits

Reviewing files that changed from the base of the PR and between c70fe1e and d30e9af.

📒 Files selected for processing (1)
  • pkg/listening-sockets/listening-sockets.go
📝 Walkthrough

Walkthrough

The PR adds a defensive nil-check in the toComDetails function to guard against parseComDetail returning nil when port parsing fails. When nil is returned, the entry is skipped instead of being dereferenced, preventing potential panics.

Nil-check in parseComDetail flow

Layer / File(s) Summary
ParseComDetail nil-guard defense
pkg/listening-sockets/listening-sockets.go		 Added nil-check after parseComDetail(ssEntry) invocation; when nil is returned, the current entry is skipped to avoid dereferencing nil and prevent downstream panics during port parsing failures.

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a nil pointer dereference guard in toComDetails when port parsing fails.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the bug (nil dereference when parseComDetail fails), the fix (adding nil guard), and the test plan.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Reviewers

@oribon oribon Awaiting requested review from oribon

@yuvalk yuvalk Awaiting requested review from yuvalk

Assignees

@aabughosh aabughosh

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sebrandon1 @aabughosh @oribon