Skip to content

fix: prevent reparsing escape() results - #4486

Merged
matthew-dean merged 1 commit into
less:masterfrom
snowyukitty:fix/escape-literal-interpolation
Aug 13, 2026
Merged

fix: prevent reparsing escape() results#4486
matthew-dean merged 1 commit into
less:masterfrom
snowyukitty:fix/escape-literal-interpolation

Conversation

@snowyukitty

@snowyukitty snowyukitty commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What:

  • Mark escape() results that contain percent escapes as literal CSS so lazy
    declaration lookup does not parse their encoded output as Less source.
  • Preserve that marker when Anonymous nodes are evaluated.
  • Add regression coverage for direct unquoted output and literal SVG data URL
    interpolation.

Why:

When an escape() result is stored in a variable, Ruleset.parseValue()
treats the evaluated Anonymous value as unparsed source. Percent escapes such
as %3C are then lexed as Less and raise Invalid % without number.

Keeping the existing Anonymous node and carrying a no-reparse marker only
when encoding emits % fixes affected variable references while leaving
unencoded values on the existing lazy-parse path. This preserves
escape()'s observable node type, spacing, and url() rootpath behavior.

Fixes #3314.

Checklist:

  • Documentation — N/A
  • Added/updated unit tests
  • Code complete

Validation:

  • pnpm test — 208 runs passed
  • pnpm --filter less typecheck
  • pnpm --filter less exec eslint --no-ignore lib/less/functions/string.js lib/less/tree/anonymous.js lib/less/tree/ruleset.js

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of escaped strings so percent-encoded characters remain literal CSS values.
    • Fixed escaped SVG data URI processing to prevent unintended reparsing or alteration.
    • Preserved escaped values correctly when expressions are evaluated and reused.
  • Tests

    • Added coverage for direct escaping and escaped SVG data URI literals.

@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 41c6ddb7-cfdb-4fb9-b11b-2e3d528914f5

📥 Commits

Reviewing files that changed from the base of the PR and between 89c33e0 and 73cd5c1.

📒 Files selected for processing (5)
  • packages/less/lib/less/functions/string.js
  • packages/less/lib/less/tree/anonymous.js
  • packages/less/lib/less/tree/ruleset.js
  • packages/test-data/tests-unit/functions/functions.css
  • packages/test-data/tests-unit/functions/functions.less

📝 Walkthrough

Walkthrough

The escape function now marks percent-encoded Anonymous values, preserves that marker during evaluation, and prevents ruleset reparsing. Unit fixtures cover direct escaped values and escaped SVG data URIs.

Changes

Escaped value preservation

Layer / File(s) Summary
Escape and reparse flow
packages/less/lib/less/functions/string.js, packages/less/lib/less/tree/anonymous.js, packages/less/lib/less/tree/ruleset.js, packages/test-data/tests-unit/functions/*
The e function stores encoded output in an Anonymous node, marks values containing %, preserves the marker during evaluation, and skips reparsing marked declarations. Direct and SVG data URI fixtures exercise the updated behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant StringFunctions
  participant Anonymous
  participant Ruleset
  participant FunctionFixtures
  StringFunctions->>Anonymous: create percent-encoded escaped value
  Anonymous->>Anonymous: preserve _preventReparse during eval
  Anonymous->>Ruleset: provide marked declaration value
  Ruleset->>Ruleset: skip reparsing marked value
  FunctionFixtures->>Ruleset: validate direct and SVG escaped declarations
Loading

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: preventing reparsing of escape() results.
Linked Issues check ✅ Passed The changes implement the issue by marking escaped values to skip reparsing and add regression tests for nested escaped variables.
Out of Scope Changes check ✅ Passed All code and test changes are directly related to the escape() reparsing fix and its regressions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

Prevents percent-escaped escape() results from being reparsed as Less source.

  • Marks encoded Anonymous values as literal CSS when their output contains %.
  • Preserves the marker when Anonymous nodes are evaluated.
  • Skips lazy declaration reparsing for marked values.
  • Adds regression coverage for direct escaped output and SVG data URL interpolation.

Confidence Score: 5/5

The PR appears safe to merge, with the marker scoped to percent-containing escape() results and preserved through the relevant evaluation path.

The changed declaration path leaves marked escape results as literal Anonymous values, while unmarked anonymous declarations retain the existing lazy-parsing behavior; no actionable failure was identified.

Reviews (1): Last reviewed commit: "fix: prevent reparsing escape() results" | Re-trigger Greptile

@matthew-dean

Copy link
Copy Markdown
Member

Seems right!

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 13, 2026
@matthew-dean
matthew-dean merged commit 4eb6d3b into less:master Aug 13, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Escape function with nested var failed

2 participants