Page MenuHomePhabricator

Error When Editing Pages with Specific Unicode Character in Visual Editor
Closed, ResolvedPublicPRODUCTION ERROR

Description

Steps to Reproduce:

  • Open a page containing the symbol "Combining Long Solidus Overlay" (Hex Code: ̸) in the Visual Editor and do any change.
  • Attempt to:
    • Switch to the code editor, or
    • Save the changes.

Actual Results:

  • The operation does not complete (e.g., switching to code editor or saving changes does nothing).
  • The following error appears in the browser console:
ve.dm.Converter.js:203 Uncaught InvalidCharacterError: Failed to execute 'setAttribute' on 'Element': '≯<' is not a valid attribute name.
    at ve.dm.Converter.static.renderHtmlAttributeList (ve.dm.Converter.js:203:28)
    at ve.dm.Converter.getDomElementsFromDataElement (ve.dm.Converter.js:506:27)
    at ve.dm.Converter.getDomSubtreeFromData (ve.dm.Converter.js:1710:35)
    at ve.dm.Converter.getDomSubtreeFromModel (ve.dm.Converter.js:1286:7)
    at ve.dm.Converter.getDomFromModel (ve.dm.Converter.js:1242:7)
    at ve.dm.Surface.getDom (ve.dm.Surface.js:189:26)
    at ve.ui.Surface.getDom (ve.ui.Surface.js:273:25)
    at ve.init.mw.ArticleTarget.createDocToSave (ve.init.mw.ArticleTarget.js:1213:27)
    at ve.init.mw.ArticleTarget.getDocToSave (ve.init.mw.ArticleTarget.js:1198:25)
    at ve.init.mw.DesktopArticleTarget.switchToFallbackWikitextEditor (ve.init.mw.DesktopArticleTarget.js:1449:31)

Expected Results:
The editor should handle the Unicode character correctly.

Originally reported at https://ru.wikipedia.org/wiki/Википедия:Форум/Технический#Визиред:_Невозможно_получить_Parsoid_HTML

Event Timeline

parsoid transform this:

<p>a&#x338;aaaa</p>

into this:

a<span typeof="mw:Entity" id="mwAw" ≯<="" span="">aaaa</span>

Restricted Application changed the subtype of this task from "Bug Report" to "Production Error". · View Herald TranscriptDec 25 2024, 9:53 AM

@ssastry the problem is with all combining diacritics.

Is this still happening? I attempted to reproduce on my local instance and on mediawiki.org and en.wikipedia.org and was unable to duplicate this behaviour.

Note: Chrome Version 133.0.6943.55 (Official Build) (arm64), on MacOS 14.4.1

I can reproduce this locally if the page actually contains the symbol 'Combining Long Solidus Overlay' (Hex Code: &#x338;). Also, I can reproduce the issue with Parsoid transforms <p>a&#x338;aaaa</p> into <a<span typeof="mw:Entity" id="mwAw" ≯<="" span="">aaaa</span> by running echo <p>a&#x338;aaaa</p> | node bin/parse

Thank you for double checking and for providing further details.

I downloaded Parsoid from gerritt (mediawiki/services/parsoid) but I didn't find bin/parse. Could you let me know the repository and branch for parsoid you were on while testing from the command line?

I did find bin/parse.php, and I was unable to reproduce using it:

parsoid master $ echo '<p>a&#x338;aaaa</p>' | php bin/parse.php
<p data-parsoid='{"stx":"html","dsr":[0,19,3,4]}'>a<span typeof="mw:Entity" data-parsoid='{"src":"&amp;#x338;","srcContent":"̸","dsr":[4,11,null,null]}'≯</span>aaaa</p>
parsoid master $ echo '<p>a&#x338;aaaa</p>' | php bin/parse.php | xxd
00000000: 3c70 2064 6174 612d 7061 7273 6f69 643d  <p data-parsoid=
00000010: 277b 2273 7478 223a 2268 746d 6c22 2c22  '{"stx":"html","
00000020: 6473 7222 3a5b 302c 3139 2c33 2c34 5d7d  dsr":[0,19,3,4]}
00000030: 273e 613c 7370 616e 2074 7970 656f 663d  '>a<span typeof=
00000040: 226d 773a 456e 7469 7479 2220 6461 7461  "mw:Entity" data
00000050: 2d70 6172 736f 6964 3d27 7b22 7372 6322  -parsoid='{"src"
00000060: 3a22 2661 6d70 3b23 7833 3338 3b22 2c22  :"&amp;#x338;","
00000070: 7372 6343 6f6e 7465 6e74 223a 22cc b822  srcContent":".."
00000080: 2c22 6473 7222 3a5b 342c 3131 2c6e 756c  ,"dsr":[4,11,nul
00000090: 6c2c 6e75 6c6c 5d7d 273e ccb8 3c2f 7370  l,null]}'>..</sp
000000a0: 616e 3e61 6161 613c 2f70 3e0a            an>aaaa</p>.

I was able to reproduce by putting a bare combining character in its own paragraph, then saving, and then by making another edit and attempting to switch to the source editor. I also found that deleting the bare combining character before switching does not trigger the error.

By placing a breakpoint in ve.dm.Converter.static.renderHtmlAttributeList I did spot this:

originalDomElements[i].outerHTML
'<p id="mwBg" ≯<="" p="">\n\n</p>'

And indeed I was able to dig this out of the store:

d.getStore().hashStore['h98407532307a395d'][0].outerHTML
'<p id="mwBg" ≯<="" p="">\n\n</p>'

I'm not sure whether we're getting this from parsoid or not, but it's certainly not a state we would want the store to get into. I'll continue investigating.

Here's another example page where it breaks: https://ko.wikipedia.org/wiki/%EC%9C%A0%EB%8B%88%EC%BD%94%EB%93%9C_0000~0FFF (relevantly enough: the page for a unicode codepoint range that includes some combining characters)

I'm inclined to think that the issue is essentially that Parsoid is outputting invalid HTML from a given bit of wikitext, albeit very peculiar wikitext. It should presumably be HTML-escaping any bare combining-character if found immediately after a >.

On VE's side, we should probably not break so hard when the converter fails. It's a sign of an unrecoverable state of some sort, but we should probably show an error message at least.

Thanks @zoe , the bare character on its own in a paragraph is a really useful minimal test case. Something is normalizing this sequence into this one . They look the same, so for clarity that is:

U+003E GREATER-THAN SIGN followed by
U+0338 COMBINING LONG SOLIDUS OVERLAY

getting normalized into a single character:

U+226F NOT GREATER THAN

It's probably somewhere where we convert to Unicode Normalization Form C. I'll investigate further.

Ok it seems to be the API that performs the breaking normalization.

Steps to reproduce
Load up any Wikipedia page, then run this in the browser debugging console:

const COMBINING_LONG_SOLIDUS = '\u0338';
new mw.Api().post( {
        action:  'visualeditor',
        paction: 'parsefragment',
        page: 'Test',
        wikitext: COMBINING_LONG_SOLIDUS + 'FOOBAR'
} ).done( ( data ) => { 
        const content = data.visualeditor.content; 
        const codeUnit = content[ content.indexOf( 'FOOBAR' ) - 1 ];
        const hex = codeUnit.codePointAt( 0 ).toString( 16 );
        console.log( 'Content:', content );
        console.log( 'Code unit before FOOBAR:', codeUnit, hex );
} ).fail( ( err ) => console.error( err ) );

Expected response
The COMBINING LONG SOLIDUS is present in the parsed output:

Content: <p id="mwAg"≯FOOBAR</p>
VM4934:12 Code unit before FOOBAR: ̸ 338

Actual response
The COMBINING LONG SOLIDUS has been combined with the preceding GREATER-THAN SIGN to form a NOT GREATER THAN symbol, corrupting the html tag:

Content: <p id="mwAg"≯FOOBAR</p>
Code unit before FOOBAR: ≯ 226f

