Skip to content

Fix over-blocking of safe URLs by the unsafe link filter - #1131

Merged
colinodell merged 2 commits into
thephpleague:2.8from
GrahamCampbell:anchor-unsafe-protocol-regex
Jul 12, 2026
Merged

Fix over-blocking of safe URLs by the unsafe link filter#1131
colinodell merged 2 commits into
thephpleague:2.8from
GrahamCampbell:anchor-unsafe-protocol-regex

Conversation

@GrahamCampbell

@GrahamCampbell GrahamCampbell commented Jul 12, 2026

Copy link
Copy Markdown
Member

When allow_unsafe_links is disabled, the ^ anchor in REGEX_UNSAFE_PROTOCOL binds only to the first alternative, so vbscript:, file:, and data: match anywhere in the URL rather than just at the start. Safe links such as https://example.com/view?src=data:image/png are misclassified as unsafe and silently lose their href/src.

The first commit adds tests demonstrating the bug and the second applies the one-line fix.


The JS implementation has a similar bug: commonmark/commonmark.js#305.

@GrahamCampbell GrahamCampbell changed the title Add tests demonstrating unsafe link filter over-blocking Fix over-blocking of safe URLs by the unsafe link filter Jul 12, 2026
@colinodell
colinodell requested a review from Copilot July 12, 2026 15:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a regex anchoring bug in the “unsafe link” detection logic which previously caused safe URLs to be misclassified as unsafe when they merely contained vbscript:, file:, or data: later in the URL (e.g., in query parameters), resulting in dropped href/src when unsafe links are forbidden.

Changes:

  • Add unit tests covering RegexHelper::isLinkPotentiallyUnsafe() for dangerous leading schemes and safe URLs containing those substrings elsewhere.
  • Fix REGEX_UNSAFE_PROTOCOL so the ^ anchor applies to all protocol alternatives.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/unit/Util/RegexHelperTest.php Adds data-driven tests reproducing the over-blocking and validating safe/unsafe URL cases.
src/Util/RegexHelper.php Corrects the unsafe-protocol regex to properly anchor all alternatives at the start of the URL.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@colinodell colinodell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching and fixing this!

@colinodell
colinodell merged commit cfb11ec into thephpleague:2.8 Jul 12, 2026
17 of 18 checks passed
@GrahamCampbell
GrahamCampbell deleted the anchor-unsafe-protocol-regex branch July 12, 2026 15:30
@GrahamCampbell

Copy link
Copy Markdown
Member Author

Nice, this was merged in the JS package too, so they are kept in sync. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants