#2520 [Win32] Crash in FontDirectWrite::HFont when DirectWrite refused the font (e.g. SCI_STYLESETWEIGHT 1000), on showing autocompletion

[Pyre]

With a DirectWrite technology, FontDirectWrite leaves pTextFormat null when CreateTextFormat fails, for example for a weight DirectWrite refuses, set with SCI_STYLESETWEIGHT (Scintilla doesn't validate it; Windows documents 1..999 as the valid weights). The drawing code checks pTextFormat, but FontDirectWrite::HFont() doesn't, so showing an autocompletion list then crashes: ListBoxX::SetFont calls HFont(), which calls pTextFormat->GetFontFamilyName on a null pointer.

Steps: SCI_SETTECHNOLOGY(SC_TECHNOLOGY_DIRECTWRITE), SCI_STYLESETWEIGHT(STYLE_DEFAULT, 1000), SCI_STYLECLEARALL, SCI_AUTOCSHOW(0, "alpha beta gamma") -> access violation reading address 0 in FontDirectWrite::HFont (called by ListBoxX::SetFont). Reproduced with MinGW-w64 builds of 5.6.6 and 5.6.7 under Wine 9, whose DirectWrite accepts weights 0..950: negative weights and weights above 950 crash (tried -100, -1, 951, 999, 1000, 5000), 0..950 work. With the attached patch applied to 5.6.7, none of them crash.

The attached patch returns no HFONT when there's no text format, as HFont() already does when GetFontFamilyName fails. Alternatively SCI_STYLESETWEIGHT could clamp the weight, but SCI_STYLESETSTRETCH doesn't validate its value either (not tried), and any other failure of CreateTextFormat would leave pTextFormat null the same way.

The attached hfontcrash.cpp reproduces it: "hfontcrash 1000" crashes, "hfontcrash 400" works (build line in its header).

This was found and prepared with the help of an AI assistant (Claude), then reviewed and tested.

[Zufu Liu]

It seems the range is non-portable between platform:
GDI [0, 1000] or [0, 1000)?: https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-logfontw
D2D [1, 999]: https://learn.microsoft.com/en-us/windows/win32/api/dwrite/ne-dwrite-dwrite_font_weight
GTK [100, 1000]: https://docs.gtk.org/Pango/enum.Weight.html
QT [1, 1000]: https://doc.qt.io/qt-6/qfont.html#Weight-enum
wxWidgets [1, 1000] (same as CSS): https://docs.wxwidgets.org/3.2/classwx_font.html
CSS [1,1000]: https://www.w3.org/TR/css-fonts-4/#font-weight-prop

[Zufu Liu]

Another fix is clamping font weight, stretch in E_INVALIDARG block.

if (hr == E_INVALIDARG) {
    // Possibly a bad locale name like "/" so try "en-us".
    hr = pIDWriteFactory->CreateTextFormat(wsFace.c_str(), nullptr,
        static_cast<DWRITE_FONT_WEIGHT>(fp.weight),
        style,
        static_cast<DWRITE_FONT_STRETCH>(fp.stretch),
        fHeight, L"en-us", pTextFormat.ReleaseAndGetAddressOf());
}

[Pyre]

Thanks, good point about the ranges. Sounds like the clamping belongs in the Win32 DirectWrite code rather than in SCI_STYLESETWEIGHT.

I tried clamping in the E_INVALIDARG retry and it's nicer where it works: the font gets created and the text is drawn instead of nothing. It also covers stretch, which crashes the same way (stretch 0, 10 and -1 crash 5.6.7 too).

It doesn't catch everything though, at least under Wine 9: its DirectWrite refuses weights above 950 even though DWRITE_FONT_WEIGHT is documented as 1..999, so 1000 clamped to 999 still fails and HFont() still crashes. So I'd suggest doing both: clamp in the retry, and keep the null check in HFont() as a backstop for anything CreateTextFormat still refuses. Updated patch attached (scintilla-5.6.7-directwrite-font-clamp-and-hfont-guard.diff).

What I see with 5.6.7 under Wine 9, before -> after:

• weight -1: crash -> text drawn
• weight 951, 999, 1000: crash -> no crash (no text, Wine refuses 999 too)
• stretch 0, 10, -1: crash -> text drawn
• weights 0..950 and stretch 1..9: unchanged

I can't test on real Windows here. There, 1000 clamped to 999 should be accepted and drawn.

[Neil Hodgson]

Committed [fa2ab2] change that handles null pTextFormat and clamps weight and stretch.

There are different places to perform the clamp with a choice between safety, precision, and allowing some edge cases that may cause reasonable behaviour in some situations.

There is a documented stretch value DWRITE_FONT_STRETCH_UNDEFINED = 0 that always caused E_INVALIDARG for me on Windows 10. It may work somewhere, perhaps as a wildcard that matches any available stretch.

Only clamping inside the retry replaces the wsLocale argument with "en-us" so will lower the precision for out-of-bounds arguments. There could be multiple layers of retry but it seems reasonable to me to clamp weight and stretch to the known good range for the first try and use the second just for changing to the 'generic' locale.

If Wine fails with some values that work on Windows, that could be reported to Wine as it should be trying to maximize compatibility with Windows.