Cause
This seems to be something in the API glue, because Parsoid gets this right if called on the command line:

$ echo -e '\u0338FOOBAR' | php bin/parse.php |hexdump -c
0000000   <   p       d   a   t   a   -   p   a   r   s   o   i   d   =
0000010   '   {   "   d   s   r   "   :   [   0   ,   8   ,   0   ,   0
0000020   ]   }   '   > 314 270   F   O   O   B   A   R   <   /   p   >
0000030  \n                                                            
0000031

You can see the > 314 270 in the output, i.e. GREATER THAN SIGN followed by COMBINING LONG SOLIDUS OVERLAY.

Unfortunately, the cause is deep in the generic MediaWiki API. ApiVisualEditor.php just calls $apiResult->addValue( $result ), and that automatically applies Unicode Normal Form C, with the following call stack:

MediaWiki::Api::ApiResult::addValue
  MediaWiki::Api::ApiResult::validateValue
    MediaWiki::Language::normalize
      UtfNormal::Validator::cleanUp
        normalizer_normalize( $string, Normalizer::FORM_C )

Therefore I think the simplest fix is to escape combining characters in $result into HTML entities before passing them to addValue.

while this gets right:

echo '\u0338FOOBAR' | php bin/parse.php
<p data-parsoid='{"dsr":[1,13,0,0]}'>\u0338FOOBAR</p>

this gets for me:

echo '&#x338;FOOBAR' | php bin/parse.php
<p data-parsoid='{"dsr":[0,14,0,0]}'><span typeof="mw:Entity" data-parsoid='{"src":"&amp;#x338;","srcContent":"̸","dsr":[0,7,null,null]}'≯</span>FOOBAR</p>

PS C:\Users\mediawiki-services-parsoid>
'&#x338;FOOBAR' | php bin/parse.php | Format-Hex

00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F

00000000 3C 70 20 64 61 74 61 2D 70 61 72 73 6F 69 64 3D <p data-parsoid=
00000010 27 7B 22 64 73 72 22 3A 5B 30 2C 31 34 2C 30 2C '{"dsr":[0,14,0,
00000020 30 5D 7D 27 3E 3C 73 70 61 6E 20 74 79 70 65 6F 0]}'><span typeo
00000030 66 3D 22 6D 77 3A 45 6E 74 69 74 79 22 20 64 61 f="mw:Entity" da
00000040 74 61 2D 70 61 72 73 6F 69 64 3D 27 7B 22 73 72 ta-parsoid='{"sr
00000050 63 22 3A 22 26 61 6D 70 3B 23 78 33 33 38 3B 22 c":"&amp;#x338;"
00000060 2C 22 73 72 63 43 6F 6E 74 65 6E 74 22 3A 22 3F ,"srcContent":"?
00000070 3F 22 2C 22 64 73 72 22 3A 5B 30 2C 37 2C 6E 75 ?","dsr":[0,7,nu
00000080 6C 6C 2C 6E 75 6C 6C 5D 7D 27 3E 3F 3F 3C 2F 73 ll,null]}'>??</s
00000090 70 61 6E 3E 46 4F 4F 42 41 52 3C 2F 70 3E pan>FOOBAR</p>

PS C:\Users\mediawiki-services-parsoid> '\u0338FOOBAR' | php bin/parse.php | Format-Hex

00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F

00000000 3C 70 20 64 61 74 61 2D 70 61 72 73 6F 69 64 3D <p data-parsoid=
00000010 27 7B 22 64 73 72 22 3A 5B 31 2C 31 33 2C 30 2C '{"dsr":[1,13,0,
00000020 30 5D 7D 27 3E 5C 75 30 33 33 38 46 4F 4F 42 41 0]}'>\u0338FOOBA
00000030 52 3C 2F 70 3E R</p>

Ok, I think it's specific to U+0338 as that's the only combining character that can "corrupt" an XML special character:

$ curl --silent https://www.unicode.org/Public/UNIDATA/UnicodeData.txt | perl -F';' -ne 'print if $F[5] =~ /0022|0026|0027|003C|003E/'
226E;NOT LESS-THAN;Sm;0;ON;003C 0338;;;;Y;NOT LESS THAN;;;;
226F;NOT GREATER-THAN;Sm;0;ON;003E 0338;;;;Y;NOT GREATER THAN;;;;
FE60;SMALL AMPERSAND;Po;0;ON;<small> 0026;;;;N;;;;;
FE64;SMALL LESS-THAN SIGN;Sm;0;ON;<small> 003C;;;;Y;;;;;
FE65;SMALL GREATER-THAN SIGN;Sm;0;ON;<small> 003E;;;;Y;;;;;
FF02;FULLWIDTH QUOTATION MARK;Po;0;ON;<wide> 0022;;;;N;;;;;
FF06;FULLWIDTH AMPERSAND;Po;0;ON;<wide> 0026;;;;N;;;;;
FF07;FULLWIDTH APOSTROPHE;Po;0;ON;<wide> 0027;;;;N;;;;;
FF1C;FULLWIDTH LESS-THAN SIGN;Sm;0;ON;<wide> 003C;;;;Y;;;;;
FF1E;FULLWIDTH GREATER-THAN SIGN;Sm;0;ON;<wide> 003E;;;;Y;;;;;

@ol_roman did you have an example where a different diacritic breaks things?

Change #1121643 had a related patch set uploaded (by Divec; author: Divec):

[mediawiki/extensions/VisualEditor@master] WIP: Encode U+0338 as a HTML entity

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

I do not yet fully understand the workflow, but why is the patch in VisualEditor and not in Parsoid if Parsoid is the one outputting invalid HTML?

echo '&#x338;FOOBAR' | php bin/parse.php
<p data-parsoid='{"dsr":[0,14,0,0]}'><span typeof="mw:Entity" data-parsoid='{"src":"&amp;#x338;","srcContent":"̸","dsr":[0,7,null,null]}'≯</span>FOOBAR</p>

I do not yet fully understand the workflow, but why is the patch in VisualEditor and not in Parsoid if Parsoid is the one outputting invalid HTML?

That Parsoid output is in fact 100% correct — it's just really hard to see so, because the combining characters make it look messed up.

You encode U+0338 as an HTML entity, but from what I can see, it works the other way around. The Unicode escape sequence works correctly, but the HTML entity does not.

echo '\u0338FOOBAR' | php bin/parse.php

Output:

<p data-parsoid='{"dsr":[1,13,0,0]}'>\u0338FOOBAR</p>

However, when using the HTML entity:

echo '&#x338;FOOBAR' | php bin/parse.php

Output:

<p data-parsoid='{"dsr":[0,14,0,0]}'><span typeof="mw:Entity" data-parsoid='{"src":"&amp;#x338;","srcContent":"̸","dsr":[0,7,null,null]}'≯</span>FOOBAR</p>

Well echo '\u0338' just literally outputs the string ‘\’, ‘u’, ‘0’, ‘3’, ‘3’, ‘8’, which is why it's unaffected. It needs echo -e to create the actual Unicode codepoint.

The output to echo '&#x338;FOOBAR' | php bin/parse.php is actually correct — It is the uncombined sequence ‘>’ + U+0338 — it just looks wrong because of how that renders visually as a single glyph.

@ol_roman did you have an example where a different diacritic breaks things?

No, @dchan, I have only encountered errors with U+0338.

Change #1121643 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Encode U+0338 as a html numeric character reference

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

Hello @ol_roman, this should be fixed now. Can you confirm it works for you?

VPuffetMichel assigned this task to dchan.
VPuffetMichel added a project: Essential-Work.
VPuffetMichel subscribed.

Closing this. @ol_roman Do not hesitate to let us know if it does not work for you.

For posterity: this was identified as a security issue and was fixed in https://phabricator.wikimedia.org/T387130, which is now visible to all users