1772189 - Vertical chinese text causes firefox to crash

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[me]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:90.0) Gecko/20100101 Firefox/90.0

Steps to reproduce:

My website has a page with a block of vertical chinese text: https://yijun.hu/blog-cael/3dragons/

Simply go to that tab on version 101.0 (It didn't use to happen, at least before version 100)

Actual results:

It crashes.

The crash report is https://crash-stats.mozilla.org/report/index/53286904-f500-4ba4-ae96-0b0540220601

Within seems to be a very long loop of
gfxHarfBuzzShaper::GetGlyphVAdvance(unsigned int)
gfxFont::CreateVerticalMetrics()
gfxFont::GetGlyphAdvance(unsigned short, bool)

Expected results:

The page should have loaded.

Comment 1

[Peter Magyari (Desktop QA)]

Hi,

I wasn't able to reproduce this on W10 or MacOS 11.6 on my end.

Could you check if the crash also occurs in troubleshoot mode? Here is a link on how:
https://support.mozilla.org/en-US/kb/diagnose-firefox-issues-using-troubleshoot-mode

Thanks!

Comment 2

[me]

It also crashes in troubleshoot mode.

The site loads with no problem on my Linux system, and it... well it seems to be just crashing on this Windows machine, even without extensions.

The report is https://crash-stats.mozilla.org/report/index/507d2512-bbd2-4a21-beae-dbb5f0220604

Should I reinstall firefox?

Comment 3

[Jonathan Kew [:jfkthame]]

I doubt reinstalling will help, this looks like it's simply a bug in how we're handling a certain font, resulting in infinite recursion in the font code. Whether it reproduces or not is probably dependent on what Chinese font gets used on your particular system. (I see that your site uses Noto Serif SC, but it's also possible that the crash here is happening when we try to use a fallback font before the Noto webfont has fully loaded.)

If you temporarily remove the link to Noto Serif SC, so that the text just uses the default font, does it still crash?

If it does still crash without the Noto webfont, could you try something more: temporarily remove the writing-mode property, so that the Chinese text is horizontal rather than vertical, and then use the Developer Tools element inspector to check what font is being used for the Chinese. To do this, right-click on the Chinese text and choose Inspect from the menu, and then look in the Fonts pane of the right-most Inspector panel (where there are options Layout / Computed / Changes / ... you might need to use the menu at the right-hand edge to access Fonts).

Comment 4

[me]

Ok, so, this is incredibly strange, but I did what you said, and

• If I remove the @import url for Noto Serif SC, then the page loads fine, and in Noto Serif SC (since I think I have it installed locally)
• If I then remove 'Noto Serif SC' from the font-family in the element (leaving only serif), it crashes again
• If I then remove the writing-mode: vertical-rl;, it now loads in again, with simsun, the default windows serif Chinese font.

Thus, as far as I can see, the tab crashes if: (1) Noto Serif SC is specified, and it tries to fetch the font from google, and writing mode is vertical, or (2) Only Serif is specified, and writing mode is vertical. If instead it loads Noto from my local system, or has horizontal writing, it works fine.

Comment 5

[Jonathan Kew [:jfkthame]]

Thank you for those details -- that will be really helpful in tracking down exactly what's going wrong here. Sorry you've experienced this problem; we'll try to get it fixed soon.

Comment 6

[Jonathan Kew [:jfkthame]]

I was going to investigate this today but I am having trouble reproducing the crash on my Win10 machine, either with Firefox 101 or Nightly (103). I can see that you've replaced the vertical-Chinese block with an image for now, to avoid the problem, so I downloaded a local copy of the page and tried to revert to the original content (hiding the image and uncommenting the vertical text block), but I was unable to get it to crash.

Has anything else changed about your page that might affect this? Can you make a copy of the crashing version of the page available somewhere for testing (or attach it here), so I can confirm I'm trying exactly the same thing?

I wonder if there may be slightly different font versions involved? On my Win10 machine, the C:\Windows\Fonts\simsun.ttc file is dated December 7, 2019, and has a size of 18214472 bytes; the font reports its version as 5.16. Does that match what you have, or is there any difference?

Comment 7

[me]

Hi, the site is also stored in a git repository, the relevant snapshot before the format change is here:
https://github.com/Adrakaris/Adrakaris.github.io/tree/9085241ac43b7fead9e3285356fd19bcd572b8e8

In terms of fonts, I do have the same SimSun as you have, though I appear to have another "SimSun regular" installed, which has a chinese name, is 36,608,664 bytes, and dated to 4 February 2021, which I may have installed, I'm not sure. It also has a security notification "This file came from another computer and might be blocked to help protect this computer", which I've just seen

I also do have Noto Serif SC installed, version 1.001

Comment 8

[Jonathan Kew [:jfkthame]]

Thanks. Unfortunately, I still can't get it to crash here.... puzzling.

To rule out that other SimSun font being involved somehow, can you try removing it for now, and see if the problem still persists?

Comment 9

[me]

Oh you know what, I think it was that font. It isn't crashing now. Guess having another "SimSun" screwed with my system.

Comment 10

[Jonathan Kew [:jfkthame]]

Hah! Well, I'd still like to figure out exactly what's going on, because it still looks to me like we have a bug -- but fortunately the "normal" Windows font doesn't trigger it, only that other version. Does it have a version number or other identifying information that can help us trace where it may come from?

Comment 11

[me]

SimSun special, with the font name of 物制宋体, version 5.20, last modified dated to 04 Feb 2021

I've uploaded the font if you want to have a look: https://drive.google.com/file/d/10eYsZL7g_xX0M3EqO4gJtgsqrXNWAbTk/view?usp=sharing

Comment 12

[Jonathan Kew [:jfkthame]]

Ah - when I dump data from that font using FontTools/TTX, it reports an issue:

WARNING: The vhea.numberOfVMetrics exceeds the maxp.numGlyphs
WARNING: too much 'vmtx' table data

I also notice that a lot of metadata that I would expect to find in the 'name' table of SimSun appears to have been stripped. It looks to me like this may be an "unofficially extended version" of the font, and has perhaps been created without proper testing and quality control standards. (I don't suppose you remember where it came from?)

Anyhow, my guess -- I'll do some debugging to check -- is that the errors in the vertical metrics tables here are what's causing the Firefox crash. We should make our font-loading code more robust against this kind of inconsistency, so that it either silently ignores the errors (if that's feasible) or detects that something is wrong and ignores the font altogether.

(Most likely, if a similar flaw happened in a webfont, it would be harmless because we process webfont resources through the OpenType Sanitizer before attempting to use them, and either fix such errors or reject the resource. But for a locally-installed font, that doesn't happen.)

Comment 13

[me]

I see, so that was the issue. Thanks!

Comment 14

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1772189 - Avoid potential infinite recursion when initializing vertical metrics, if the font has a broken 'vmtx' table. r=lsalzman

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2d7601357c9
Avoid potential infinite recursion when initializing vertical metrics, if the font has a broken 'vmtx' table. r=lsalzman

Comment 16

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing multiple reftest failures . CLOSED TREE

Backout link
Push with failures
Link to failure log 1
Link to failure log 2

Failure line :
TEST-UNEXPECTED-FAIL | /css/css-values/ch-unit-002.html | Testing http://web-platform.test:8000/css/css-values/ch-unit-002.html == http://web-platform.test:8000/css/css-values/reference/ch-unit-002-ref.html
REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/writing-mode/1083848-3-inline-background-repeat.html == layout/reftests/writing-mode/1083848-3-inline-background-repeat-ref.html | image comparison, max difference: 255, number of differing pixels: 5376

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e8c5926cdde
Avoid potential infinite recursion when initializing vertical metrics, if the font has a broken 'vmtx' table. r=lsalzman

Comment 18

[Marian-Vasile Laza]

Backed out changeset 8e8c5926cdde (Bug 1772189) for causing web-platform reftest failures on ch-unit-002.html.
Backout link
Push with failures <--> Wr5
Failure Log
Also Wr2 Failure Log

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7721733f5655
Avoid potential infinite recursion when initializing vertical metrics, if the font has a broken 'vmtx' table. r=lsalzman

Comment 20

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/7721733f5655

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

Copying crash signatures from duplicate bugs.

Comment 23

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Hi,
I could not reproduce the crash on Firefox 101.0(build ID: 20220526203855) on Windows 10 64-bits. Would you be so kind as to confirm the latest fix on either beta/nightly channels?
Thank you.