Conversation
- Introduce `MarkDownOutput` flag and CLI option (`-markdown` / `-md`). - Implement `Result.MarkdownOutput` to generate a formatted Markdown table with optional title and CDN columns, and a response body preview. - Add markdown escaping helper. - Update runner to create `.md` output file, handle markdown generation per result, and write to file or stdout. - Adjust option parsing and related logic to include markdown handling.
WalkthroughAdds Markdown table output: new Markdown header/row renderers in Changes
Sequence Diagram(s)sequenceDiagram
participant Runner
participant Result as "Result (reflection)"
participant FS as "MD File\n(filesystem)"
participant Stdout
Runner->>FS: Open Output + ".md" (if MarkDownOutput)
Note right of FS: file handle opened\nmarkdownHeaderWritten=false
alt header not written
Runner->>Result: MarkdownHeader()
Result-->>Runner: header string
Runner->>FS: Write header
Runner->>Stdout: Optionally print header
Note right of FS: markdownHeaderWritten=true
end
loop per response
Runner->>Result: MarkdownRow(scanopts)
Result-->>Runner: row string
Runner->>FS: Append row
Runner->>Stdout: Optionally print row
end
Runner->>FS: Close file (deferred)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (2)
runner/md_output.go (1)
61-67: Consider additional Markdown escaping for edge cases.The current implementation escapes pipes and newlines, which covers the primary cases for table cell content. However, consider whether these additional characters should be escaped:
- Backticks
`(could break inline code blocks if used in title)- Square brackets
[and](could create unintended links)For the current use case (table cells), the existing escaping is likely sufficient, but be aware of potential edge cases.
runner/runner.go (1)
845-845: Consider renamingjsonOrCsvvariable for clarity.The variable now includes Markdown output but retains the name
jsonOrCsv, which is misleading. Consider renaming to something likestructuredOutputorjsonOrCsvOrMarkdownfor better maintainability.This is used on lines 847 and 1105 to control output behavior, so the rename would affect those locations as well.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
runner/md_output.gorunner/options.gorunner/runner.go
🔇 Additional comments (5)
runner/options.go (2)
225-225: LGTM!The
MarkDownOutputfield is properly positioned alongside other output format flags and follows existing conventions.
482-482: LGTM!The CLI flag binding is properly configured and follows the same pattern as other output format flags.
runner/runner.go (3)
818-818: LGTM!The
mdFilevariable declaration follows the existing pattern for other output file handles.
835-843: LGTM!The markdown file opening logic correctly follows the pattern established for JSON and CSV outputs, including proper resume support and cleanup.
1241-1249: Markdown output integration is correctly implemented.The markdown output logic properly integrates with the existing output flow and mirrors the JSON/CSV patterns. However, note the table structure issue flagged in
runner/md_output.go: each result generates its own complete table rather than contributing rows to a unified table.Unlike CSV (lines 889-897), which writes one header then rows for all results, the current markdown implementation writes a complete table (header + separator + row) per result. This is correctly implemented given the current design of
MarkdownOutput(), but results in multiple separate tables in the output file.Suggestion: If a unified table is desired (similar to CSV), consider writing a markdown table header once at the beginning (similar to CSV header on lines 889-897), then have
MarkdownOutput()return only the data row without header/separator.
- Split Markdown generation into `MarkdownHeader` and `MarkdownRow` for clearer separation. - Remove body‑preview block from Markdown rows. - Add guard in runner to write the header only once per run.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
runner/md_output.go (2)
9-31: Table structure still inconsistent: conditional columns break Markdown rendering.The header conditionally includes Title and CDN columns based on a single Result instance's field values. This creates mismatched tables when different results have different fields populated.
Problem scenario:
- First result has
Title=""→ header generated with 7 columns (no Title column)- Second result has
Title="Hello"→ row written with 8 cells (includes Title)- Result: broken table with mismatched column counts
The split into MarkdownHeader/MarkdownRow (addressing the past review comment) solves the repeated-header issue but not the column consistency problem.
To ensure consistent table structure, always include Title and CDN columns regardless of whether individual results have those values populated. Write empty cells when values are blank.
🔎 Proposed fix to always include Title and CDN columns
func MarkdownHeader(r Result) string { var b strings.Builder - b.WriteString("| URL | Status | Method | IP | Size | Words | Lines |") - if r.Title != "" { - b.WriteString(" Title |") - } - if r.CDNName != "" { - b.WriteString(" CDN |") - } + b.WriteString("| URL | Status | Method | IP | Size | Words | Lines | Title | CDN |") b.WriteString("\n") - b.WriteString("|---|---|---|---|---|---|---|") - if r.Title != "" { - b.WriteString("---|") - } - if r.CDNName != "" { - b.WriteString("---|") - } + b.WriteString("|---|---|---|---|---|---|---|---|---|") b.WriteString("\n") return b.String() }Then update MarkdownRow to always write Title and CDN cells (see separate comment).
33-54: Critical: URL field not escaped; conditional cells break table structure.Two issues in this function:
The URL field (line 37) is not escaped, which will break the table if it contains pipe characters (e.g.,
https://example.com?param=value|other). This was flagged in the past review comment and remains unresolved.Conditional Title and CDN cells (lines 45-50) create mismatched column counts when different results have different field values. Must always write these cells (empty if blank) to match the header's column structure.
🔎 Proposed fix for URL escaping and consistent cells
func (r Result) MarkdownRow(scanopts *ScanOptions) string { var b strings.Builder fmt.Fprintf(&b, "| %s | `%d %s` | `%s` | `%s` | %d | %d | %d |", - r.URL, + escapeMarkdown(r.URL), r.StatusCode, http.StatusText(r.StatusCode), r.Method, r.HostIP, r.ContentLength, r.Words, r.Lines) - if r.Title != "" { + if r.Title != "" { fmt.Fprintf(&b, " %s |", escapeMarkdown(r.Title)) + } else { + b.WriteString(" |") } - if r.CDNName != "" { + if r.CDNName != "" { fmt.Fprintf(&b, " `%s` |", r.CDNName) + } else { + b.WriteString(" |") } b.WriteString("\n") return b.String() }
🧹 Nitpick comments (1)
runner/md_output.go (1)
33-33: Consider removing or documenting the unusedscanoptsparameter.The
scanoptsparameter is declared but never used in the function body. If it's reserved for future functionality (e.g., conditionally formatting fields based on scan options), consider adding a comment explaining its purpose. Otherwise, remove it to keep the signature clean.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
runner/md_output.gorunner/runner.go
🚧 Files skipped from review as they are similar to previous changes (1)
- runner/runner.go
🧰 Additional context used
🧬 Code graph analysis (1)
runner/md_output.go (1)
runner/types.go (1)
Result(35-105)
🔇 Additional comments (1)
runner/md_output.go (1)
56-62: LGTM: Basic escaping logic is sound.The function correctly escapes pipe characters and replaces newlines, which are the primary concerns for Markdown table structure. The main issue is that it's not being applied to the URL field (covered in the previous comment).
…d` flag * Consolidate output‑type checks into a single `jsonOrCsvOrMD` variable. * Adjust file‑opening logic to add the “.md” suffix only when needed. * Update related conditionals to respect the new combined flag handling. * Ensure markdown output is correctly written without creating “.md.md” files.
|
@coderabbitai create an issue for this PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 53 @dogancanbakir, I've created issue #2349 to track this PR: The issue has been assigned to you and includes a backlink to PR #2344. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
runner/runner.go (2)
840-841: Flag logic correctly incorporates Markdown output.The boolean expressions properly extend the existing JSON/CSV flag logic to include Markdown. These flags correctly control file creation and stdout output behavior throughout the function.
Optional: Consider renaming for clarity
The variable names
jsonOrCsvOrMDandjsonAndCsvAndMDnow include Markdown but the names only mention JSON and CSV. For future maintainability, you might consider renaming to be more explicit:-jsonOrCsvOrMD := (r.options.JSONOutput || r.options.CSVOutput || r.options.MarkDownOutput) -jsonAndCsvAndMD := (r.options.JSONOutput && r.options.CSVOutput && r.options.MarkDownOutput) +anyStructuredOutput := (r.options.JSONOutput || r.options.CSVOutput || r.options.MarkDownOutput) +allStructuredOutputs := (r.options.JSONOutput && r.options.CSVOutput && r.options.MarkDownOutput)However, keeping the existing names minimizes the diff, which may be preferred.
1247-1267: Well-implemented Markdown output logic.The Markdown output block correctly:
- Uses the
markdownHeaderWrittenguard to emit the header exactly once per response stream- Writes rows for each response
- Respects the
OutputAllflag by suppressing stdout when multiple output formats are enabled- Safely handles the case where
mdFileis nil with proper nil checksThe implementation follows the established JSON/CSV patterns consistently, making it maintainable and predictable.
Optional: Error handling consistency
Lines 1254 and 1265 ignore write errors with
_, _ = mdFile.WriteString(...). While this is consistent with how JSON and CSV files are handled throughout the function (lines 1230, 1243), silent write failures could lead to incomplete output files.If you plan to improve error handling in the future, consider logging write errors at least:
if _, err := mdFile.WriteString(header); err != nil { gologger.Warning().Msgf("Failed to write Markdown header: %v", err) }However, since this matches existing patterns, addressing this would best be done as a separate refactor across all output formats.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
runner/md_output.gorunner/runner.go
🚧 Files skipped from review as they are similar to previous changes (1)
- runner/md_output.go
🔇 Additional comments (3)
runner/runner.go (3)
818-819: LGTM! Clean variable declarations for Markdown output.The
mdFilevariable andmarkdownHeaderWrittenguard follow the existing pattern for JSON/CSV outputs. The guard correctly ensures the Markdown table header is written only once per response stream, and since it's scoped to the output goroutine, there are no concurrency concerns.
834-837: Correct file creation for OutputAll mode.The Markdown file is properly created with a
.mdextension whenOutputAllis enabled, following the same pattern as JSON and CSV files. The deferred closure ensures proper cleanup.
871-880: Properly handles Markdown file creation.The file creation logic correctly mirrors the JSON and CSV patterns:
- Creates the file only when
MarkDownOutputis enabled and the file hasn't been created yet- Appropriately adds a
.mdextension when all three output formats are enabled- Includes proper cleanup with deferred closure
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
runner/md_output.go (2)
38-63: Consider removing or utilizing the unused scanopts parameter.The
scanopts *ScanOptionsparameter is declared but never referenced in the function body. If it's not needed for current or planned functionality, consider removing it for a cleaner API.
65-88: Consider more concise formatting for time.Time values.The
Timestampfield will hit the default case (line 84) and format using%v, producing verbose output like "2006-01-02 15:04:05.999999999 -0700 MST". For table readability, consider adding a specific case forreflect.Structthat checks fortime.Timeand formats it more concisely.🔎 Proposed enhancement for time formatting
func formatMarkdownValue(v reflect.Value) string { switch v.Kind() { case reflect.String: return escapeMarkdown(v.String()) case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: return fmt.Sprintf("%d", v.Int()) case reflect.Bool: return fmt.Sprintf("%t", v.Bool()) case reflect.Slice: if v.Len() == 0 { return "" } var items []string for i := 0; i < v.Len(); i++ { items = append(items, fmt.Sprintf("%v", v.Index(i).Interface())) } return escapeMarkdown(strings.Join(items, ", ")) + case reflect.Struct: + if t, ok := v.Interface().(time.Time); ok { + if t.IsZero() { + return "" + } + return t.Format("2006-01-02 15:04:05") + } + // Fall through to default for other structs + fallthrough default: if v.CanInterface() { return escapeMarkdown(fmt.Sprintf("%v", v.Interface())) } return "" } }Note: This requires importing
timeat the top of the file.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
runner/md_output.gorunner/types.go
🧰 Additional context used
🧬 Code graph analysis (2)
runner/md_output.go (1)
runner/types.go (1)
Result(35-105)
runner/types.go (4)
runner/headless.go (1)
NetworkRequest(21-27)common/httpx/csp.go (1)
CSPData(25-28)common/httpx/response.go (2)
Response(12-30)ChainItem(33-39)common/httpx/proto.go (1)
HTTP2(8-8)
🔇 Additional comments (3)
runner/types.go (1)
36-104: Consistent md tag additions enable reflection-based Markdown rendering.The md struct tags are consistently applied across all Result fields, correctly excluding internal/complex fields (Err, Raw, ResponseBody, maps, etc.) with
md:"-"while providing descriptive tags for user-facing fields. This aligns well with the reflection-based approach inmd_output.go.runner/md_output.go (2)
9-36: Clean reflection-based header generation.The function correctly uses reflection to build a consistent Markdown table header from md tags, addressing the previous variable-column-structure concern.
90-96: Escaping strategy is appropriate for Markdown table structure.The function correctly handles the critical characters for table integrity: pipes (column separators) and newlines (row breaks). While additional Markdown special characters could be escaped, they don't break table structure and the current approach is sufficient.
|
@Mzack9999 Yes! In past, I was using some bash glue code to convert the output into Markdown, then piping it into other tools to view the data. I figured it would be cleaner if the pd tools themselves supported this as an output format. I also pipe recon output into some LLM APIs for attack-surface mapping, and they already return Markdown, so it’s easier to merge everything in one place. |
|
merge conflict! |
|
hey @dogancanbakir am i supposed to resolve conflicts; or it was for the maintainers? |
|
@ayuxsec Yes, please. It would be great if you could resolve the merge conflict. Thanks! |
|
oh ok, i will have a look it when i get spare time |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@runner/runner.go`:
- Around line 956-958: The variable jsonAndCsvAndMD currently checks for all
three structured outputs being enabled which breaks behavior when exactly two
(e.g., JSON+CSV) are set; change the boolean logic to detect multiple structured
outputs instead: replace the definition jsonAndCsvAndMD := (r.options.JSONOutput
&& r.options.CSVOutput && r.options.MarkDownOutput) with something like
multipleStructuredOutputs that counts the enabled structured outputs
(JSONOutput, CSVOutput, MarkDownOutput) and returns true when the count > 1,
then replace every use of jsonAndCsvAndMD with multipleStructuredOutputs (i.e.,
every place referencing jsonAndCsvAndMD in runner.go) so filenames get
extensions when more than one structured output is selected.
- Around line 1363-1383: The Markdown stdout branch currently only checks
!r.options.OutputAll, causing Markdown to print to stdout when JSON/CSV suppress
output; update the Markdown output conditions in the runner loop (the block
using r.options.MarkDownOutput, markdownHeaderWritten, resp.MarkdownHeader(),
resp.MarkdownRow(&r.scanopts), and mdFile) to also check !jsonAndCsvAndMD before
writing to stdout (i.e., mirror the JSON/CSV checks) so Markdown respects the
combined jsonAndCsvAndMD suppression while still writing to mdFile when
appropriate.
| jsonOrCsvOrMD := (r.options.JSONOutput || r.options.CSVOutput || r.options.MarkDownOutput) | ||
| jsonAndCsvAndMD := (r.options.JSONOutput && r.options.CSVOutput && r.options.MarkDownOutput) | ||
| if r.options.Output != "" && plainFile == nil && !jsonOrCsvOrMD { |
There was a problem hiding this comment.
Critical logic bug: jsonAndCsvAndMD breaks existing JSON+CSV behavior.
The condition jsonAndCsvAndMD requires all three outputs (JSON, CSV, and Markdown) to be enabled simultaneously. This breaks the previous behavior where enabling just JSON and CSV would add .json/.csv extensions to avoid file overwrites.
Scenario: User runs httpx -json -csv -o output
- Before this change: files written to
output.jsonandoutput.csv - After this change: both try to write to
output(no extensions), causing overwrites
The correct approach is to check if more than one structured output format is enabled:
🐛 Proposed fix
jsonOrCsvOrMD := (r.options.JSONOutput || r.options.CSVOutput || r.options.MarkDownOutput)
- jsonAndCsvAndMD := (r.options.JSONOutput && r.options.CSVOutput && r.options.MarkDownOutput)
+ // Count how many structured outputs are enabled to determine if extensions are needed
+ structuredOutputCount := 0
+ if r.options.JSONOutput {
+ structuredOutputCount++
+ }
+ if r.options.CSVOutput {
+ structuredOutputCount++
+ }
+ if r.options.MarkDownOutput {
+ structuredOutputCount++
+ }
+ multipleStructuredOutputs := structuredOutputCount > 1Then replace all occurrences of jsonAndCsvAndMD with multipleStructuredOutputs at lines 967, 978, 989, 1012, 1229, 1340, and 1353.
🤖 Prompt for AI Agents
In `@runner/runner.go` around lines 956 - 958, The variable jsonAndCsvAndMD
currently checks for all three structured outputs being enabled which breaks
behavior when exactly two (e.g., JSON+CSV) are set; change the boolean logic to
detect multiple structured outputs instead: replace the definition
jsonAndCsvAndMD := (r.options.JSONOutput && r.options.CSVOutput &&
r.options.MarkDownOutput) with something like multipleStructuredOutputs that
counts the enabled structured outputs (JSONOutput, CSVOutput, MarkDownOutput)
and returns true when the count > 1, then replace every use of jsonAndCsvAndMD
with multipleStructuredOutputs (i.e., every place referencing jsonAndCsvAndMD in
runner.go) so filenames get extensions when more than one structured output is
selected.
| if r.options.MarkDownOutput || r.options.OutputAll { | ||
| if !markdownHeaderWritten { | ||
| header := resp.MarkdownHeader() | ||
| if !r.options.OutputAll { | ||
| gologger.Silent().Msgf("%s", header) | ||
| } | ||
| if mdFile != nil { | ||
| _, _ = mdFile.WriteString(header) | ||
| } | ||
| markdownHeaderWritten = true | ||
| } | ||
|
|
||
| row := resp.MarkdownRow(&r.scanopts) | ||
|
|
||
| if !r.options.OutputAll { | ||
| gologger.Silent().Msgf("%s", row) | ||
| } | ||
| if mdFile != nil { | ||
| _, _ = mdFile.WriteString(row) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full context around lines 1363-1383 in runner.go
cat -n runner/runner.go | sed -n '1330,1390p'Repository: projectdiscovery/httpx
Length of output: 1877
🏁 Script executed:
# Find where jsonAndCsvAndMD is defined and used
rg -n "jsonAndCsvAndMD" runner/runner.go | head -20Repository: projectdiscovery/httpx
Length of output: 512
🏁 Script executed:
# Search for the MarkdownHeader and MarkdownRow method implementations
rg -n "func.*MarkdownHeader|func.*MarkdownRow" --type goRepository: projectdiscovery/httpx
Length of output: 230
🏁 Script executed:
# Let's examine the MarkdownHeader and MarkdownRow implementations
cat -n runner/md_output.goRepository: projectdiscovery/httpx
Length of output: 2614
Markdown stdout should check !jsonAndCsvAndMD like JSON and CSV output.
The Markdown stdout conditions at lines 1366 and 1377 only check !r.options.OutputAll, while JSON (line 1340) and CSV (line 1353) additionally check !jsonAndCsvAndMD. This causes inconsistent behavior when all three structured outputs are enabled—Markdown prints to stdout while JSON/CSV suppress it.
🔧 Proposed fix for consistent stdout behavior
if r.options.MarkDownOutput || r.options.OutputAll {
if !markdownHeaderWritten {
header := resp.MarkdownHeader()
- if !r.options.OutputAll {
+ if !r.options.OutputAll && !jsonAndCsvAndMD {
gologger.Silent().Msgf("%s", header)
}
if mdFile != nil {
_, _ = mdFile.WriteString(header)
}
markdownHeaderWritten = true
}
row := resp.MarkdownRow(&r.scanopts)
- if !r.options.OutputAll {
+ if !r.options.OutputAll && !jsonAndCsvAndMD {
gologger.Silent().Msgf("%s", row)
}
if mdFile != nil {
_, _ = mdFile.WriteString(row)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if r.options.MarkDownOutput || r.options.OutputAll { | |
| if !markdownHeaderWritten { | |
| header := resp.MarkdownHeader() | |
| if !r.options.OutputAll { | |
| gologger.Silent().Msgf("%s", header) | |
| } | |
| if mdFile != nil { | |
| _, _ = mdFile.WriteString(header) | |
| } | |
| markdownHeaderWritten = true | |
| } | |
| row := resp.MarkdownRow(&r.scanopts) | |
| if !r.options.OutputAll { | |
| gologger.Silent().Msgf("%s", row) | |
| } | |
| if mdFile != nil { | |
| _, _ = mdFile.WriteString(row) | |
| } | |
| } | |
| if r.options.MarkDownOutput || r.options.OutputAll { | |
| if !markdownHeaderWritten { | |
| header := resp.MarkdownHeader() | |
| if !r.options.OutputAll && !jsonAndCsvAndMD { | |
| gologger.Silent().Msgf("%s", header) | |
| } | |
| if mdFile != nil { | |
| _, _ = mdFile.WriteString(header) | |
| } | |
| markdownHeaderWritten = true | |
| } | |
| row := resp.MarkdownRow(&r.scanopts) | |
| if !r.options.OutputAll && !jsonAndCsvAndMD { | |
| gologger.Silent().Msgf("%s", row) | |
| } | |
| if mdFile != nil { | |
| _, _ = mdFile.WriteString(row) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@runner/runner.go` around lines 1363 - 1383, The Markdown stdout branch
currently only checks !r.options.OutputAll, causing Markdown to print to stdout
when JSON/CSV suppress output; update the Markdown output conditions in the
runner loop (the block using r.options.MarkDownOutput, markdownHeaderWritten,
resp.MarkdownHeader(), resp.MarkdownRow(&r.scanopts), and mdFile) to also check
!jsonAndCsvAndMD before writing to stdout (i.e., mirror the JSON/CSV checks) so
Markdown respects the combined jsonAndCsvAndMD suppression while still writing
to mdFile when appropriate.
3 participants
MarkDownOutputflag and CLI option (-markdown/-md).Result.MarkdownOutputto generate a formatted Markdown table with optional title and CDN columns, and a response body preview..mdoutput file, handle markdown generation per result, and write to file or stdout.Preview:
Summary by CodeRabbit