Skip to content

feat: fixed max-host-error blocking + progress mismatch + misc#6193

Merged
ehsandeep merged 5 commits intodevfrom
fix-maxhosterror-issue-port
May 7, 2025
Merged

feat: fixed max-host-error blocking + progress mismatch + misc#6193
ehsandeep merged 5 commits intodevfrom
fix-maxhosterror-issue-port

Conversation

@ehsandeep
Copy link
Member

@ehsandeep ehsandeep commented Apr 24, 2025

Proposed changes

Reference - #5633 (Added in Nuclei v3.3.3)
Reference - #5668 (Added in Nuclei v3.3.4)

Changes Summary

  • fixed max-host-error blocking wrong port for template with error
  • log total results with time taken at end of execution
  • skip non-executed requests with progress in flow protocol
  • fixed request calculation in http protocol for progress causing progress stuck at 80%

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • New Features

    • Added tracking and display of total scan duration and number of matches in the final scan summary.
    • Progress statistics now accurately reflect completed and unexecuted requests.
    • Enhanced output writers to report the number of results found.
    • Introduced detailed request counting methods to improve request generation tracking.
  • Bug Fixes

    • Improved cache key normalization and error tracking for host errors.
    • Fixed progress tracking for failed or skipped HTTP requests.
  • Refactor

    • Simplified and clarified request counting and progress update logic for HTTP requests.
    • Streamlined error handling and progress increments in HTTP request execution.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 24, 2025

Walkthrough

This set of changes introduces enhancements to request tracking, result counting, error handling, and progress reporting across multiple components. Timing and summary logging are improved in the enumeration runner. Output writers now track and expose the number of results written. The HTTP request generator gains accurate methods for total and remaining requests, with corresponding changes in request counting logic. Flow execution now tracks executed requests and reconciles progress for unexecuted ones. Host error caching and context key extraction are refined. Minor improvements are made to variable scoping, error handling, and progress counter updates.

Changes

File(s) Change Summary
internal/runner/runner.go Added timing measurement for enumeration execution, improved scan summary logging with duration and result count, and ensured progress bar stops before summary. Introduced helper shortDur for formatting durations.
pkg/core/execute_options.go Added variable shadowing to ensure correct template variable capture in goroutine closure.
pkg/output/output.go, pkg/output/multi_writer.go, pkg/output/output_stats.go, pkg/testutils/testutils.go Extended Writer interface and implementations with ResultCount() to track and expose the number of results written. Updated StandardWriter to use atomic counter. MultiWriter aggregates counts from underlying writers. StatsOutputWriter and MockOutputWriter return zero.
pkg/progress/progress.go Simplified SetRequests to increment by provided count without delta calculation.
pkg/protocols/common/hosterrorscache/hosterrorscache.go Improved normalization and key extraction logic to prioritize error attributes and prevent unnecessary reparsing of host:port strings.
pkg/protocols/http/build_request.go Removed the Total() method from requestGenerator.
pkg/protocols/http/request_generator.go Added Remaining() and Total() methods to requestGenerator for accurate request counting, excluding @once marked requests from repeated counting across payload iterations.
pkg/protocols/http/http.go Simplified Requests() method to use the new generator's total directly.
pkg/protocols/http/request.go Refined error handling, progress tracking, and host error marking to avoid redundant increments and ensure correctness. Moved error marking earlier and guarded against marking on nil errors.
pkg/tmplexec/flow/flow_executor.go, pkg/tmplexec/flow/flow_internal.go Added executed request tracking to FlowExecutor using a sync map, and reconciled progress for requests that were never executed. Introduced helper for request key generation.

Sequence Diagram(s)

sequenceDiagram
    participant Runner
    participant ProgressBar
    participant Enumeration
    participant OutputWriter

    Runner->>ProgressBar: Start()
    Runner->>Enumeration: RunEnumeration()
    Enumeration->>OutputWriter: Write(result)
    OutputWriter->>OutputWriter: Increment resultCount()
    Enumeration-->>Runner: Done
    Runner->>ProgressBar: Stop()
    Runner->>OutputWriter: ResultCount()
    Runner->>Runner: Log summary with duration and result count
