fix(loader): avoid panic when dialers are missing#7066
fix(loader): avoid panic when dialers are missing#7066chuhuan88 wants to merge 1 commit intoprojectdiscovery:devfrom
Conversation
Attempt protocolstate initialization when dialers are absent and return safely with warnings if still unavailable.
WalkthroughIn Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/catalog/loader/loader.go (1)
718-729: LGTM - Graceful degradation replaces panic appropriately.The fallback initialization pattern correctly prevents hard crashes in partial-init paths. The flow is safe since this check executes before concurrent goroutines spawn.
Minor observation: The
dialersvariable is assigned but not directly used after this block (dialers are accessed through global state downstream). Consider simplifying for clarity:♻️ Optional simplification
- dialers := protocolstate.GetDialersWithId(typesOpts.ExecutionId) - if dialers == nil { + if protocolstate.GetDialersWithId(typesOpts.ExecutionId) == nil { if err := protocolstate.Init(typesOpts); err != nil { store.logger.Warning().Msgf("could not initialize dialers for executionId %s: %s", typesOpts.ExecutionId, err) return loadedTemplates.Slice } - dialers = protocolstate.GetDialersWithId(typesOpts.ExecutionId) - if dialers == nil { + if protocolstate.GetDialersWithId(typesOpts.ExecutionId) == nil { store.logger.Warning().Msgf("dialers with executionId %s not found", typesOpts.ExecutionId) return loadedTemplates.Slice } },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/catalog/loader/loader.go` around lines 718 - 729, The local variable dialers is assigned but never used downstream; simplify the block by removing the unused dialers variable and directly checking protocolstate.GetDialersWithId(typesOpts.ExecutionId) for nil, calling protocolstate.Init(typesOpts) on miss, re-checking GetDialersWithId, and keeping the existing store.logger.Warning messages and return loadedTemplates.Slice paths (referencing protocolstate.GetDialersWithId, protocolstate.Init, typesOpts.ExecutionId, store.logger.Warning, and loadedTemplates.Slice).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/catalog/loader/loader.go`:
- Around line 718-729: The local variable dialers is assigned but never used
downstream; simplify the block by removing the unused dialers variable and
directly checking protocolstate.GetDialersWithId(typesOpts.ExecutionId) for nil,
calling protocolstate.Init(typesOpts) on miss, re-checking GetDialersWithId, and
keeping the existing store.logger.Warning messages and return
loadedTemplates.Slice paths (referencing protocolstate.GetDialersWithId,
protocolstate.Init, typesOpts.ExecutionId, store.logger.Warning, and
loadedTemplates.Slice).
Proposed Changes
This PR removes a panic path in template loading when dialers are not initialized for the current execution ID.
pkg/catalog/loader/loader.go, whenprotocolstate.GetDialersWithId(typesOpts.ExecutionId)returnsnil, it now attempts a safe initialization viaprotocolstate.Init(typesOpts).This keeps behavior stable for normal scanning flows while preventing hard crashes in non-scanning/partial-init paths.
Proof
Before
panic("dialers with executionId ... not found")After
Local verification
go test ./pkg/catalog/loader -count=1Checklist
dev)/claim #6674
Summary by CodeRabbit