fix(sdk): NoHostErrors not set #5783
Conversation
|
Thanks so much for your contribution @iuliu8899 ! :) |
WalkthroughThe changes in this pull request primarily involve modifications to the handling of host error management within the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NucleiEngine
participant Config
User->>Config: Set Network Config
Config->>NucleiEngine: WithNetworkConfig(opts)
NucleiEngine->>NucleiEngine: Check ShouldUseHostError()
NucleiEngine->>NucleiEngine: Adjust MaxHostError if needed
NucleiEngine->>NucleiEngine: Initialize hostErrCache
NucleiEngine->>User: Configuration updated
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
lib/sdk_private.go (2)
164-178: Consider reorganizing the executor options initializationWhile the conditional assignment of
HostErrorsCacheis correct, consider reorganizing the code to determine all fields before creating theExecutorOptionsstruct. This would make the initialization more maintainable and easier to understand.Here's a suggested refactor:
+ // Prepare executor options fields + hostErrorsCache := e.hostErrCache + if !e.opts.ShouldUseHostError() || e.hostErrCache == nil { + hostErrorsCache = nil + } + e.executerOpts = protocols.ExecutorOptions{ Output: e.customWriter, Options: e.opts, Progress: e.customProgress, Catalog: e.catalog, IssuesClient: e.rc, RateLimiter: e.rateLimiter, Interactsh: e.interactshClient, Colorizer: aurora.NewAurora(true), ResumeCfg: types.NewResumeCfg(), Browser: e.browserInstance, Parser: e.parser, InputHelper: input.NewHelper(), + HostErrorsCache: hostErrorsCache, } - if e.opts.ShouldUseHostError() && e.hostErrCache != nil { - e.executerOpts.HostErrorsCache = e.hostErrCache - }
Line range hint
228-234: Fix error variable shadowing in httpx client initializationThe error variable is shadowed in the if-else statement, which could mask errors from the outer scope. Consider using a separate variable name or restructuring the error handling.
Here's the suggested fix:
httpxOptions := httpx.DefaultOptions httpxOptions.Timeout = 5 * time.Second - if client, err := httpx.New(&httpxOptions); err != nil { + client, httpxErr := httpx.New(&httpxOptions) + if httpxErr != nil { - return err + return httpxErr } else { e.httpxClient = nucleiUtils.GetInputLivenessChecker(client) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
lib/config.go(1 hunks)lib/multi.go(1 hunks)lib/sdk_private.go(2 hunks)
🔇 Additional comments (5)
lib/multi.go (2)
32-42: LGTM: ExecutorOptions initialization is well-structured
The initialization of ExecutorOptions follows a clear pattern and properly sets all required fields from the base instance.
43-44: LGTM: Proper conditional initialization of HostErrorsCache
The added condition properly ensures that HostErrorsCache is only set when host error handling is enabled and the cache exists. This fix addresses the "NoHostErrors not set" issue (#5782).
Let's verify the fix is complete by checking for any other locations that might need similar conditional handling:
✅ Verification successful
Based on the search results, I can now generate the final response:
Verified: HostErrorsCache initialization is properly handled across the codebase
The conditional initialization of HostErrorsCache in lib/multi.go is consistent with other implementations in the codebase. All usages follow the same pattern:
- Initialization only occurs when
ShouldUseHostError()is true - Null checks are performed before using the cache
- The cache is properly cleaned up when needed
Key verified locations:
lib/sdk_private.go: Similar conditional initializationlib/config.go: Consistent error handling setup- Protocol implementations (HTTP, Network): Proper null checks before usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other potential locations handling HostErrorsCache
rg -A 2 "HostErrorsCache|hostErrCache|ShouldUseHostError"
Length of output: 6177
lib/sdk_private.go (1)
76-78: LGTM: Improved host error cache initialization
The condition for initializing the host error cache has been properly updated to check both if the cache is nil and if host error handling is enabled. This prevents unnecessary resource allocation when host error handling is disabled.
lib/config.go (2)
289-290: LGTM: Proper initialization of host error settings
The code correctly initializes the host error settings from the NetworkConfig options:
- NoHostErrors is set from DisableMaxHostErr
- MaxHostError is set from the config
291-302: Verify host error cache initialization logic
The host error cache initialization looks good with proper validation and adjustment of MaxHostError. However, there are a few points to consider:
- The warning message about concurrency being higher than max-host-error could be more specific.
- The cache initialization should be protected against potential edge cases.
Consider these improvements:
if e.opts.ShouldUseHostError() {
maxHostError := opts.MaxHostError
if e.opts.TemplateThreads > maxHostError {
- gologger.Print().Msgf("[%v] The concurrency value is higher than max-host-error", e.executerOpts.Colorizer.BrightYellow("WRN"))
- gologger.Info().Msgf("Adjusting max-host-error to the concurrency value: %d", e.opts.TemplateThreads)
+ gologger.Print().Msgf("[%v] Template concurrency (%d) is higher than max-host-error (%d)",
+ e.executerOpts.Colorizer.BrightYellow("WRN"), e.opts.TemplateThreads, maxHostError)
+ gologger.Info().Msgf("Adjusting max-host-error to match template concurrency: %d", e.opts.TemplateThreads)
maxHostError = e.opts.TemplateThreads
e.opts.MaxHostError = maxHostError
}
+ if maxHostError <= 0 {
+ maxHostError = hosterrorscache.DefaultMaxHostsCount
+ }
cache := hosterrorscache.New(maxHostError, hosterrorscache.DefaultMaxHostsCount, e.opts.TrackError)
cache.SetVerbose(e.opts.Verbose)
e.hostErrCache = cache
}Let's verify the usage of ShouldUseHostError and related host error settings:
✅ Verification successful
Host error cache initialization and validation is properly implemented
The verification shows that the host error handling implementation is robust and consistent across the codebase:
- The
ShouldUseHostError()function inpkg/types/types.gocorrectly checks both conditions:MaxHostError > 0 && !NoHostErrors - Default value for
MaxHostErroris set to 30 in the options - The warning about concurrency being higher than max-host-error is consistently implemented in both
lib/config.goandinternal/runner/runner.go - The host error cache properly tracks and enforces the error limits through
MaxHostErrorfield
The suggested improvements in the review comment, while making the messages more descriptive, aren't strictly necessary as the current implementation is already functionally sound and consistent.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of ShouldUseHostError and related host error settings
# Check for ShouldUseHostError implementation
ast-grep --pattern 'func $_($$) bool {
$$$
return !$_.NoHostErrors
$$$
}'
# Check for other references to NoHostErrors and MaxHostError
rg -A 3 'NoHostErrors|MaxHostError'
Length of output: 4282
Proposed changes
#5782
Checklist
Summary by CodeRabbit
New Features
HostErrorsCachebased on user-defined options.Bug Fixes
Documentation