fix(pkg/yamlprocessor): guard GetValue, ReplaceString, and Bytes against nil processor - #7378
Closed
Norway-02 wants to merge 1 commit into
Closed
fix(pkg/yamlprocessor): guard GetValue, ReplaceString, and Bytes against nil processor#7378Norway-02 wants to merge 1 commit into
Norway-02 wants to merge 1 commit into
Conversation
…nst nil processor Signed-off-by: Norway-02 <anshulkhetade02@gmail.com>
Author
|
cc @mohammedfirdouss @TheRealShek @Warashi - Please review when you get a chance. Thanks! |
✅ Deploy Preview for pipecd-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues were identified.
Pull request overview
Adds nil-safety guards to YAML processor methods, preventing panics for nil or uninitialized processors.
Changes:
- Guards
GetValue,ReplaceString, andBytes. - Adds regression tests for nil and uninitialized processors.
File summaries
| File | Description |
|---|---|
pkg/yamlprocessor/yamlprocessor.go |
Adds defensive nil and file checks. |
pkg/yamlprocessor/yamlprocessor_test.go |
Tests nil and uninitialized processor behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
Thanks for the PR, but I don't think this change is warranted. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does:
Adds defensive nil-receiver and uninitialized AST file pointer guards (
p == nil || p.file == nil) toGetValue(),ReplaceString(), andBytes()inpkg/yamlprocessor.Why we need it:
Calling
GetValue(),ReplaceString(), orBytes()on an uninitialized or nil*Processorpreviously caused an unhandledruntime error: invalid memory address or nil pointer dereferencepanic. Adding these checks prevents panics and safely returns clean error messages or nil byte slices.Which issue(s) this PR fixes:
Fixes # NONE
Does this PR introduce a user-facing change?:
Screenshots/Videos (for documentation or website changes):
N/A