Problem/Motivation
When rendering internal links that contain both a query string and a fragment identifier, Pathologic incorrectly encodes the URL by duplicating the query and fragment parts. For example, a stored URL in an <a> tag like:
<a>href="/node/84623?test=hello#anchor"></a>
is transformed into:
/path/alias%3Ftest%3Dhello%23anchor?test=hello#anchor
This results in invalid URLs with broken behavior (e.g., anchor targeting fails and query parameters are corrupted), and leads to degraded UX and broken routing.
This issue seems to occur because Pathologic passes query and fragment data both embedded in the path string and again separately in the $options array to Url::fromUri(), which does not sanitize duplication.
Steps to reproduce
- Ensure Pathologic 2.x is installed and enabled for a text format used by a node field. I used it with Full HTML for the WYSIWYG Editor.
-
Use that text format to save the following raw HTML into a node body or custom field:
/node/{real-node-id}?params=test#anchor
- View rendered node on the frontend
-
Inspect the rendered
<a>tag.
Expected Result:
<a href="/your-path-alias?test=hello#anchor">HELLO/a>
Actual Result:
<a href="/your-path-alias%3Ftest%3Dhello%23anchor?test=hello#anchor">HELLO</a>
Proposed resolution
Before passing $url_params['path'] into Url::fromUri(), strip out any embedded query string or fragment component by splitting on the first occurrence of ? or #:
$url_params['path'] = preg_split('/[?#]/', $url_params['path'], 2)[0];
This ensuresUrl::fromUri() receives the clean path separately from its query and fragment parts already correctly provided in $url_params['path'].
This change should be added just before $url_params['path'] is used to construct $path in _pathologic_replace().
Remaining tasks
- Add a functional test using pathologic_test to confirm output with both query string and fragment.
User interface changes
None
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| 2895153-1.patch.txt | 507 bytes | willfrost |
Comments
Comment #2
avpadernoComment #3
fjgarlin commentedIgnore this. Extra comment to fix indexing issue.
Comment #4
dwwThanks for the bug report, clear description of the problem and solution, and the patch!
Agreed with remaining tasks needing some test coverage for this bug. Tagging as such, and moving to "Needs work".
Also, it'd be great to convert this into a GitLab merge request so that the automated tests and other tooling will run.
Once we have an MR with a test that fails with the current code in Git, and passes with the proposed fix, this can probably be committed.
Thanks again!
-Derek
Comment #5
santanu mondal commentedComment #6
santanu mondal commentedI have try to produce this but i am not get any problem like this
the url is coming like this "HELLO"
Comment #7
santanu mondal commented