ProcessPageEditLink applies $sanitizer->url, but only to existing values

[Toutouwai]

Short description of the issue

When creating a new link in a CKEditor field via ProcessPageEditLink no $sanitizer method is applied to the link href. Arguably this is fine as the link will be part of the CKEditor content and subject to whatever ACF and HTML Purifier settings the user has configured.

But when editing an existing link the href value is passed through $sanitizer->url - this is inconsistent. Either it is essential to apply this sanitizer in which case it should be applied to new links also, or it is not essential in which case it should not be applied when editing an existing link.

Or better: it should be configurable whether to apply the sanitizer to both new and existing links and configurable what $sanitizer->url options are applied.

Incidentally, it is impossible for a javascript link to pass validation by $sanitizer->url even when the 'javascript' scheme is explicitly allowed, as a javascript link cannot pass PHP's FILTER_VALIDATE_URL which is always applied by $sanitizer->url. This makes it impossible to edit an existing link in ProcessPageEditLink that uses the 'javascript' scheme. It's probably not a great idea to allow JS links in a CKEditor field but it seems like it ought to be possible if a user is determined to allow this.

The forum topic that prompted this issue: https://processwire.com/talk/topic/16212-ckeditor-custom-config-option-linkjavascriptlinksallowed-true-doesnt-work/

Setup/Environment

• ProcessWire version: 3.0.61

[ryancramerdesign]

The reason the sanitizer runs for an existing link is because the existing link comes through as user input to the server via a GET variable. We always sanitize all input that gets sent to the server, especially before echoing it back out to the user. But when entering a new link, it never gets sent to the server, as it all happens in Javascript by a CKEditor plugin. There's no opportunity for the sanitizer to run here, since there is no trip to the server... at least not until the page is saved.

I don't think it's good for us to be supporting javascript scheme links, as that's just a security problem. If someone wants to open those up to ACF and Purifier for some specific case, that's fine (even if I'd never recommend it), but I think that's going to be rare and probably appropriate to use the view source dialog for entering javascript code... definitely don't want to get ProcessPageEditLink in the business of having to accept and validate javascript as input.

[Toutouwai]

Makes sense, thanks for the explanation.