Skip to content

Escape attribute values in attribution markup builder - #5607

Merged
obenland merged 2 commits into
mainfrom
escape-attribution-markup-attributes
Aug 27, 2026
Merged

Escape attribute values in attribution markup builder#5607
obenland merged 2 commits into
mainfrom
escape-attribution-markup-attributes

Conversation

@obenland

@obenland obenland commented Aug 27, 2026

Copy link
Copy Markdown
Member

Description

The attribution HTML builder's h() helper interpolated attribute values straight into the tag string, while only text-node content was passed through escapeHtml. This tightens the builder so attribute values get the same treatment:

  • Apply escapeHtml to attribute values in h(), consistent with how text nodes are already handled.
  • Validate the URL scheme in extLink() — only http(s)/mailto hrefs are emitted; anything else falls back to about:blank. Leading/embedded control characters and spaces are stripped before the scheme check to match how browsers parse URLs.

Keeps the rendered output identical for the normal case (URLs, src, style, class are unaffected) while making the generated markup robust against unexpected characters in provider-supplied metadata fields.

Testing Instructions

  • cd frontend && pnpm test:unit test/unit/specs/utils/attribution-html.spec.ts
  • The reuse/attribution panel on any media detail page renders unchanged.

Checklist

  • Unit tests added and passing.
  • Lint passing.

🤖 Generated with Claude Code

@obenland @claude
Escape attribute values in attribution markup builder
7f970be 
The `h()` helper interpolated attribute values directly into the tag
string while only text-node content was escaped. Apply the existing
`escapeHtml` to attribute values too, and validate the URL scheme in
`extLink` so only http(s)/mailto hrefs are emitted. Adds unit coverage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 27, 2026 19:39
@obenland
obenland requested a review from a team as a code owner August 27, 2026 19:39
@obenland
obenland requested review from dhruvkb and obulat August 27, 2026 19:39
Copilot AI reviewed Aug 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@openverse-bot openverse-bot added 🧱 stack: frontend Related to the Nuxt frontend 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Aug 27, 2026
@openverse-bot openverse-bot moved this to 👀 Needs Review in Openverse PRs Aug 27, 2026
@obenland @claude
Fix vacuous assertion and formatting in href sanitization tests
416560e 
The obfuscated-javascript: test asserted on the raw attribute text, where
the embedded tab means 'javascript' never appears — the assertion passed
even with sanitizeHref removed. Assert on the parsed href property, which
normalizes away the control characters, and run Prettier on the spec.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@obenland obenland added 🛠 goal: fix Bug fix 💻 aspect: code Concerns the software code in the repository 🟩 priority: low Low priority and doesn't need to be rushed 🐛 versioning: patch Patch version bump, associated with bug fix 🟨 tech: javascript Involves JavaScript labels Aug 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

Latest k6 run output1

     ✓ status was 200

     checks.........................: 100.00% ✓ 408      ✗ 0   
     data_received..................: 95 MB   394 kB/s
     data_sent......................: 130 kB  540 B/s
     http_req_blocked...............: avg=108.33µs min=2.26µs   med=5.46µs   max=3.16ms   p(90)=193.11µs p(95)=993.27µs
     http_req_connecting............: avg=46.99µs  min=0s       med=0s       max=2.32ms   p(90)=94.22µs  p(95)=171.62µs
     http_req_duration..............: avg=162.64ms min=17.77ms  med=105.03ms max=968.11ms p(90)=362.3ms  p(95)=461.15ms
       { expected_response:true }...: avg=162.64ms min=17.77ms  med=105.03ms max=968.11ms p(90)=362.3ms  p(95)=461.15ms
   ✓ http_req_failed................: 0.00%   ✓ 0        ✗ 408 
     http_req_receiving.............: avg=176.07µs min=69.29µs  med=162.65µs max=636.14µs p(90)=252.41µs p(95)=298.48µs
     http_req_sending...............: avg=31.46µs  min=6.85µs   med=25.15µs  max=1.12ms   p(90)=47.08µs  p(95)=65.58µs 
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=162.43ms min=17.64ms  med=104.87ms max=967.83ms p(90)=362.09ms p(95)=460.84ms
     http_reqs......................: 408     1.693727/s
     iteration_duration.............: avg=892.29ms min=252.43ms med=912.81ms max=1.8s     p(90)=1.19s    p(95)=1.26s   
     iterations.....................: 75      0.311347/s
     vus............................: 3       min=0      max=6 
     vus_max........................: 60      min=60     max=60

Footnotes

  1. This comment will automatically update with new output each time k6 runs for this PR ↩

@obenland
obenland merged commit bda5a56 into main Aug 27, 2026
68 of 71 checks passed
@obenland
obenland deleted the escape-attribution-markup-attributes branch August 27, 2026 20:18
@github-project-automation github-project-automation Bot moved this from 👀 Needs Review to 🤝 Merged in Openverse PRs Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@obulat obulat Awaiting requested review from obulat obulat is a code owner automatically assigned from WordPress/openverse-frontend
@dhruvkb dhruvkb Awaiting requested review from dhruvkb dhruvkb is a code owner automatically assigned from WordPress/openverse-frontend

Assignees

No one assigned

Labels

💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work 🟨 tech: javascript Involves JavaScript 🐛 versioning: patch Patch version bump, associated with bug fix

Projects

Openverse PRs
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@obenland @openverse-bot