GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [HtmlSanitizer] Accept percent-encoded line breaks and tabs in the query of hostless URLs by webdevsamran · Pull Request #65662 · symfony/symfony

Conversation

@webdevsamran

@webdevsamran webdevsamran commented Aug 27, 2026 •
edited by nicolas-grekas
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #65650
License MIT

Since the percent-decoded check for denied characters was added to UrlSanitizer::parse(), a mailto: URL with a multi-line body was rejected and the link lost its href silently. RFC 6068 requires line breaks in the body to be encoded as %0D%0A, so a URL that follows the scheme's own specification was dropped.

The check now ignores percent-encoded CR, LF and TAB in the query and fragment of hostless schemes (mailto, sms, tel, ...). Everything else is unchanged: those characters stay rejected in every other component, for every scheme with an authority (https://example.org/a?x=1%0A2 is still rejected), and the BiDi marks and Unicode whitespace stay rejected everywhere, encoded or not. The output keeps the percent-encoded form.

Before: <a href="mailto:[email protected]?body=send%20current-issue%0D%0Asend%20index">contact</a> became <a>contact</a>.
After: <a href="mailto:infobot&#64;example.com?body&#61;send%20current-issue%0D%0Asend%20index">contact</a>.

The \v escape in the denied-characters pattern is PCRE's vertical whitespace class, not the vertical tab; it is replaced by \x0B. This changes nothing, since every character \v matches is listed explicitly in the pattern.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@carsonbot carsonbot added Status: Needs Review Bug HtmlSanitizer labels Aug 27, 2026
@carsonbot carsonbot added this to the 8.2 milestone Aug 27, 2026
@webdevsamran @nicolas-grekas
[HtmlSanitizer] Accept percent-encoded line breaks and tabs in the qu…
8c904f6
…ery of hostless URLs

RFC 6068 requires line breaks in the body of a mailto URL to be encoded
as %0D%0A. Since the percent-decoded check for denied characters was
added, such URLs were rejected and the href silently dropped. The check
now ignores CR, LF and TAB in the query and fragment of hostless schemes
(mailto, sms, ...) and keeps rejecting them in every other component and
for every other scheme, together with the BiDi marks and the Unicode
whitespace.

The "\v" escape in the pattern is PCRE vertical whitespace, not the
vertical tab. It is replaced by "\x0B", which changes nothing since every
character "\v" matches is listed explicitly.
@nicolas-grekas nicolas-grekas modified the milestones: 8.2, 6.4 Aug 27, 2026
@nicolas-grekas
nicolas-grekas changed the base branch from 8.2 to 6.4 August 27, 2026 12:11
@nicolas-grekas
nicolas-grekas requested review from chalasr, dunglas, lyrixx, xabbuh and yceruto as code owners August 27, 2026 12:11
@nicolas-grekas
nicolas-grekas force-pushed the fix/issue-65650-htmlsanitizer-mailto-rfc6068 branch from bb242a6 to 8c904f6 Compare August 27, 2026 12:11
@nicolas-grekas nicolas-grekas removed the Status: Needs Review label Aug 27, 2026
@nicolas-grekas nicolas-grekas changed the title [HtmlSanitizer] Preserve RFC 6068 mailto URIs with percent-encoded CRLF body parameters [HtmlSanitizer] Accept percent-encoded line breaks and tabs in the query of hostless URLs Aug 27, 2026
@nicolas-grekas nicolas-grekas added the Status: Reviewed label Aug 27, 2026
@nicolas-grekas

nicolas-grekas commented Aug 27, 2026

Copy link
Copy Markdown
Member

Thank you for the fix. The scope is the right one (query and fragment of hostless schemes only), but the submitted patch did not change the outcome for the CRLF cases, which is why the two new test cases failed on CI: the new constant is a single-quoted PHP string, so PCRE receives \v and reads it as its vertical-whitespace class, which matches CR and LF as well. Only the tab case changed.

I reworked it and pushed to your branch, retargeted to 6.4 where the check was introduced:

  • one pattern kept, with the exception expressed in the loop (str_replace(["\r", "\n", "\t"], '', $decoded) for the query and fragment of hostless schemes), and a comment stating the rule and the RFC;
  • \v in DENIED_CHARS_PATTERN replaced by \x0B, with no behavior change, so the next copy of the pattern does not hit the same trap;
  • the provider now also asserts what must stay rejected: encoded CR/LF in the user, host or path of hostless URLs, https://...?x=1%0A2, BiDi marks and NBSP everywhere, raw CR/LF/TAB; plus one end-to-end case in HtmlSanitizerAllTest with the RFC example;
  • the commit message uses the component prefix instead of the conventional-commit prefix.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nicolas-grekas

nicolas-grekas commented Aug 27, 2026

Copy link
Copy Markdown
Member

Thank you @webdevsamran.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nicolas-grekas
nicolas-grekas merged commit 7ddd4df into symfony:6.4 Aug 27, 2026
12 of 14 checks passed

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Aug 30, 2026
Release v6.4.45 #65755
Merged
Release v7.4.18 #65757
Merged
Release v8.1.6 #65758
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lyrixx lyrixx Awaiting requested review from lyrixx
@yceruto yceruto Awaiting requested review from yceruto
@chalasr chalasr Awaiting requested review from chalasr
@dunglas dunglas Awaiting requested review from dunglas
@xabbuh xabbuh Awaiting requested review from xabbuh

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Bug HtmlSanitizer Status: Reviewed

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

[HtmlSanitizer] RFC 6068 multi-line mailto: URIs lose their href

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@webdevsamran @nicolas-grekas @carsonbot
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.