fix(lib): scans didn't stop on ctx cancellation#6310
Conversation
Signed-off-by: Dwi Siswanto <git@dw1.io>
WalkthroughThe changes introduce context-aware cancellation to the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant NucleiEngine
participant ScanGoroutine
participant Context
Caller->>NucleiEngine: ExecuteCallbackWithCtx(ctx, callback)
NucleiEngine->>ScanGoroutine: Start scan in goroutine
NucleiEngine->>Context: Wait for ctx.Done()
alt Context cancelled
Context-->>NucleiEngine: ctx.Done()
NucleiEngine->>Caller: Return context.Canceled error
else Scan completes
ScanGoroutine-->>NucleiEngine: Scan finished
NucleiEngine->>Caller: Return scan result
end
Assessment against linked issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
lib/sdk_test.go (1)
29-29: Consider using a test server instead of external URL.The test depends on an external URL
http://honey.scanme.shwhich could make tests flaky if the service is unavailable. Consider using a local test server or mocking the target.Would you like me to help create a local test server setup to make the test more reliable and self-contained?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/sdk.go(6 hunks)lib/sdk_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
lib/sdk.go (1)
Learnt from: dogancanbakir
PR: projectdiscovery/nuclei#6059
File: pkg/authprovider/file.go:144-145
Timestamp: 2025-02-19T13:21:35.933Z
Learning: In the Nuclei project, when checking auth strategies returned from LookupAddr, use len() check instead of nil check as the function returns an empty slice for no matches.
🧬 Code Graph Analysis (2)
lib/sdk_test.go (2)
lib/sdk.go (1)
NewNucleiEngineCtx(306-322)lib/config.go (4)
WithTemplateFilters(64-95)TemplateFilters(48-60)EnableStatsWithOpts(228-242)StatsOptions(220-224)
lib/sdk.go (2)
pkg/protocols/common/contextargs/contextargs.go (1)
Context(22-33)pkg/output/output.go (1)
ResultEvent(146-221)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests (windows-latest)
- GitHub Check: Tests (macOS-latest)
🔇 Additional comments (3)
lib/sdk.go (3)
251-254: Good variable naming to avoid shadowing.Renaming the loop variable from
callbacktocbprevents shadowing of the parameter name.
283-288: Good backward compatibility implementation.The method correctly uses the stored context if available, falling back to
context.Background()for compatibility.
329-337: Well-implemented helper function for WaitGroup to channel conversion.The
waitfunction correctly converts a WaitGroup into a channel that can be used with select statements. The channel is properly closed when the WaitGroup completes.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Dwi Siswanto <git@dw1.io>
Mzack9999
left a comment
There was a problem hiding this comment.
lgtm! Minor change on return error on scan when SDK is used
* fix(lib): scans didn't stop on ctx cancellation Signed-off-by: Dwi Siswanto <git@dw1.io> * Update lib/sdk_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(lib): wait resources to be released b4 return Signed-off-by: Dwi Siswanto <git@dw1.io> --------- Signed-off-by: Dwi Siswanto <git@dw1.io> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Proposed changes
ctxfield toNucleiEnginestruct to store the context.NucleiEngineinstance inExecuteWithCallbackmethod.ExecuteCallbackWithCtxmethod to makeExecuteScanWithOptsrun in a goroutine and wait for either:Fixes #6298
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests