fix: handle comma-separated entries in -l/-list and stdin input#959
fix: handle comma-separated entries in -l/-list and stdin input#959Gengyscan wants to merge 3 commits intoprojectdiscovery:mainfrom
Conversation
Split comma-separated targets when reading from -l file or stdin, matching the existing -u flag behavior that uses CommaSeparatedStringSliceOptions. Fixes projectdiscovery#859
Neo - PR Security ReviewNo security issues found Highlights
Comment |
WalkthroughInput parsing now splits each input line on commas, trims whitespace, and enqueues each non-empty segment as a separate input item. File and stdin read errors are wrapped with descriptive messages and returned. Public function signatures remain unchanged. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/runner/runner.go`:
- Around line 439-449: The scanner loops that read from file and stdin use
bufio.NewScanner(file) / bufio.NewScanner(os.Stdin) and call
r.processInputItem(...) for each token but never check scanner.Err(); after each
loop (the file-scanner loop around the call to r.processInputItem and the
stdin-scanner loop) call scanner.Err(), and if non-nil return or propagate that
error (or wrap it with context) instead of silently returning nil so token-size
or I/O errors are not swallowed; update the function to handle and surface these
errors where the scanner variables are defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96b26ef1-e5a1-4bde-8d26-2de2cbc0439b
📒 Files selected for processing (1)
internal/runner/runner.go
Address CodeRabbit review: check scanner.Err() after both scanner loops to surface token-size or I/O errors instead of silently dropping targets.
Adds 6 table-driven subtests covering: - comma-separated entries (basic split) - spaces around commas (trimming) - single entry passthrough (no comma) - trailing comma filtering (empty entries) - host:port with commas (custom port parsing) - multiple lines with commas (multi-line file) All tests exercise normalizeAndQueueInputs via temp file input.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/runner/runner_test.go (1)
409-413: Consider verifying custom port extraction in the "host with port" test.The test validates host extraction from
example.com:8443,scanme.sh:443but only checks thehostfield. SinceprocessInputItemuses custom ports instead of the default when present, verifying theportfield would strengthen coverage against regressions in port parsing.🔧 Optional enhancement to also verify ports
{ name: "host with port", content: "example.com:8443,scanme.sh:443", - expected: []string{"example.com", "scanme.sh"}, + expected: []string{"example.com:8443", "scanme.sh:443"}, },Then update the collection loop to include the port:
var hosts []string for task := range inputs { - hosts = append(hosts, task.host) + hosts = append(hosts, task.host+":"+task.port) }Alternatively, keep the current test focused on comma-splitting and add a separate subtest for port extraction verification.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/runner/runner_test.go` around lines 409 - 413, The "host with port" test only asserts the host names but should also verify that processInputItem correctly parses custom ports; update the test block named "host with port" (and the loop that collects results) to assert the parsed port values (e.g., expect 8443 for example.com and 443 for scanme.sh) against the produced items' port field, or add a focused subtest that calls processInputItem with "example.com:8443,scanme.sh:443" and asserts both host and port for each parsed item so port-parsing regressions are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/runner/runner_test.go`:
- Around line 409-413: The "host with port" test only asserts the host names but
should also verify that processInputItem correctly parses custom ports; update
the test block named "host with port" (and the loop that collects results) to
assert the parsed port values (e.g., expect 8443 for example.com and 443 for
scanme.sh) against the produced items' port field, or add a focused subtest that
calls processInputItem with "example.com:8443,scanme.sh:443" and asserts both
host and port for each parsed item so port-parsing regressions are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7fd80867-27ca-4da1-973c-2cab3e0adcb1
📒 Files selected for processing (1)
internal/runner/runner_test.go
|
Thank you for the contribution! This issue has been resolved by #953 which was merged with the same approach along with a dedicated test. Closing as superseded. |
2 participants
Description
Fixes #859 —
-l/-listand stdin input now correctly handle comma-separated targets on a single line, matching the existing-uflag behavior.Problem
When using
-u host1,host2,host3, targets are correctly split by comma viaCommaSeparatedStringSliceOptionsin goflags. However, when using-l file.txtwhere the file contains comma-separated targets on a single line, the entire line is treated as one target, causing connection failures.Root Cause
In
normalizeAndQueueInputs, the bufio scanner reads each line and passes it directly toprocessInputItemwithout splitting on commas. The-uflag gets comma-splitting for free from goflags, but file/stdin inputs bypass that.Fix
Split each scanned line on commas (with trimming) before passing to
processInputItem, in both the file-input and stdin-input code paths.Changes
internal/runner/runner.go: Addedstrings.Split(text, ",")loop withstrings.TrimSpacein both the-lfile scanner and the stdin scanner blocks ofnormalizeAndQueueInputs.Testing
go build -buildvcs=false ./cmd/tlsx/)stringswas already imported)/claim #859
Summary by CodeRabbit
New Features
Bug Fixes
Tests