Loading
sequenceDiagram
    participant FlowExecutor
    participant Protocol
    participant Request
    participant Progress

    FlowExecutor->>Protocol: Iterate requests
    Protocol->>Request: Execute (with id)
    Request-->>FlowExecutor: Execution complete
    FlowExecutor->>FlowExecutor: Mark request as executed
    FlowExecutor->>FlowExecutor: After all, reconcileProgress()
    FlowExecutor->>Progress: Mark unexecuted requests as finished
Loading

Poem

In the warren of code, the rabbits delight,
Counting each hop, every scan through the night.
With progress now tracked and results tallied true,
Errors are handled, and logs shine anew.
Flow hops are remembered, no request left behind—
A burrow of order, efficiency, and mind!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 0753b68 and 759aa51.

📒 Files selected for processing (3)
  • internal/runner/runner.go (3 hunks)
  • pkg/protocols/http/request.go (3 hunks)
  • pkg/tmplexec/flow/flow_internal.go (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • pkg/tmplexec/flow/flow_internal.go
  • internal/runner/runner.go
  • pkg/protocols/http/request.go
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Tests (macOS-latest)
  • GitHub Check: Tests (windows-latest)
  • GitHub Check: Tests (ubuntu-latest)

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Ice3man543 Ice3man543 marked this pull request as ready for review April 29, 2025 15:23
@auto-assign auto-assign bot requested a review from dwisiswant0 April 29, 2025 15:23
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
pkg/tmplexec/flow/flow_executor.go (1)

263-273: Progress reconciliation logic is well-implemented

The method correctly identifies unexecuted requests and updates the progress tracker to maintain accurate statistics. Consider enhancing the comment to better explain the purpose of this reconciliation.

-			// never executed → pretend it finished so that stats match
+			// For requests that weren't executed, update the progress counter
+			// to reflect completion. This ensures progress statistics remain
+			// accurate by accounting for requests that were skipped or not run.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 24311cc and 0753b68.

📒 Files selected for processing (14)
  • internal/runner/runner.go (2 hunks)
  • pkg/core/execute_options.go (1 hunks)
  • pkg/output/multi_writer.go (1 hunks)
  • pkg/output/output.go (4 hunks)
  • pkg/output/output_stats.go (1 hunks)
  • pkg/progress/progress.go (1 hunks)
  • pkg/protocols/common/hosterrorscache/hosterrorscache.go (2 hunks)
  • pkg/protocols/http/build_request.go (0 hunks)
  • pkg/protocols/http/http.go (1 hunks)
  • pkg/protocols/http/request.go (3 hunks)
  • pkg/protocols/http/request_generator.go (1 hunks)
  • pkg/testutils/testutils.go (1 hunks)
  • pkg/tmplexec/flow/flow_executor.go (4 hunks)
  • pkg/tmplexec/flow/flow_internal.go (2 hunks)
💤 Files with no reviewable changes (1)
  • pkg/protocols/http/build_request.go
🧰 Additional context used
🧬 Code Graph Analysis (4)
pkg/tmplexec/flow/flow_executor.go (1)
pkg/progress/progress.go (1)
  • Progress (17-35)
pkg/protocols/http/request_generator.go (2)
pkg/fuzz/dataformat/raw.go (1)
  • Raw (3-3)
pkg/protocols/http/request_annotations.go (1)
  • Once (46-46)
pkg/protocols/common/hosterrorscache/hosterrorscache.go (1)
pkg/protocols/common/contextargs/metainput.go (1)
  • MetaInput (18-30)
pkg/protocols/http/request.go (1)
pkg/progress/progress.go (1)
  • Progress (17-35)
🪛 GitHub Check: Lint
pkg/tmplexec/flow/flow_internal.go

[failure] 79-79:
Error return value of f.executed.Set is not checked (errcheck)

🔇 Additional comments (20)
pkg/progress/progress.go (1)

122-124: Improved request counting behavior

The implementation of SetRequests now directly increments the counter by the provided count value, which simplifies the logic and improves consistency with other progress tracking components in the codebase.

pkg/output/output_stats.go (1)

52-54: Implementation of ResultCount interface method

This correctly implements the Writer interface by adding the ResultCount method to StatsOutputWriter. Returning 0 is appropriate since this writer doesn't store actual results but only tracks statistics.

pkg/testutils/testutils.go (1)

136-138: Mock implementation of ResultCount

This properly implements the ResultCount method for testing purposes, maintaining consistency with the interface changes in the output package.

pkg/output/multi_writer.go (1)

69-77: Implement result counting for MultiWriter

The implementation correctly aggregates result counts from all writers, returning the first positive count found. This supports the request tracking system being implemented across the codebase.

pkg/tmplexec/flow/flow_internal.go (1)

91-96: Well-structured helper function for request identification

The requestKey function properly handles request identification by combining protocol name and request ID. This supports the new functionality for tracking executed requests.

internal/runner/runner.go (2)

705-705: Good practice: Capture start time for performance tracking

Capturing the start time right before execution allows for accurate time tracking of the scan duration.


729-738: Improved scan completion reporting

The code now:

  1. Explicitly stops the progress bar before printing the final message
  2. Calculates and displays the scan duration in minutes
  3. Shows the number of matches found when results exist

This provides better feedback to users about the scan results.

pkg/protocols/http/http.go (1)

519-521: Simplified request counting logic

The code now uses the generator's Total() method directly to get the count of requests, instead of complex conditional logic. This makes the code more maintainable and ensures consistent request counting across the codebase.

pkg/output/output.go (4)

57-58: Interface extension looks good!

Adding the ResultCount() method to the Writer interface is a clean way to expose result counting functionality to all implementors.


85-85: Good use of atomic for thread safety

Using atomic.Int32 is the right choice for thread-safe counter operations in a concurrent environment.


294-296: Method implementation looks correct

The implementation properly uses atomic loading to retrieve the counter value in a thread-safe manner.


347-347: Atomic increment in the right place

The counter is incremented after successful writing operations, ensuring accurate counting of actual written results.

pkg/tmplexec/flow/flow_executor.go (3)

55-55: Good addition for tracking executed requests

Using a SyncLockMap with an empty struct value is a memory-efficient way to track executed requests in a concurrent environment.


103-103: Proper initialization in constructor

Initializing the executed map in the constructor ensures it's never nil when accessed later.


249-249: Progress reconciliation at the right place

Calling reconcileProgress() after program execution but before error handling ensures accurate progress reporting regardless of execution outcome.

pkg/protocols/common/hosterrorscache/hosterrorscache.go (2)

92-94: Performance optimization for host:port format

Early return for strings containing a colon avoids unnecessary URL parsing for values that likely represent host:port formats, reducing processing overhead.


242-254: Improved address priority logic

The refactored code now correctly prioritizes the error-specific address before falling back to the context's meta input address. This ensures that errors are tracked with the most accurate address information, leading to more precise error handling, especially for templates with errors.

pkg/protocols/http/request_generator.go (2)

139-173: Well-implemented Remaining() method

The method accurately calculates remaining requests by accounting for the current position, request markers, and remaining payload iterations. The logic correctly handles special cases like @once marked requests.


175-201: Total() method handles edge cases properly

The implementation correctly calculates the total number of requests, properly accounting for requests with the @once marker that should only be executed once regardless of payload iterations.

pkg/protocols/http/request.go (1)

546-549: Better error wrapping using errkit instead of errorutil.

Replaced the previous error wrapping method with errkit.FromError, which provides improved error handling capabilities, better structure, and more detailed context about the error. The formatted error message now includes the specific URL that caused the issue.

@Ice3man543 Ice3man543 changed the title feat: fixed max-host-error blocking wrong port for template with error feat: fixed max-host-error blocking + progress mismatch + misc Apr 29, 2025
Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm !

@ehsandeep ehsandeep merged commit 4801cc6 into dev May 7, 2025
23 of 24 checks passed
@ehsandeep ehsandeep deleted the fix-maxhosterror-issue-port branch May 7, 2025 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] nuclei command Unexpected exit [BUG] CVE-2019-19781 template not executed in broad scan despite being listed as loaded (v3.4.1+)

3 participants