Skip to content

fix(unhead): parse HTML attribute values without backslash escaping#774

Merged
harlan-zw merged 1 commit into
mainfrom
fix/parser-attr-backslash
Jun 12, 2026
Merged

fix(unhead): parse HTML attribute values without backslash escaping#774
harlan-zw merged 1 commit into
mainfrom
fix/parser-attr-backslash

Conversation

@harlan-zw

@harlan-zw harlan-zw commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

🔗 Linked issue

No linked issue. Found during a security sweep of packages/unhead/src.

❓ Type of change

  • 📖 Documentation
  • 🐞 Bug fix
  • 👌 Enhancement
  • ✨ New feature
  • 🧹 Chore
  • ⚠️ Breaking change

📚 Description

parseAttributes (used by transformHtmlTemplate) treated \ as an escape character inside quoted attribute values. HTML has no backslash escaping there, so a trailing backslash swallowed the closing quote and diverged from both the browser and the tag-boundary scanner in the same file:

  • href="C:\" parsed href as C:\" (browser: C:\)
  • content="a\" name="description" parsed content as a\" name= plus a junk attribute (browser: content = a\, name = description)

Output is re-escaped on render, so this is a parsing-correctness issue, not an injection. Dropped the backslash branch in the QUOTED_VALUE state so the quote char always terminates the value. BACKSLASH_CHAR is still used by findClosingTag for quote-aware script/style scanning. Added test/unit/parser/parseAttributes.test.ts covering the trailing-backslash cases plus basic quoted, unquoted, and boolean attributes.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bf1ec539-9c88-4463-961e-82088259cb91

📥 Commits

Reviewing files that changed from the base of the PR and between d3cc3b1 and 735cd1b.

📒 Files selected for processing (2)
  • packages/unhead/src/parser/index.ts
  • packages/unhead/test/unit/parser/parseAttributes.test.ts
📝 Walkthrough

Walkthrough

The PR updates the HTML attribute parser to stop treating backslashes as escape characters in quoted values. A quote character now unconditionally terminates a quoted attribute value, matching browser behavior. A clarifying comment and three test cases validate the updated parsing logic for backslashes and mixed attribute syntax.

Changes

Attribute Parser Backslash Handling

Layer / File(s) Summary
Parser logic: remove backslash escape skipping
packages/unhead/src/parser/index.ts		 Within the QUOTED_VALUE state of parseAttributes, backslash-escape skipping is removed so the quote character unconditionally terminates the quoted value; a clarifying comment is added stating HTML attribute values do not support backslash escaping.
Test suite: backslash and mixed attribute parsing
packages/unhead/test/unit/parser/parseAttributes.test.ts		 Vitest unit tests cover three parsing scenarios: backslashes are treated as literal characters before closing quotes, backslashes do not prevent parsing of adjacent attributes, and mixed quoted, unquoted, and boolean-style attributes are correctly parsed into expected objects.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A backslash's dreams of escape disappear,
For in HTML quotes, they're just letters here.
No magic to skip the quote's final say—
The parser stands firm, lets the quote have its way. 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title precisely describes the main change: removing backslash escaping from HTML attribute value parsing to align with browser behavior.
Description check ✅ Passed The description provides problem statement, solution, and test coverage details. However, the 'Type of change' checklist section from the template is not completed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 fix/parser-attr-backslash

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 and usage tips.

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Bundle Size Analysis

Bundle Size Gzipped
Client (Minimal) 10.8 kB 4.4 kB
Server (Minimal) 10.7 kB 4.3 kB
Vue Client (Minimal) 11.8 kB 4.9 kB
Vue Server (Minimal) 11.7 kB 4.8 kB

@harlan-zw harlan-zw merged commit 88c9d6d into main Jun 12, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@harlan-zw