Skip to content

Conversation

@dogancanbakir
Copy link
Member

@dogancanbakir dogancanbakir commented Aug 29, 2025

closes #878

Summary by CodeRabbit

  • New Features
    • Support comma-separated host values with the --list/-l flag, alongside file or stdin input.
  • Bug Fixes
    • Enforce mutual exclusivity of stdin across domains, wordlist, and hosts flags with clearer error messages.
    • In stream mode, explicitly disallow hosts input and provide a descriptive error.
  • Documentation
    • Updated --list/-l flag description to include comma-separated input.

@dogancanbakir dogancanbakir self-assigned this Aug 29, 2025
@coderabbitai
Copy link

coderabbitai bot commented Aug 29, 2025

Walkthrough

Adds comma-separated input support for the -l/--list flag, centralizes hosts input parsing via preProcessArgument, and tightens stdin/stream-mode validation (stdin exclusivity across input flags; disallow hosts in stream mode).

Changes

Cohort / File(s) Summary
Flag description and validation updates
internal/runner/options.go
- Updates -l/--list help text to mention comma-separated input.
- Enforces stdin as mutually exclusive across domains, wordlist, and hosts flags; errors if stdin provided to more than one.
- In stream mode, explicitly disallows hosts input and reports dedicated errors for unsupported stdin/hosts.
Input preprocessing refactor
internal/runner/runner.go
- Replaces manual hosts loading in prepareInput with r.preProcessArgument(r.options.Hosts) to handle file, stdin, and inline (including comma-separated) uniformly.
- Propagates errors from preProcessArgument instead of custom messages; downstream flow unchanged.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant CLI as CLI Options
    participant Runner
    participant ArgProc as preProcessArgument

    User->>CLI: Provide -l/--list (file | stdin | comma-separated)
    CLI->>CLI: Validate stdin exclusivity across flags
    CLI-->>User: Error if multiple flags use stdin
    CLI->>CLI: If stream mode, disallow hosts
    CLI-->>User: Error if hosts in stream mode

    User->>Runner: Run with validated options
    Runner->>ArgProc: Resolve hosts input
    ArgProc-->>Runner: []hosts or error
    Runner-->>User: Proceed or report error
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective Addressed Explanation
Support comma-separated values for -l/--list (#878)

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
Expanded stdin mutual-exclusivity validation across domains/wordlist/hosts (internal/runner/options.go: lines not specified) Issue #878 only requires supporting comma-separated values for -l; cross-flag stdin exclusivity is not mentioned.
Disallowing hosts in stream mode with new explicit error (internal/runner/options.go: lines not specified) The linked issue does not reference stream mode behavior; restricting hosts in stream mode appears unrelated.

Poem

I thump my paws with timely cheer,
Lists now split by commas clear!
No tangled stdin here today,
Stream mode keeps the hosts at bay.
I nibble bytes with whiskered grace—
Inputs prepped, we’re off to race! 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 878_add_comma_support

🪧 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.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit 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:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @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

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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.

Copy link

@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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/runner/options.go (1)

286-295: Remove stream-mode check from the multi-stdin exclusivity block.

The inner if options.Stream { ... "argument stdin not supported in stream mode" } only triggers when two flags use stdin and is redundant/confusing since stream-mode constraints are enforced below. Keep this block focused on exclusivity across flags.

Apply this diff:

-	// stdin can be set only on one flag
-	if (argumentHasStdin(options.Domains) && argumentHasStdin(options.WordList)) ||
-		(argumentHasStdin(options.Domains) && argumentHasStdin(options.Hosts)) ||
-		(argumentHasStdin(options.WordList) && argumentHasStdin(options.Hosts)) {
-		if options.Stream {
-			gologger.Fatal().Msgf("argument stdin not supported in stream mode")
-		}
-		gologger.Fatal().Msgf("stdin can be set for one flag")
-	}
+	// stdin can be set only on one flag
+	if (argumentHasStdin(options.Domains) && argumentHasStdin(options.WordList)) ||
+		(argumentHasStdin(options.Domains) && argumentHasStdin(options.Hosts)) ||
+		(argumentHasStdin(options.WordList) && argumentHasStdin(options.Hosts)) {
+		gologger.Fatal().Msgf("stdin can be set for one flag")
+	}
🧹 Nitpick comments (2)
internal/runner/options.go (1)

303-305: Clarify the stream-mode error for -l with a usage hint.

Suggest making the message actionable so users know how to feed stdin in stream mode.

-			gologger.Fatal().Msgf("hosts not supported in stream mode")
+			gologger.Fatal().Msgf("hosts (-l) not supported in stream mode; pipe input instead (e.g., dnsx -stream < hosts.txt)")
internal/runner/runner.go (1)

256-259: Add empty-item guard and clearer empty-input error

  • In the if sc == nil block, distinguish Hosts == "" and return
    errors.New("no input provided: use -l <file|comma-separated values> or pipe via stdin") instead of the opaque "empty argument".
  • In the host loop, immediately skip blank entries after trimming:
    for item := range sc {
        item := normalize(item)
        if item == "" {
            continue
        }
        …
    }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cfdced5 and 8215aaa.

📒 Files selected for processing (2)
  • internal/runner/options.go (3 hunks)
  • internal/runner/runner.go (1 hunks)
⏰ 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: Analyze (go)
  • GitHub Check: Functional Test (windows-latest)
🔇 Additional comments (1)
internal/runner/options.go (1)

97-97: Help text update for -l looks good.

Accurately reflects comma-separated support and aligns with -d/-w descriptions.

}
} else {
return errors.New("hosts file or stdin not provided")
sc, err = r.preProcessArgument(r.options.Hosts)
Copy link
Member

Choose a reason for hiding this comment

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

This seems to break stdin support:

% echo scanme.sh | go run . -verbose

      _             __  __
   __| | _ __   ___ \ \/ /
  / _' || '_ \ / __| \  / 
 | (_| || | | |\__ \ /  \ 
  \__,_||_| |_||___//_/\_\

                projectdiscovery.io

[INF] Current dnsx version 1.2.2 (latest)
%

@Mzack9999 Mzack9999 self-requested a review September 12, 2025 10:28
Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

breaks stdin import + failing tests

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.

-l from comma separated

3 participants