Skip to content

Commit f7066e0

Browse files
committed
Bug 1455848 - validate access to DWrite font files in WR and output helpful log messages on failure. r=jrmuizel
1 parent c13d380 commit f7066e0

File tree

6 files changed

+128
-1
lines changed

6 files changed

+128
-1
lines changed

gfx/2d/Logging.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,23 @@ class Log {
302302
}
303303
return *this;
304304
}
305+
#ifdef WIN32
306+
Log& operator<<(const wchar_t aWStr[]) {
307+
if (MOZ_UNLIKELY(LogIt())) {
308+
int wLen = (int)wcslen(aWStr);
309+
std::vector<char> str;
310+
int n = WideCharToMultiByte(0, 0, aWStr, wLen, nullptr, 0, nullptr,
311+
nullptr);
312+
if (n > 0) {
313+
std::vector<char> str(n+1);
314+
WideCharToMultiByte(0, 0, aWStr, wLen, str.data(), n+1, nullptr,
315+
nullptr);
316+
mMessage << str.data();
317+
}
318+
}
319+
return *this;
320+
}
321+
#endif
305322
Log& operator<<(bool aBool) {
306323
if (MOZ_UNLIKELY(LogIt())) {
307324
mMessage << (aBool ? "true" : "false");

gfx/2d/ScaledFontDWrite.cpp

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,14 +385,64 @@ bool UnscaledFontDWrite::GetWRFontDescriptor(WRFontDescriptorOutput aCb,
385385
return false;
386386
}
387387

388+
// FIXME: Debugging kluge for bug 1455848. Remove once debugged!
389+
UINT32 numFiles;
390+
hr = mFontFace->GetFiles(&numFiles, nullptr);
391+
if (FAILED(hr) || !numFiles) {
392+
return false;
393+
}
394+
std::vector<RefPtr<IDWriteFontFile>> files;
395+
files.resize(numFiles);
396+
hr = mFontFace->GetFiles(&numFiles, getter_AddRefs(files[0]));
397+
if (FAILED(hr)) {
398+
return false;
399+
}
400+
MOZ_ASSERT(familyName.size() >= 1 && familyName.back() == 0);
401+
for(auto& file : files) {
402+
const void* key;
403+
UINT32 keySize;
404+
hr = file->GetReferenceKey(&key, &keySize);
405+
if (FAILED(hr)) {
406+
return false;
407+
}
408+
RefPtr<IDWriteFontFileLoader> loader;
409+
hr = file->GetLoader(getter_AddRefs(loader));
410+
if (FAILED(hr)) {
411+
return false;
412+
}
413+
RefPtr<IDWriteLocalFontFileLoader> localLoader;
414+
loader->QueryInterface(__uuidof(IDWriteLocalFontFileLoader), (void**)getter_AddRefs(localLoader));
415+
if (!localLoader) {
416+
return false;
417+
}
418+
UINT32 pathLen;
419+
hr = localLoader->GetFilePathLengthFromKey(key, keySize, &pathLen);
420+
if (FAILED(hr)) {
421+
return false;
422+
}
423+
size_t offset = familyName.size();
424+
familyName.resize(offset + pathLen + 1);
425+
hr = localLoader->GetFilePathFromKey(key, keySize, &familyName[offset], pathLen + 1);
426+
if (FAILED(hr)) {
427+
return false;
428+
}
429+
MOZ_ASSERT(familyName.back() == 0);
430+
DWORD attribs = GetFileAttributesW(&familyName[offset]);
431+
if (attribs == INVALID_FILE_ATTRIBUTES) {
432+
gfxCriticalNote << "sending font family \"" << &familyName[0]
433+
<< "\" with invalid file \"" << &familyName[offset]
434+
<< "\"";
435+
}
436+
}
437+
388438
// The style information that identifies the font can be encoded easily in
389439
// less than 32 bits. Since the index is needed for font descriptors, only
390440
// the family name and style information, pass along the style in the index
391441
// data to avoid requiring a more complicated structure packing for it in
392442
// the data payload.
393443
uint32_t index = weight | (stretch << 16) | (style << 24);
394444
aCb(reinterpret_cast<const uint8_t*>(familyName.data()),
395-
(familyName.size() - 1) * sizeof(WCHAR), index, aBaton);
445+
familyName.size() * sizeof(WCHAR), index, aBaton);
396446
return true;
397447
}
398448

gfx/layers/ipc/PWebRenderBridge.ipdl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ parent:
8282
async FlushApzRepaints();
8383
sync GetAPZTestData() returns (APZTestData data);
8484

