commit c168d366bdedb91b8f513de20be7f82627af5722
author Tom Sepez <tsepez@chromium.org> Wed Jul 24 17:52:29 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jul 24 17:52:29 2024 +0000
tree b95dc7209bafbc0b74d464eaa6da3f52d6843a53
parent 116971e7b71ee764399ac7734d936d1acbad2200

Resolve UNSAFE_TODO() in CFGAS_DefaultFontManager::GetFont()

There is already an explicit strlen() call, so converting to
a string view based upon an implicit strlen() isn't going to
force traversal of more characters.

-- add braces and return nullptr in two spots for clarity.

Bug: 42271776
Change-Id: I78c1b002eba2070dce03c5df7317e190f55678f2
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/122412
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

xfa/fgas/font/cfgas_defaultfontmanager.cpp

diff --git a/xfa/fgas/font/cfgas_defaultfontmanager.cpp b/xfa/fgas/font/cfgas_defaultfontmanager.cpp
index ccfb928..6472a78 100644
--- a/xfa/fgas/font/cfgas_defaultfontmanager.cpp
+++ b/xfa/fgas/font/cfgas_defaultfontmanager.cpp
@@ -22,41 +22,40 @@
   CFGAS_FontMgr* pFontMgr = CFGAS_GEModule::Get()->GetFontMgr();
   RetainPtr<CFGAS_GEFont> pFont = pFontMgr->LoadFont(
       wsFontName.c_str(), dwFontStyles, FX_CodePage::kFailure);
-  if (pFont)
+  if (pFont) {
     return pFont;
-
+  }
   const FGAS_FontInfo* pCurFont =
       FGAS_FontInfoByFontName(wsFontName.AsStringView());
-  if (!pCurFont || !pCurFont->pReplaceFont)
-    return pFont;
-
+  if (!pCurFont || !pCurFont->pReplaceFont) {
+    return nullptr;
+  }
   uint32_t dwStyle = 0;
   // TODO(dsinclair): Why doesn't this check the other flags?
-  if (FontStyleIsForceBold(dwFontStyles))
+  if (FontStyleIsForceBold(dwFontStyles)) {
     dwStyle |= FXFONT_FORCE_BOLD;
-  if (FontStyleIsItalic(dwFontStyles))
-    dwStyle |= FXFONT_ITALIC;
-
-  const char* pReplace = pCurFont->pReplaceFont;
-  int32_t iLength = pdfium::checked_cast<int32_t>(strlen(pReplace));
-  while (iLength > 0) {
-    const char* pNameText = pReplace;
-    while (*pNameText != ',' && iLength > 0) {
-      UNSAFE_TODO(pNameText++);
-      iLength--;
-    }
-    WideString wsReplace = WideString::FromASCII(
-        UNSAFE_TODO(ByteStringView(pReplace, pNameText - pReplace)));
-    pFont =
-        pFontMgr->LoadFont(wsReplace.c_str(), dwStyle, FX_CodePage::kFailure);
-    if (pFont)
-      break;
-
-    iLength--;
-    UNSAFE_TODO(pNameText++);
-    pReplace = pNameText;
   }
-  return pFont;
+  if (FontStyleIsItalic(dwFontStyles)) {
+    dwStyle |= FXFONT_ITALIC;
+  }
+  ByteStringView replace_view(pCurFont->pReplaceFont);
+  while (!replace_view.IsEmpty()) {
+    ByteStringView segment;
+    auto found = replace_view.Find(',');
+    if (found.has_value()) {
+      segment = replace_view.First(found.value());
+      replace_view = replace_view.Substr(found.value() + 1);
+    } else {
+      segment = replace_view;
+      replace_view = ByteStringView();
+    }
+    pFont = pFontMgr->LoadFont(WideString::FromASCII(segment).c_str(), dwStyle,
+                               FX_CodePage::kFailure);
+    if (pFont) {
+      return pFont;
+    }
+  }
+  return nullptr;
 }
 
 // static