Page MenuHomePhabricator

VE crashes on page with partially percent encoded URL in anchor of internal link
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

What happens?:

  • VE startup crashes with a JS error URIError.

mw.utils.parsoid.js: mw.libs.ve.getTargetDataFromHref

		if ( href.match( /^\.\// ) ) {
			// The specific case of parsoid resource URIs, which are in the form `./Title`.
			// If they're redlinks they now include a querystring which should be stripped.
			uri = new mw.Uri( href );

The mw.Uri fails with a URIError exception, after which VE stops loading, with no user feedback.

What should have happened instead?:
Editor should have given user feedback, as invalid anchors in wikitext are 'allowed'. Any new mw.Uri with untrusted user input (wikitext) should probably always be wrapped in a try/catch.

Software version (skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

Event Timeline

Change 867192 had a related patch set uploaded (by DLynch; author: DLynch):

[mediawiki/extensions/VisualEditor@master] Try/catch around mw.Uri when decoding parsoid resources

https://gerrit.wikimedia.org/r/867192

Change 867192 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Try/catch around mw.Uri when decoding parsoid resources

https://gerrit.wikimedia.org/r/867192

.. a page with the incorrectly encoded anchor ..

Note that putting "0.7% ODA" in the fragment is not incorrect encoding. The spec is very loose on how to interpret percentages:

Otherwise, if byte is 0x25 (%) and the next two bytes after byte in input are not in the ranges 0x30 (0) to 0x39 (9), 0x41 (A) to 0x46 (F), and 0x61 (a) to 0x66 (f), all inclusive, append byte to output.

i.e. it should only try to decode % followed by two hex chars, so mw.Uri is the buggy code in this instance.

If we use the native URL API it should work fine, and we have that loaded via a polyfill.

It is displayed as a red link and VE works fine without crashing.

@DLynch , confirming that this is the expected behaviour.
Cc: @Esanders , @matmarex

It should, indeed, be a red link and not crash VE.

Esanders renamed this task from VE crashes on page with incorrectly uri encoded anchor on internal link to VE crashes on page with partially percent encoded URL in anchor of internal link.Dec 19 2022, 6:56 PM
Esanders updated the task description. (Show Details)