Skip to content

Formatting: Keep URLs whose colon follows a path, query or fragment in KSES. - #14320

Open
dhrupo wants to merge 1 commit into
WordPress:trunkfrom
dhrupo:41304-kses-colon-after-path
Open

Formatting: Keep URLs whose colon follows a path, query or fragment in KSES.#14320
dhrupo wants to merge 1 commit into
WordPress:trunkfrom
dhrupo:41304-kses-colon-after-path

Conversation

@dhrupo

@dhrupo dhrupo commented Oct 16, 2026

Copy link
Copy Markdown

Trac ticket: https://core.trac.wordpress.org/ticket/41304

Summary

wp_kses_bad_protocol_once() treats everything before the first colon of a URL as its protocol. The only exception is when that part contains the literal two-character sequence /?.

Browsers end a scheme at the first /, ? or # (a scheme may only contain letters, digits, +, - and .), so a colon after any of those characters can never separate a protocol. KSES still strips the text before it:

Input Before After
#fn:1 (Markdown footnote) 1 #fn:1
/page#section:2 2 /page#section:2
images?q=tbn:abc (the example in the ticket) abc images?q=tbn:abc
#:~:text=highlight (text fragment, #60347) text=highlight #:~:text=highlight
//example.org:8080/ 8080/ //example.org:8080/

As a result, users without unfiltered_html lose these links when they save a post, and esc_url() returns '' for them.

Reproduction

Log in as an Author, and in the code editor save a post containing:

<p>A footnote<a href="#fn:1">1</a> and an <a href="/images?q=tbn:abc">image search</a>.</p>

Reload the editor. The saved content is <a href="1"> and <a href="abc">. Before and after screenshots of this run are attached on the Trac ticket.

Fix

One line in wp_kses_bad_protocol_once(). The '%/\?%' check becomes '%[/?]|(?<!&)(?<!&amp;)#%': a colon is not a protocol separator when a /, ? or # comes before it.

  • A # directly after & or &amp; is not counted, because it may belong to a character reference (&#106;avascript: stays stripped, and so does the existing &amp;#14; javascript: test case).
  • The new check is a superset of the old one, so every URL that is kept today is still kept.

Behaviour changes worth noting

  • esc_url(), esc_url_raw() and sanitize_url() now return these URLs instead of ''. This includes scheme-less URLs such as www.example.com/wiki/Help:Contents, which are now kept as relative links. Without a colon, that URL gets http:// prepended. Aligning the prepend check is a possible follow-up, deliberately not bundled here.
  • wp_http_validate_url( '//example.org:8080/' ) now returns the URL, as //example.org/ already does. Private and loopback hosts are still rejected, and WP_Http::request() still refuses URLs without a scheme.
  • This covers #60347. The tests in PR Fix: Allow text fragments in kses for Trac ticket #60347 #7890 expect #:~:text= to be stripped when only mailto is allowed; here a fragment is kept regardless of the allowed protocols, the same as #footer and //host/ are today. Details are on the ticket.

Testing

New tests (@ticket 41304):

  • Tests_Kses: data providers for URLs that must be kept and for protocols that must still be stripped, plus wp_kses_post() cases.
  • Tests_Formatting_EscUrl::test_colon_after_fragment_or_query.

Before the fix, the "kept" cases, the wp_kses_post() footnote case and the esc_url() case fail; the "still stripped" cases pass both before and after. After the fix:

./vendor/bin/phpunit --group 41304       OK (23 tests, 24 assertions)
./vendor/bin/phpunit --group kses        OK (499 tests, 1556 assertions)
./vendor/bin/phpunit --group formatting  Tests: 2170, Assertions: 1118481, Warnings: 4
./vendor/bin/phpunit --group http        Tests: 189, Assertions: 294, Warnings: 1

No failures. The 5 warnings are existing PHPUnit 10 deprecation notices in unrelated tests (Tests_Formatting_wpStripAllTags, Tests_HTTP_IncludeOldRequestsClass).

Security checks beyond the unit tests:

  • About 10,000 inputs whose output changes were rendered as links in Chromium and Firefox. None resolved to a scheme other than an allowed one.
  • A randomized comparison of 400,000 strings built from scheme words, entity-encoded colons, / ? #, &#, &amp;, tabs and control characters found 98,391 changed outputs and 0 dangerous ones.
  • The new strip cases fail against '%[/?#]%' (no entity guard) and against a guard without &amp;.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Tracing the protocol check and its callers, writing the fix and tests, and running the browser and fuzz checks. All changes and results were reviewed by me.

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@dhrupo
Formatting: Keep URLs whose colon follows a path, query or fragment i…
640801d 
…n KSES.

`wp_kses_bad_protocol_once()` treats everything before the first colon as a protocol, unless that part contains the literal sequence `/?`. Browsers end a URL scheme at the first `/`, `?` or `#`, so relative URLs such as `#fn:1`, `/page#section:2` or `images?q=tbn:abc` were cut down to the text after the colon. Users without the `unfiltered_html` capability lost these links when saving content, and `esc_url()` returned an empty string for them.

A colon is no longer treated as a protocol separator when a `/`, `?` or `#` comes before it. A `#` directly after `&` or `&amp;` is not counted, since it may belong to a character reference.

Props bogdanpreda, dmsnell.
Fixes #41304.
@github-actions

github-actions Bot commented Oct 16, 2026 •
edited
Loading

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props dhrupo, rejaulalomkhan.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

github-actions Bot commented Oct 16, 2026

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@rejaulalomkhan

rejaulalomkhan commented Oct 16, 2026

Copy link
Copy Markdown

Thanks for working on this, @dhrupo! The table of before/after inputs in the description made it easy to check against browser parsing rules. I tested it locally:

Patch Testing Report

Patch tested: #14320

Environment

  • WordPress: 7.2-alpha-63166-src (trunk 3effce4 with the PR applied)
  • PHP: 8.3.35
  • Server: PHP CLI (PHPUnit 9.6.38, wordpress-develop Docker environment)
  • Database: MySQL 9.7.2
  • Browser: N/A (unit tests)
  • OS: macOS 27.0.1
  • Theme: N/A
  • MU Plugins: None activated
  • Plugins: None activated

Steps taken

  1. Ran Tests_Kses and Tests_Formatting_EscUrl with the PR: 519 tests pass on single site and multisite.
  2. Reverted src/wp-includes/kses.php with the PR's tests kept: the new tests fail (test_colon_after_fragment_or_query and every data set of test_wp_kses_bad_protocol_should_keep_colon_after_path_query_or_fragment, among others).
  3. Since this is a sanitization change, I ran 24 inputs through wp_kses_post() and esc_url() on trunk and with the PR, including the usual javascript: variants (mixed case, tab/newline/NUL, numeric and named entities for letters and the colon, feed:javascript:, vbscript:, data:). All of them are handled exactly as on trunk.
  4. The only outputs that change are the intended ones: #fn:1, /page#section:2, images?q=tbn:abc and javascript/:alert(1) (and its &#47; form) are now kept. The last one has a / before the colon, so browsers resolve it as a relative path, not a javascript: URL.
  5. ✅ Patch is solving the problem: colons after a path, query or fragment no longer lose the text before them, and protocol stripping for real schemes is unchanged.

Expected result

  • URLs whose first colon follows /, ? or # are kept as written, while disallowed protocols are still removed.

Additional Notes

  • AI assistance: Claude Code was used to run the tests and the input comparison; results reviewed by me.

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.

2 participants

@dhrupo @rejaulalomkhan