Skip to content

fix(expressions): prefer exact placeholders over exprs#7397

Open
dwisiswant0 wants to merge 2 commits into
devfrom
dwisiswant0/fix/expressions/prefer-exact-placeholders-over-exprs
Open

fix(expressions): prefer exact placeholders over exprs#7397
dwisiswant0 wants to merge 2 commits into
devfrom
dwisiswant0/fix/expressions/prefer-exact-placeholders-over-exprs

Conversation

@dwisiswant0
Copy link
Copy Markdown
Member

@dwisiswant0 dwisiswant0 commented May 11, 2026

Proposed changes

FindExpressions checks marker contents with
govaluate before doing normal placeholder
replacement. That breaks valid hyphenated names
like request-id: govaluate reads them as
subtraction lol, and the lookup fails with a
missing request parameter.

If a marker exactly matches a key in the
replacement map, treat it as a plain placeholder
and let normal replacement handle it. Authored
expressions should still be evaluated, and
malformed helper calls should still return errors.

Fixes #7395

Proof

patch:

$ go test -v -run "TestEvaluateHyphenatedPlaceholderNames" ./pkg/protocols/common/expressions/
=== RUN   TestEvaluateHyphenatedPlaceholderNames
--- PASS: TestEvaluateHyphenatedPlaceholderNames (0.00s)
PASS
ok  	github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/expressions	0.065s

dev:

$ git cherry-pick 4f3e77f7
[dev 9e9a4cf3] test(expressions): add TestEvaluateHyphenatedPlaceholderNames
 Date: Tue May 12 04:45:41 2026 +0700
 1 file changed, 8 insertions(+)
$ go test -v -run "TestEvaluateHyphenatedPlaceholderNames" ./pkg/protocols/common/expressions/
=== RUN   TestEvaluateHyphenatedPlaceholderNames
    expressions_test.go:129: 
        	Error Trace:	/home/dw1/Development/PD/nuclei/pkg/protocols/common/expressions/expressions_test.go:129
        	Error:      	Received unexpected error:
        	            	failed to evaluate expression "foo-bar": No parameter 'foo' found.
        	Test:       	TestEvaluateHyphenatedPlaceholderNames
--- FAIL: TestEvaluateHyphenatedPlaceholderNames (0.00s)
FAIL
FAIL	github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/expressions	0.298s
FAIL

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved expression detection logic to correctly handle string literals in template evaluation
  • New Features

    • Template placeholders with hyphenated names (e.g., {{foo-bar}}) are now properly resolved from variable mappings

Review Change Stack

Signed-off-by: Dwi Siswanto <git@dw1.io>
`FindExpressions` checks marker contents with
govaluate before doing normal placeholder
replacement. That breaks valid hyphenated names
like `request-id`: govaluate reads them as
subtraction lol, and the lookup fails with a
missing request parameter.

If a marker exactly matches a key in the
replacement map, treat it as a plain placeholder
and let normal replacement handle it. Authored
expressions should still be evaluated, and
malformed helper calls should still return errors.

Fixes #7395

Signed-off-by: Dwi Siswanto <git@dw1.io>
@auto-assign auto-assign Bot requested a review from dogancanbakir May 11, 2026 21:48
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 73c05286-af62-4eca-bda7-469fbbcfcc02

📥 Commits

Reviewing files that changed from the base of the PR and between d65c795 and b4a9e0a.

📒 Files selected for processing (2)
  • pkg/protocols/common/expressions/expressions.go
  • pkg/protocols/common/expressions/expressions_test.go

Walkthrough

This PR fixes template variable resolution for names containing hyphens. The isExpression function now returns false when input exactly matches a key in the base map, preventing govaluate from misinterpreting hyphenated names like request-id as subtraction operations. A test validates this behavior.

Changes

Hyphenated Template Variable Resolution

Layer / File(s) Summary
Expression Guard for Exact Keys
pkg/protocols/common/expressions/expressions.go
isExpression adds an early guard: if data is an exact key in base, return false before attempting govaluate compilation or helper-function name detection.
Hyphenated Placeholder Test
pkg/protocols/common/expressions/expressions_test.go
New test TestEvaluateHyphenatedPlaceholderNames verifies Evaluate resolves hyphenated placeholders like {{foo-bar}} using the matching key from the variables map without evaluation errors.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A hyphen-named variable found its way,
Once mistaken for math in govaluate's sway,
Now exact keys are caught with a guard so keen,
The clearest path taken: don't evaluate the scene,
Request-id and friends now flow as they should.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: preferring exact placeholder matches over expression evaluation for hyphenated names.
Linked Issues check ✅ Passed The PR directly addresses issue #7395 by treating exact placeholder matches as non-expressions before govaluate compilation, preventing hyphenated names from being misparsed as subtraction operations.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the hyphenated placeholder issue: modifying isExpression logic and adding a validating test for the fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dwisiswant0/fix/expressions/prefer-exact-placeholders-over-exprs

Comment @coderabbitai help to get the list of available commands and usage tips.

@neo-by-projectdiscovery-dev
Copy link
Copy Markdown

Neo — Command Did Not Complete

Warning

Your @pdneo command did not complete due to a transient error.

To retry: Comment your command again.

Comment @pdneo help for available commands.

@neo-by-projectdiscovery-dev
Copy link
Copy Markdown

neo-by-projectdiscovery-dev Bot commented May 11, 2026

Neo - PR Security Review

No security issues found

Comment @pdneo help for available commands. · Open in Neo

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.

[BUG] Template variables with hyphens (e.g. request-id) fail with "No parameter 'request' found."

1 participant