-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Javascript Multi-Port Support #6501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughAdds multi-port support to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant JSHandler as js.go
participant PortParser as getPorts()
participant PerPortExec as executeWithResults(port)
Client->>JSHandler: Request (may include Port header / target URL)
JSHandler->>PortParser: Parse & dedupe ports
PortParser-->>JSHandler: Unique ports[]
alt ports found
loop for each port
JSHandler->>PerPortExec: executeWithResults(port, ...)
PerPortExec-->>JSHandler: per-port result / error
end
JSHandler-->>Client: aggregated per-port results/errors
else no ports (fallback)
JSHandler->>PerPortExec: executeWithResults(single-port flow)
PerPortExec-->>JSHandler: result / error
JSHandler-->>Client: result/error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
pkg/protocols/**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)pkg/protocols/javascript/js.go (4)
⏰ 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). (3)
🔇 Additional comments (2)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
cmd/nuclei/ssh.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
pkg/protocols/javascript/js.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code using go fmt
Run static analysis with go vet
Files:
pkg/protocols/javascript/js.go
pkg/protocols/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Each protocol implementation must provide a Request interface with methods Compile(), ExecuteWithResults(), Match(), and Extract()
Files:
pkg/protocols/javascript/js.go
🧠 Learnings (1)
📚 Learning: 2025-09-10T18:57:51.327Z
Learnt from: Mzack9999
PR: projectdiscovery/nuclei#6465
File: pkg/catalog/loader/loader.go:504-505
Timestamp: 2025-09-10T18:57:51.327Z
Learning: The mapsutil.SyncLockMap from projectdiscovery/utils provides thread-safe operations including Has() and Set() methods that can be used safely in concurrent goroutines without additional synchronization. The separate Has() and Set() calls are internally synchronized by the SyncLockMap implementation.
Applied to files:
pkg/protocols/javascript/js.go
🧬 Code graph analysis (1)
pkg/protocols/javascript/js.go (3)
pkg/protocols/common/contextargs/metainput.go (1)
MetaInput(18-30)pkg/protocols/protocols.go (2)
Request(308-333)OutputEventCallback(336-336)pkg/protocols/common/contextargs/contextargs.go (1)
Context(22-33)
🪛 GitHub Actions: 🔨 Tests
pkg/protocols/javascript/js.go
[error] 858-858: getExcludePorts is unused (unused)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/protocols/javascript/js.go(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code using go fmt
Run static analysis with go vet
Files:
pkg/protocols/javascript/js.go
pkg/protocols/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Each protocol implementation must provide a Request interface with methods Compile(), ExecuteWithResults(), Match(), and Extract()
Files:
pkg/protocols/javascript/js.go
🧬 Code graph analysis (1)
pkg/protocols/javascript/js.go (2)
pkg/protocols/network/network.go (1)
Request(20-104)pkg/protocols/protocols.go (2)
Request(308-333)OutputEventCallback(336-336)
🔇 Additional comments (2)
pkg/protocols/javascript/js.go (2)
39-39: LGTM!The
sliceutilimport is appropriately added to support port deduplication at line 780.
137-142: LGTM!The validation correctly iterates over all ports to ensure none contain DSL expressions, maintaining the security constraint.
| // Get default port(s) if specified in template | ||
| ports := request.getPorts() | ||
|
|
||
| for _, port := range ports { | ||
| err := request.executeWithResults(port, target, dynamicValues, previous, callback) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Templates without Port won't execute.
When getPorts() returns an empty slice (no Port specified in template args), the loop doesn't execute and the function returns immediately. This breaks existing JavaScript templates that don't specify a Port argument.
Apply this diff to execute once with an empty port when no ports are specified:
func (request *Request) ExecuteWithResults(target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error {
// Get default port(s) if specified in template
ports := request.getPorts()
+ if len(ports) == 0 {
+ ports = []string{""}
+ }
for _, port := range ports {
err := request.executeWithResults(port, target, dynamicValues, previous, callback)
if err != nil {
return err
}
}
return nil
}📝 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.
| // Get default port(s) if specified in template | |
| ports := request.getPorts() | |
| for _, port := range ports { | |
| err := request.executeWithResults(port, target, dynamicValues, previous, callback) | |
| if err != nil { | |
| return err | |
| } | |
| } | |
| return nil | |
| } | |
| func (request *Request) ExecuteWithResults(target *contextargs.Context, dynamicValues, previous output.InternalEvent, callback protocols.OutputEventCallback) error { | |
| // Get default port(s) if specified in template | |
| ports := request.getPorts() | |
| if len(ports) == 0 { | |
| ports = []string{""} | |
| } | |
| for _, port := range ports { | |
| err := request.executeWithResults(port, target, dynamicValues, previous, callback) | |
| if err != nil { | |
| return err | |
| } | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In pkg/protocols/javascript/js.go around lines 288 to 299, the current logic
returns immediately when request.getPorts() yields an empty slice, which
prevents templates without a Port arg from running; change the flow so that if
ports is empty you call request.executeWithResults once with an empty-string (or
zero-value) port (preserving the same target, dynamicValues, previous, callback)
and return any error; otherwise keep iterating over ports and return any error
from each call. Ensure you do not alter error handling semantics and that a
single execution occurs when no ports are specified.
| func (request *Request) getPorts() []string { | ||
| for k, v := range request.Args { | ||
| if strings.EqualFold(k, "Port") { | ||
| return types.ToString(v) | ||
| ports := types.ToStringSlice(strings.Split(types.ToString(v), ",")) | ||
| return sliceutil.Dedupe(ports) | ||
| } | ||
| } | ||
| return "" | ||
| return []string{} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplify port parsing and add whitespace trimming.
Two issues with the current implementation:
types.ToStringSliceis redundant sincestrings.Splitalready returns[]string- Port values aren't trimmed, so
"22, 80, 443"would parse as["22", " 80", " 443"]with leading spaces causing invalid ports
Apply this diff to fix both issues:
func (request *Request) getPorts() []string {
for k, v := range request.Args {
if strings.EqualFold(k, "Port") {
- ports := types.ToStringSlice(strings.Split(types.ToString(v), ","))
- return sliceutil.Dedupe(ports)
+ portStr := types.ToString(v)
+ ports := []string{}
+ for _, p := range strings.Split(portStr, ",") {
+ trimmed := strings.TrimSpace(p)
+ if trimmed != "" {
+ ports = append(ports, trimmed)
+ }
+ }
+ return sliceutil.Dedupe(ports)
}
}
return []string{}
}📝 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.
| func (request *Request) getPorts() []string { | |
| for k, v := range request.Args { | |
| if strings.EqualFold(k, "Port") { | |
| return types.ToString(v) | |
| ports := types.ToStringSlice(strings.Split(types.ToString(v), ",")) | |
| return sliceutil.Dedupe(ports) | |
| } | |
| } | |
| return "" | |
| return []string{} | |
| } | |
| func (request *Request) getPorts() []string { | |
| for k, v := range request.Args { | |
| if strings.EqualFold(k, "Port") { | |
| portStr := types.ToString(v) | |
| ports := []string{} | |
| for _, p := range strings.Split(portStr, ",") { | |
| trimmed := strings.TrimSpace(p) | |
| if trimmed != "" { | |
| ports = append(ports, trimmed) | |
| } | |
| } | |
| return sliceutil.Dedupe(ports) | |
| } | |
| } | |
| return []string{} | |
| } |
🤖 Prompt for AI Agents
In pkg/protocols/javascript/js.go around lines 776 to 784, simplify port parsing
by removing the redundant types.ToStringSlice call and trim whitespace for each
split port; when you find the "Port" arg, call strings.Split(types.ToString(v),
",") to get []string, iterate over the slice trimming each element with
strings.TrimSpace, skip empty entries, collect cleaned ports into a []string,
then return sliceutil.Dedupe(cleanedPorts); keep the existing strings.EqualFold
key check and return an empty slice if no Port arg is found.
|
@CodeRabbit create an issue for this PR. It should be about adding the capability to support multi default ports in javascript templates |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 54 @Mzack9999 I've created issue #6513 to track the capability for supporting multiple default ports in JavaScript templates. Issue: #6513 The issue includes context about this PR, backlinks, and has been assigned to you. |
Proposed changes
Added support for multiple ports in the Nuclei engine for JavaScript templates
Checklist
./nuclei -u 127.0.0.1 -t ssh.yaml
Sample Template
Summary by CodeRabbit
New Features
Behavior