I have a text format that has the token_filter enabled.

I want to use the [site:url] token as the href of a link, added via the standard core ckeditor link dialog.

On output the first time, the link is correct. But when re-editing the node, the token will be silently truncated to url] and saved into the database.

I would not expect a token in a url to be silently broken and saved into the database.

To reproduce:

  1. Instantiate a simplytest.me instance with token_filter module.
  2. Edit the basic html text format, enabling token replacement.
  3. Move the token replacement filter before "Limit allowed HTML tags and correct faulty HTML".
  4. Create a basic page node.
  5. Select basic html format for the text field.
  6. Add a link to the text field using the editor link button, with [site:url] as the link href.
  7. Also add the same token to the text for checking.
  8. Save the node.
  9. Inspect the output. The link href is replaced by the site url. The token in the text is replaced by the site url.
  10. Edit the node again. Just add some text and save, don't touch the link.
  11. Inspect the output. The link href has been truncated to url]. The token in the text is replaced by the site url.
  12. Edit the node again. Double click the link to check it. The token has been saved in its truncated state.

Comments

John Pitcairn created an issue. See original summary.

johnpitcairn’s picture

johnpitcairn commented
Issue summary: View changes
johnpitcairn’s picture

johnpitcairn commented
Component: filter.module » editor.module

Moving to editor module. Line 84 of \Drupal\editor\EditorXssFilter\Standard.php:

$output = static::filter($html, $blacklisted_tags);

This is where the token is getting mangled on input.

johnpitcairn’s picture

johnpitcairn commented

It all boils down to UrlHelper::filterBadProtocol() being run on the link href at ckeditor load. In the absence of a full URL, everything up to the colon in the token is interpreted as a protocol.

This means you cannot reliably use a token as the path component of a link via ckeditor. The same token in the query component of the link is fine.

If you precede the token with a slash, the token is not truncated, but that will result in an invalid URL when the token is replaced by either a Drupal path (itself beginning with /) or by a full URL.

johnpitcairn’s picture

johnpitcairn commented
Issue summary: View changes
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Component: editor.module » base system
Status: Active » Postponed (maintainer needs more info)
Issue tags: +Needs tests

This is one of so many cases of UrlHelper::filterBadProtocol() getting in the way. There's nothing that editor.module can do to help here.

I'm assuming this is when you also use https://www.drupal.org/project/token_filter?

What would help is test coverage.

johnpitcairn’s picture

johnpitcairn commented

Thanks

I'm not using token_filter module, I have a custom module that supplies its own token and filter. The filter runs before the "limit tags" filter, but that doesn't help, if the token is not replaced and left as-is it still will get mangled. I think this can be tested without any filter, since the problem occurs when the token is not replaced.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

k-l’s picture

k-l commented

Solved same issue by creating custom EditorXssFilter which extended \Drupal\editor\EditorXssFilter\Standard.php and just implements attributes() method. Which is identical to attributes method in \Drupal\Component\Utility\Xss.php only adjusted to set $skip_protocol_filtering = TRUE; when href contains token. Then using hook_editor_xss_filter_alter() custom filter class is set for selected text format.
When displaying text, tokens needs to be replaced before 'Correct faulty and chopped off HTML' filter kicks in.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue tags: +Bug Smash Initiative

Thanks for reporting this issue. We rely on issue reports like this one to resolve bugs and improve Drupal core.

As part of the Bug Smash Initiative, we are triaging issues that are marked "Postponed (maintainer needs more info)". This issue was marked "Postponed (maintainer needs more info)" 5 years ago however it appears that the status was not reset.

Regardless I think the fix here is for you to run the token filter before the limit/fix HTML one.

Is there a reason this isn't possible? Perhaps that your filter uses tags that shouldn't ordinarily be allowed?

Is this still an issue?

johnpitcairn’s picture

johnpitcairn commented
Title: Limit tags and correct faulty html filter destructively breaks tokens in ckeditor link href » UrlHelper::filterBadProtocol() destructively truncates tokens in ckeditor link href
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Active

Thanks @larowlan - it's been a while since I've looked at this. I've reproduced the issue in a clean simplytest.me install.

Running the token filter before limit/correct html does not resolve the problem if a link url contains a token. On subsequent editing, that token will be silently truncated and saved to the database.

I have updated the title and issue summary steps to reproduce to reflect the real problem, which is UrlHelper::stripBadProtocol() as per #6.

johnpitcairn’s picture

johnpitcairn commented
Issue summary: View changes
johnpitcairn’s picture

johnpitcairn commented
Priority: Normal » Major

Bumping to major as user input is irrevocably altered.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Postponed (maintainer needs more info)

if a link url contains a token

Can you provide an example URL here

Is the token url encoded?

At that point it aught to be

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

e.g. I assume your use case here is something like /foo/bar/get-token?token=[foo:bar] but at that point it should be encoded to %5Bfoo%3Abar%5D

or does that not matter because filterBadProtocol decodes it anyway?

Perhaps a good starting point is to add a failing case to \Drupal\Tests\Component\Utility\UrlHelperTest::providerTestFilterBadProtocol so we're all talking about the same thing

johnpitcairn’s picture

johnpitcairn commented

Simplest case is <a href="[site:url]">This will break</a>, as per the issue summary.

That seems a not unreasonable use case for something somebody might want to do using the editor link button and a token.

I'll try to find some time to add a failing test.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

I'm sorry, I still don't understand the problem.

If your example is <a href="[site:url]">This will break</a> and your token filter runs before the HTML validation, doesn't it get transformed to <a href="https://somesite.com">This will break</a> before the HTML validation?

I must be missing something

johnpitcairn’s picture

johnpitcairn commented

Yes, the first time after saving.

But if you edit the node again, without touching the link at all, the token still gets run through the protocol check when ckeditor loads or something, and the link href gets saved in that broken state.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Postponed (maintainer needs more info) » Closed (duplicate)

#2944173: CKEditor mangles tokens in URLs, due to bug in Xss::attributes() is a duplicate and has a patch so closing this one

johnpitcairn’s picture

johnpitcairn commented

Thanks @larowlan, I agree this is a duplicate.