Closed
Conversation
Previously, each template was parsed twice during validation: 1. First in `LoadTemplate()` via `Parser.ParseTemplate()`. 2. Then again in `templates.Parse()` for validation checks. This caused significant overhead when validating 10,000+ templates in nuclei-templates repo. Closes #6502. Signed-off-by: Dwi Siswanto <git@dw1.io>
Contributor
WalkthroughAdds optional parsed-template cache usage to loader validation: asserts a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant L as Loader.areWorkflowOrTemplatesValid
participant C as Config
participant P as templates.Parser
participant Cache as ParsedCache
participant Parse as templates.Parse
L->>C: get Parser
alt Parser implements templates.Parser
L->>P: access parsedCache
loop per templatePath
L->>Cache: Has(templatePath)
alt cache hit (parsed != nil)
Cache-->>L: return parsed template
note right of L: use cached parsed template
else cache miss or no cache
L->>Parse: Parse(templatePath)
Parse-->>L: parsed template or error
alt parse error
note right of L: mark invalid, continue
end
end
alt parsed template is nil
note right of L: continue to next path
else parsed template non-nil
L->>L: track duplicate IDs & validate workflows
end
end
else parser assertion fails
L-->>L: log error & return false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/catalog/loader/loader.go (1)
321-343: Potential nil-pointer dereference in LoadTemplatesOnlyMetadata.template may be nil; len(template.RequestsHeadless) dereferences it.
for templatePath := range validPaths { - template, _, _ := templatesCache.Has(templatePath) - - if len(template.RequestsHeadless) > 0 && !store.config.ExecutorOptions.Options.Headless { + template, _, _ := templatesCache.Has(templatePath) + if template == nil { + // e.g., not cached yet or global matchers stub; skip safely + continue + } + if len(template.RequestsHeadless) > 0 && !store.config.ExecutorOptions.Options.Headless { continue } - if len(template.RequestsCode) > 0 && !store.config.ExecutorOptions.Options.EnableCodeTemplates { + if len(template.RequestsCode) > 0 && !store.config.ExecutorOptions.Options.EnableCodeTemplates { continue } - if template.IsFuzzing() && !store.config.ExecutorOptions.Options.DAST { + if template.IsFuzzing() && !store.config.ExecutorOptions.Options.DAST { continue } - if template.SelfContained && !store.config.ExecutorOptions.Options.EnableSelfContainedTemplates { + if template.SelfContained && !store.config.ExecutorOptions.Options.EnableSelfContainedTemplates { continue } - if template.HasFileProtocol() && !store.config.ExecutorOptions.Options.EnableFileTemplates { + if template.HasFileProtocol() && !store.config.ExecutorOptions.Options.EnableFileTemplates { continue }
🧹 Nitpick comments (1)
pkg/catalog/loader/loader.go (1)
421-435: Make “global matchers” an explicit sentinel error (future refactor).Current continue-on-nil works; consider introducing ErrGlobalMatchersTemplate and using errors.Is for clarity and testability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/catalog/loader/loader.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code using go fmt
Run static analysis with go vet
Files:
pkg/catalog/loader/loader.go
🧠 Learnings (1)
📚 Learning: 2025-07-16T21:27:14.937Z
Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: To make the template caching mechanism in pkg/templates/compile.go production-ready, DSLs need to be updated to use runtime options instead of cached variables, rather than restoring the Compile() calls on each request.
Applied to files:
pkg/catalog/loader/loader.go
🧬 Code graph analysis (1)
pkg/catalog/loader/loader.go (4)
pkg/protocols/protocols.go (1)
ExecutorOptions(61-141)pkg/templates/parser.go (1)
Parser(21-30)pkg/templates/cache.go (1)
Cache(9-11)pkg/templates/compile.go (1)
Parse(52-230)
⏰ 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). (1)
- GitHub Check: Lint
Signed-off-by: Dwi Siswanto <git@dw1.io>
Member
Author
|
brb |
dwisiswant0
deleted the
dwisiswant0/perf/loader/eliminate-double-parsing
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Previously, each template was parsed twice during
validation:
LoadTemplate()viaParser.ParseTemplate().templates.Parse()for validation checks.This caused significant overhead when validating
10,000+ templates in nuclei-templates repo.
Proof
Note
./bin/nuclei-devbuilt against fix: suppress warn code flag not found & excludes known misc dir #6500../bin/nucleibuilt against this PR branch.Key Performance Metrics:
Checklist
Summary by CodeRabbit