85+
// Debugging routine for bug 1455848.
86+
async ValidateFontDescriptor(uint8_t[] desc);
87+
8588
async Shutdown();
8689
sync ShutdownSync();
8790
child:

gfx/layers/wr/WebRenderBridgeChild.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,14 @@ static void WriteFontDescriptor(const uint8_t* aData, uint32_t aLength,
225225

226226
*sink->mFontKey = sink->mWrBridge->GetNextFontKey();
227227

228+
#ifdef XP_WIN
229+
// FIXME: Debugging kluge for bug 1455848. Remove once debugged!
230+
nsTArray<uint8_t> data;
231+
data.AppendElements(aData, aLength);
232+
sink->mWrBridge->SendValidateFontDescriptor(data);
233+
aLength = uint32_t(wcsnlen_s((const wchar_t*)aData, aLength / sizeof(wchar_t)) * sizeof(wchar_t));
234+
#endif
235+
228236
sink->mResources->AddFontDescriptor(
229237
*sink->mFontKey, Range<uint8_t>(const_cast<uint8_t*>(aData), aLength),
230238
aIndex);

gfx/layers/wr/WebRenderBridgeParent.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@
3535
#include "mozilla/webrender/RenderThread.h"
3636
#include "mozilla/widget/CompositorWidget.h"
3737

38+
#ifdef XP_WIN
39+
#include "dwrite.h"
40+
#endif
41+
3842
using mozilla::Telemetry::LABELS_CONTENT_FRAME_TIME_REASON;
3943

4044
#ifdef MOZ_GECKO_PROFILER
@@ -697,6 +701,48 @@ void WebRenderBridgeParent::ObserveSharedSurfaceRelease(
697701
}
698702
}
699703

704+
// Debugging kluge for bug 1455848. Remove once debugged!
705+
mozilla::ipc::IPCResult WebRenderBridgeParent::RecvValidateFontDescriptor(
706+
nsTArray<uint8_t>&& aData) {
707+
if (mDestroyed) {
708+
return IPC_OK();
709+
}
710+
#ifdef XP_WIN
711+
nsTArray<uint8_t> data(aData);
712+
wchar_t* family = (wchar_t*)data.Elements();
713+
size_t remaining = data.Length() / sizeof(wchar_t);
714+
size_t familyLength = wcsnlen_s(family, remaining);
715+
MOZ_ASSERT(familyLength < remaining && family[familyLength] == 0);
716+
remaining -= familyLength + 1;
717+
wchar_t* files = family + familyLength + 1;
718+
BOOL exists = FALSE;
719+
if (RefPtr<IDWriteFontCollection> systemFonts = Factory::GetDWriteSystemFonts()) {
720+
UINT32 idx;
721+
systemFonts->FindFamilyName(family, &idx, &exists);
722+
}
723+
if (!remaining) {
724+
gfxCriticalNote << (exists ? "found" : "MISSING")
725+
<< " font family \"" << family
726+
<< "\" has no files!";
727+
}
728+
while (remaining > 0) {
729+
size_t fileLength = wcsnlen_s(files, remaining);
730+
MOZ_ASSERT(fileLength < remaining && files[fileLength] == 0);
731+
DWORD attribs = GetFileAttributesW(files);
732+
if (!exists || attribs == INVALID_FILE_ATTRIBUTES) {
733+
gfxCriticalNote << (exists ? "found" : "MISSING")
734+
<< " font family \"" << family
735+
<< "\" has " << (attribs == INVALID_FILE_ATTRIBUTES ? "INVALID" : "valid")
736+
<< " file \"" << files
737+
<< "\"";
738+
}
739+
remaining -= fileLength + 1;
740+
files += fileLength + 1;
741+
}
742+
#endif
743+
return IPC_OK();
744+
}
745+
700746
mozilla::ipc::IPCResult WebRenderBridgeParent::RecvUpdateResources(
701747
nsTArray<OpUpdateResource>&& aResourceUpdates,
702748
nsTArray<RefCountedShmem>&& aSmallShmems,

gfx/layers/wr/WebRenderBridgeParent.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ class WebRenderBridgeParent final : public PWebRenderBridgeParent,
123123
nsTArray<WebRenderParentCommand>&& commands) override;
124124
mozilla::ipc::IPCResult RecvGetSnapshot(PTextureParent* aTexture) override;
125125

126+
mozilla::ipc::IPCResult RecvValidateFontDescriptor(
127+
nsTArray<uint8_t>&& aData) override;
128+
126129
mozilla::ipc::IPCResult RecvSetLayersObserverEpoch(
127130
const LayersObserverEpoch& aChildEpoch) override;
128131

0 commit comments

Comments
 (0)