From 7adf81f4047402e88cc2439374ee521ff2435cac Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 3 Nov 2026 10:33:18 -0800 Subject: [PATCH 01/34] Add tests for reentrant node calculations. Add a test verifying that re-entrant calculate script execution preserves the outer script's node array. Ensure dependencies referenced after nested calculations trigger recalculations upon modification. Follow-up to https://pdfium-review.googlesource.com/c/162130 Change-Id: I1d557383957c023a9acb3347e807a0dd05f7c98c Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/162150 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- .../xfa_specific/reentrant_calculate.in | 65 +++++++++++++++++++ .../reentrant_calculate_expected.txt | 2 + 2 files changed, 67 insertions(+) create mode 100644 testing/resources/javascript/xfa_specific/reentrant_calculate.in create mode 100644 testing/resources/javascript/xfa_specific/reentrant_calculate_expected.txt diff --git a/testing/resources/javascript/xfa_specific/reentrant_calculate.in b/testing/resources/javascript/xfa_specific/reentrant_calculate.in new file mode 100644 index 0000000000..aacfce3f32 --- /dev/null +++ b/testing/resources/javascript/xfa_specific/reentrant_calculate.in @@ -0,0 +1,65 @@ +{{header}} +{{include ../../xfa_catalog_1_0.fragment}} +{{include ../../xfa_object_2_0.fragment}} +{{include ../../xfa_preamble_3_0.fragment}} +{{include ../../xfa_config_4_0.fragment}} +{{object 5 0}} << + {{streamlen}} +>> +stream +
+
+ +endstream +endobj +{{include ../../xfa_locale_6_0.fragment}} +{{include ../../xfa_postamble_7_0.fragment}} +{{include ../../xfa_pages_8_0.fragment}} +{{xref}} +{{trailer}} +{{startxref}} +%%EOF diff --git a/testing/resources/javascript/xfa_specific/reentrant_calculate_expected.txt b/testing/resources/javascript/xfa_specific/reentrant_calculate_expected.txt new file mode 100644 index 0000000000..711b276a4f --- /dev/null +++ b/testing/resources/javascript/xfa_specific/reentrant_calculate_expected.txt @@ -0,0 +1,2 @@ +Alert: target: 1+2 +Alert: target: 1+99 From 145911ae9bdeeb4329e9a8c4240110a0ae917fa4 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 3 Nov 2026 10:45:59 -0800 Subject: [PATCH 02/34] Roll third_party/freetype/src/ 891f904e2..25167984d (36 commits) https://chromium.googlesource.com/chromium/src/third_party/freetype2.git/+log/891f904e2e7b..25167984d243 Created with: roll-dep third_party/freetype/src Change-Id: I1ddea28e185ddae8c8479147d10f38212810765b Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163070 Reviewed-by: Andy Phan Commit-Queue: Andy Phan Auto-Submit: Lei Zhang --- DEPS | 2 +- third_party/freetype/README.pdfium | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/DEPS b/DEPS index 1d0314ab3a..7e10cd0cd3 100644 --- a/DEPS +++ b/DEPS @@ -142,7 +142,7 @@ vars = { # Three lines of non-changing comments so that # the commit queue can handle CLs rolling freetype # and whatever else without interference from each other. - 'freetype_revision': '891f904e2e7b4289f615598cd4b69832f87d2f3a', + 'freetype_revision': '25167984d2435930f9178e8d9d2ca3bdfa71a4e7', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling GN CIPD package version # and whatever else without interference from each other. diff --git a/third_party/freetype/README.pdfium b/third_party/freetype/README.pdfium index 613e495c27..7e3e6cb401 100644 --- a/third_party/freetype/README.pdfium +++ b/third_party/freetype/README.pdfium @@ -1,7 +1,7 @@ Name: FreeType URL: https://gitlab.freedesktop.org/freetype/freetype -Version: VER-2-14-3-110 -Revision: 891f904e2e7b4289f615598cd4b69832f87d2f3a +Version: VER-2-14-3-146 +Revision: 25167984d2435930f9178e8d9d2ca3bdfa71a4e7 Update Mechanism: Manual CPEPrefix: cpe:/a:freetype:freetype:2.14.3 Security Critical: yes From 582f5879609922441689806d3e33cdbec67e4d7c Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 3 Nov 2026 10:46:30 -0800 Subject: [PATCH 03/34] Roll third_party/libunwind/src/ eb1ca4993..9e44ea2ff (5 commits) https://chromium.googlesource.com/external/github.com/llvm/llvm-project/libunwind.git/+log/eb1ca4993b53..9e44ea2ff4f5 Created with: roll-dep third_party/libunwind/src Change-Id: Ib857b3344d0a4bbae2ff6a83c6d9924164345075 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163031 Reviewed-by: Andy Phan Auto-Submit: Lei Zhang Commit-Queue: Andy Phan --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index 7e10cd0cd3..b327b6daed 100644 --- a/DEPS +++ b/DEPS @@ -196,7 +196,7 @@ vars = { # Three lines of non-changing comments so that # the commit queue can handle CLs rolling libunwind # and whatever else without interference from each other. - 'libunwind_revision': 'eb1ca4993b534b7d565daee144bbc7f9072b967b', + 'libunwind_revision': '9e44ea2ff4f53a60c2a3d2b6f511d7bab9ab12f0', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling libyuv # and whatever else without interference from each other. From caed91b4b45b9ecd298e6d1c3c93d612976065c1 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 3 Nov 2026 10:46:54 -0800 Subject: [PATCH 04/34] Roll third_party/libc++abi/src/ 09351f6ec..ce814c994 (6 commits) https://chromium.googlesource.com/external/github.com/llvm/llvm-project/libcxxabi.git/+log/09351f6ec00c..ce814c994055 Created with: roll-dep third_party/libc++abi/src Change-Id: I724c592d91baf287ceeb23f360a939387cad820e Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163050 Reviewed-by: Andy Phan Commit-Queue: Andy Phan Auto-Submit: Lei Zhang --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index b327b6daed..d214f0b4c6 100644 --- a/DEPS +++ b/DEPS @@ -188,7 +188,7 @@ vars = { # Three lines of non-changing comments so that # the commit queue can handle CLs rolling libc++abi # and whatever else without interference from each other. - 'libcxxabi_revision': '09351f6ec00c2b65f2d03585ae6da07098b024dc', + 'libcxxabi_revision': 'ce814c994055b107c631c310b1c668e3714c0704', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling libpng # and whatever else without interference from each other. From ba50ddfbcf94a234f9658ba29678004b45bdb22e Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 3 Nov 2026 10:56:05 -0800 Subject: [PATCH 05/34] Use GCed container wrappers for stack-local node collections Replace stack-local raw pointer collections and vectors of Persistent handles with GCed container wrappers (`GCedVector`, `GCedMap`, `GCedSet`) rooted via single `cppgc::Persistent<>` handles on the stack. Potentially avoids issues similiar to crbug.com/574304407. - In CopyContainer_SubformSet(): replace `subformArray`, `subformMapArray`, `nodeArray`, and `rgItemUnmatchList` - In CXFA_FFDocView::RunSubformIndexChange(): replace `seen`. - In CXFA_Node::ExecuteScript() and CFXJSE_Engine: replace the vector of individual Persistent handles with `GCedVector`. Change-Id: I9a63cea03b22751e66533a1fa67c53162296f42a Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/162050 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- fxjs/xfa/cfxjse_engine.cpp | 11 ++-- fxjs/xfa/cfxjse_engine.h | 12 ++-- xfa/fxfa/cxfa_ffdocview.cpp | 8 ++- xfa/fxfa/parser/cxfa_document.cpp | 63 +++++++++++-------- xfa/fxfa/parser/cxfa_node.cpp | 30 +++++---- .../parser/xfa_document_datamerger_imp.cpp | 2 +- xfa/fxfa/parser/xfa_document_datamerger_imp.h | 3 +- 7 files changed, 77 insertions(+), 52 deletions(-) diff --git a/fxjs/xfa/cfxjse_engine.cpp b/fxjs/xfa/cfxjse_engine.cpp index 5981872810..15f4bc94b9 100644 --- a/fxjs/xfa/cfxjse_engine.cpp +++ b/fxjs/xfa/cfxjse_engine.cpp @@ -15,6 +15,7 @@ #include "core/fxcrt/widetext_buffer.h" #include "fxjs/cjs_runtime.h" #include "fxjs/fxv8.h" +#include "fxjs/gc/containers.h" #include "fxjs/xfa/cfxjse_class.h" #include "fxjs/xfa/cfxjse_context.h" #include "fxjs/xfa/cfxjse_formcalc_context.h" @@ -193,8 +194,8 @@ CFXJSE_Engine::EventParamScope::~EventParamScope() { CFXJSE_Engine::ScriptNodeArrayScope::ScriptNodeArrayScope( CFXJSE_Engine* engine, - std::vector>* array) - : restorer_(&engine->script_node_array_, array) {} + cppgc::Persistent>&& array) + : restorer_(&engine->script_node_array_, std::move(array)) {} CFXJSE_Engine::ScriptNodeArrayScope::~ScriptNodeArrayScope() = default; @@ -960,9 +961,9 @@ v8::Local CFXJSE_Engine::GetOrCreateJSBindingFromMap( void CFXJSE_Engine::AddNodesOfRunScript(CXFA_Node* pNode) { if (script_node_array_ && - !pdfium::Contains(*script_node_array_, pNode, - &cppgc::Persistent::Get)) { - script_node_array_->emplace_back(pNode); + !pdfium::Contains(script_node_array_->storage, pNode, + &cppgc::Member::Get)) { + script_node_array_->storage.emplace_back(pNode); } } diff --git a/fxjs/xfa/cfxjse_engine.h b/fxjs/xfa/cfxjse_engine.h index 4cccc257b1..683b849d86 100644 --- a/fxjs/xfa/cfxjse_engine.h +++ b/fxjs/xfa/cfxjse_engine.h @@ -16,6 +16,7 @@ #include "core/fxcrt/mask.h" #include "core/fxcrt/unowned_ptr.h" #include "fxjs/cfx_isolate_wrapper.h" +#include "fxjs/gc/containers.h" #include "fxjs/xfa/cfxjse_context.h" #include "v8/include/cppgc/persistent.h" #include "v8/include/v8-forward.h" @@ -135,12 +136,11 @@ class CFXJSE_Engine final : public CFX_IsolateWrapper { public: ScriptNodeArrayScope(CFXJSE_Engine* engine, - std::vector>* array); + cppgc::Persistent>&& array); ~ScriptNodeArrayScope(); private: - AutoRestorer>>> - restorer_; + AutoRestorer>> restorer_; }; CXFA_Node* GetEventTarget() const { return target_; } @@ -166,6 +166,10 @@ class CFXJSE_Engine final : public CFX_IsolateWrapper { CFXJSE_Class* GetJseNormalClass() const { return js_class_; } CXFA_Document* GetDocument() const { return document_.Get(); } + GCedVector* GetScriptNodeArray() const { + return script_node_array_.Get(); + } + void AddNodesOfRunScript(CXFA_Node* pNode); void SetRunAtType(XFA_AttributeValue eRunAt) { run_at_type_ = eRunAt; } @@ -222,7 +226,7 @@ class CFXJSE_Engine final : public CFX_IsolateWrapper { cppgc::Persistent target_; UnownedPtr event_param_; std::vector> up_object_array_; - UnownedPtr>> script_node_array_; + cppgc::Persistent> script_node_array_; std::unique_ptr const node_helper_; std::unique_ptr const resolve_processor_; std::unique_ptr form_calc_context_; diff --git a/xfa/fxfa/cxfa_ffdocview.cpp b/xfa/fxfa/cxfa_ffdocview.cpp index 9b1665249d..d6c5d39afa 100644 --- a/xfa/fxfa/cxfa_ffdocview.cpp +++ b/xfa/fxfa/cxfa_ffdocview.cpp @@ -14,7 +14,7 @@ #include "core/fxcrt/fx_extension.h" #include "core/fxcrt/stl_util.h" #include "core/fxcrt/xml/cfx_xmlparser.h" -#include "fxjs/gc/container_trace.h" +#include "fxjs/gc/containers.h" #include "fxjs/xfa/cfxjse_engine.h" #include "fxjs/xfa/cjx_object.h" #include "xfa/fxfa/cxfa_ffapp.h" @@ -555,11 +555,13 @@ bool CXFA_FFDocView::RunLayout() { } void CXFA_FFDocView::RunSubformIndexChange() { - std::set seen; + cppgc::Persistent> seen = + cppgc::MakeGarbageCollected>( + doc_->GetHeap()->GetAllocationHandle()); while (!index_changed_subforms_.empty()) { CXFA_Node* pSubformNode = index_changed_subforms_.front(); index_changed_subforms_.pop_front(); - bool bInserted = seen.insert(pSubformNode).second; + bool bInserted = seen->storage.insert(pSubformNode).second; if (!bInserted || !pSubformNode->IsWidgetReady()) { continue; } diff --git a/xfa/fxfa/parser/cxfa_document.cpp b/xfa/fxfa/parser/cxfa_document.cpp index f3730f8105..d16de943ad 100644 --- a/xfa/fxfa/parser/cxfa_document.cpp +++ b/xfa/fxfa/parser/cxfa_document.cpp @@ -188,10 +188,11 @@ void MergeNode(CXFA_Node* pDestNode, CXFA_Node* pProtoNode) { } } -CXFA_Node* CloneOrMergeInstanceManager(CXFA_Document* document, - CXFA_Node* pFormParent, - CXFA_Node* pTemplateNode, - std::vector* subforms) { +CXFA_Node* CloneOrMergeInstanceManager( + CXFA_Document* document, + CXFA_Node* pFormParent, + CXFA_Node* pTemplateNode, + std::vector>* subforms) { WideString wsSubformName = pTemplateNode->JSObject()->GetCData(XFA_Attribute::Name); WideString wsInstMgrNodeName = L"_" + wsSubformName; @@ -874,14 +875,17 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, bool bUseInstanceManager = pFormParentNode->GetElementType() != XFA_Element::Area; CXFA_Node* pInstMgrNode = nullptr; - std::vector subformArray; - std::vector* pSearchArray = nullptr; + cppgc::Persistent> subform_array = + cppgc::MakeGarbageCollected>( + document->GetHeap()->GetAllocationHandle()); + std::vector>* pSearchArray = nullptr; if (!bOneInstance && (eType == XFA_Element::SubformSet || eType == XFA_Element::Subform)) { - pInstMgrNode = bUseInstanceManager ? CloneOrMergeInstanceManager( - document, pFormParentNode, - pTemplateNode, &subformArray) - : nullptr; + pInstMgrNode = bUseInstanceManager + ? CloneOrMergeInstanceManager(document, pFormParentNode, + pTemplateNode, + &subform_array->storage) + : nullptr; if (CXFA_Occur* pOccurTemplateNode = pTemplateNode->GetFirstChildByClass( XFA_Element::Occur)) { @@ -898,13 +902,14 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, } if (pInstMgrNode) { pInstMgrNode->SetInitializedFlagAndNotify(); - pSearchArray = &subformArray; + pSearchArray = &subform_array->storage; if (pFormParentNode->GetElementType() == XFA_Element::PageArea) { bOneInstance = true; - if (subformArray.empty()) { + if (subform_array->storage.empty()) { pSearchArray = nullptr; } - } else if (pTemplateNode->GetNameHash() == 0 && subformArray.empty()) { + } else if (pTemplateNode->GetNameHash() == 0 && + subform_array->storage.empty()) { pSearchArray = nullptr; } } @@ -931,8 +936,12 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, if (eType == XFA_Element::SubformSet || eType == XFA_Element::Area) { sNodeIterator.MoveToNext(); } else { - std::map subformMapArray; - std::vector nodeArray; + cppgc::Persistent> subform_map = + cppgc::MakeGarbageCollected>( + document->GetHeap()->GetAllocationHandle()); + cppgc::Persistent> node_array = + cppgc::MakeGarbageCollected>( + document->GetHeap()->GetAllocationHandle()); for (; iMax < 0 || iCurRepeatIndex < iMax; iCurRepeatIndex++) { bool bSelfMatch = false; XFA_AttributeValue eBindMatch = XFA_AttributeValue::None; @@ -952,14 +961,14 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, CreateDataBinding(pSubformNode, pDataNode, true); DCHECK(pSubformNode); - subformMapArray[pSubformNode] = pDataNode; - nodeArray.push_back(pSubformNode); + subform_map->storage[pSubformNode] = pDataNode; + node_array->storage.push_back(pSubformNode); } - for (CXFA_Node* pSubform : nodeArray) { + for (CXFA_Node* pSubform : node_array->storage) { CXFA_Node* pDataNode = nullptr; - auto it = subformMapArray.find(pSubform); - if (it != subformMapArray.end()) { + auto it = subform_map->storage.find(pSubform); + if (it != subform_map->storage.end()) { pDataNode = it->second; } for (CXFA_Node* pTemplateChild = pTemplateNode->GetFirstChild(); @@ -975,7 +984,7 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, } } } - subformMapArray.clear(); + subform_map->storage.clear(); } for (; iMax < 0 || iCurRepeatIndex < iMax; iCurRepeatIndex++) { @@ -1001,7 +1010,9 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, } std::vector rgItemMatchList; - std::vector rgItemUnmatchList; + cppgc::Persistent> item_unmatch_list = + cppgc::MakeGarbageCollected>( + document->GetHeap()->GetAllocationHandle()); for (CXFA_Node* pTemplateChild = pTemplateNode->GetFirstChild(); pTemplateChild; pTemplateChild = pTemplateChild->GetNextSibling()) { @@ -1027,10 +1038,10 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, rgItemMatchList.push_back(sNewRecord); } } else { - rgItemUnmatchList.push_back(pTemplateChild); + item_unmatch_list->storage.push_back(pTemplateChild); } } else { - rgItemUnmatchList.push_back(pTemplateChild); + item_unmatch_list->storage.push_back(pTemplateChild); } } } @@ -1053,7 +1064,7 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, false, true, true); } } - for (auto* unmatched : rgItemUnmatchList) { + for (CXFA_Node* unmatched : item_unmatch_list->storage) { document->DataMerge_CopyContainer(unmatched, pSubformSetNode, pDataScope, false, true, true); } @@ -1088,7 +1099,7 @@ CXFA_Node* CopyContainer_SubformSet(CXFA_Document* document, if (iCurRepeatIndex == 0 && !bAccessedDataDOM) { int32_t iLimit = iMax; if (pInstMgrNode && pTemplateNode->GetNameHash() == 0) { - iLimit = fxcrt::CollectionSize(subformArray); + iLimit = fxcrt::CollectionSize(subform_array->storage); if (iLimit < iMin) { iLimit = iInit; } diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp index 7c0e3e4676..00c2641ace 100644 --- a/xfa/fxfa/parser/cxfa_node.cpp +++ b/xfa/fxfa/parser/cxfa_node.cpp @@ -36,7 +36,7 @@ #include "core/fxge/dib/cfx_dibitmap.h" #include "core/fxge/fx_font.h" #include "fxjs/fxv8.h" -#include "fxjs/gc/container_trace.h" +#include "fxjs/gc/containers.h" #include "fxjs/xfa/cfxjse_engine.h" #include "fxjs/xfa/cfxjse_isolatetracker.h" #include "fxjs/xfa/cjx_node.h" @@ -2940,9 +2940,13 @@ CXFA_Node::BoolScriptResult CXFA_Node::ExecuteBoolScript( // pinned until ExecuteBoolScript() exits. const bool needs_ref_nodes = pEventParam->type_ == XFA_EVENT_InitCalculate || pEventParam->type_ == XFA_EVENT_Calculate; - std::vector> refNodes; - CFXJSE_Engine::ScriptNodeArrayScope node_array_scope( - context, needs_ref_nodes ? &refNodes : nullptr); + cppgc::Persistent> ref_nodes; + if (needs_ref_nodes) { + ref_nodes = cppgc::MakeGarbageCollected>( + doc->GetHeap()->GetAllocationHandle()); + } + CFXJSE_Engine::ScriptNodeArrayScope node_array_scope(context, + std::move(ref_nodes)); CFXJSE_Context::ExecutionResult exec_result; { @@ -2979,15 +2983,17 @@ CXFA_Node::BoolScriptResult CXFA_Node::ExecuteBoolScript( pDocView->AddValidateNode(this); } } - for (CXFA_Node* pRefNode : refNodes) { - if (pRefNode == this) { - continue; - } + if (context->GetScriptNodeArray()) { + for (CXFA_Node* pRefNode : context->GetScriptNodeArray()->storage) { + if (pRefNode == this) { + continue; + } - CJX_Object::CalcData* pGlobalData = - pRefNode->JSObject()->GetOrCreateCalcData(doc->GetHeap()); - if (!pdfium::Contains(pGlobalData->globals_, this)) { - pGlobalData->globals_.push_back(this); + CJX_Object::CalcData* pGlobalData = + pRefNode->JSObject()->GetOrCreateCalcData(doc->GetHeap()); + if (!pdfium::Contains(pGlobalData->globals_, this)) { + pGlobalData->globals_.push_back(this); + } } } } diff --git a/xfa/fxfa/parser/xfa_document_datamerger_imp.cpp b/xfa/fxfa/parser/xfa_document_datamerger_imp.cpp index 58b7fd5f22..43284051d3 100644 --- a/xfa/fxfa/parser/xfa_document_datamerger_imp.cpp +++ b/xfa/fxfa/parser/xfa_document_datamerger_imp.cpp @@ -44,7 +44,7 @@ CXFA_Node* XFA_NodeMerge_CloneOrMergeContainer( CXFA_Node* pFormParent, CXFA_Node* pTemplateNode, bool bRecursive, - std::vector* pSubformArray) { + std::vector>* pSubformArray) { if (pTemplateNode->GetPacketType() != XFA_PacketType::Template) { return nullptr; } diff --git a/xfa/fxfa/parser/xfa_document_datamerger_imp.h b/xfa/fxfa/parser/xfa_document_datamerger_imp.h index 643ee6bdbb..f3c5671fe9 100644 --- a/xfa/fxfa/parser/xfa_document_datamerger_imp.h +++ b/xfa/fxfa/parser/xfa_document_datamerger_imp.h @@ -9,6 +9,7 @@ #include +#include "v8/include/cppgc/member.h" #include "xfa/fxfa/fxfa_basic.h" class CXFA_Document; @@ -21,7 +22,7 @@ CXFA_Node* XFA_NodeMerge_CloneOrMergeContainer( CXFA_Node* pFormParent, CXFA_Node* pTemplateNode, bool bRecursive, - std::vector* pSubformArray); + std::vector>* pSubformArray); CXFA_Node* XFA_DataMerge_FindDataScope(CXFA_Node* pParentFormNode); CXFA_Node* XFA_DataMerge_FindFormDOMInstance(CXFA_Document* document, XFA_Element eType, From 777f7688648648bc5344295ef794153f791b68b9 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 3 Nov 2026 15:32:21 -0800 Subject: [PATCH 06/34] Update gn_version to 972691e86ce4828aaca38b5e44d7efe947fd1fd9 To make the new version of GN work, import //build/strict_deps_failures.gni to allow for check_includes_strict_exceptions. Change-Id: I6023faf2684da872062d5bd5bfa15036a7fa3d0e Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163092 Commit-Queue: Lei Zhang Reviewed-by: Tom Sepez --- .gn | 4 ++++ DEPS | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.gn b/.gn index 874f695a88..516519769d 100644 --- a/.gn +++ b/.gn @@ -4,6 +4,10 @@ import("//build/dotfile_settings.gni") +# This enables check_includes_strict_exceptions, which is necessary for V8 to +# pass `gn check` with `check_includes_strict = true`. +# TODO(thestig): Remove once V8 supports `check_includes_strict = true`. +import("//build/strict_deps_failures.gni") buildconfig = "//build/config/BUILDCONFIG.gn" # The python interpreter to use by default. On Windows, this will look diff --git a/DEPS b/DEPS index d214f0b4c6..55fde232fa 100644 --- a/DEPS +++ b/DEPS @@ -146,7 +146,7 @@ vars = { # Three lines of non-changing comments so that # the commit queue can handle CLs rolling GN CIPD package version # and whatever else without interference from each other. - 'gn_version': 'git_revision:841e072bef419ae1116477dc75e8c5a662c0212c', + 'gn_version': 'git_revision:98f6dd2be85d0eba8f31ee3ad1def5402383cff3', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling goldctl CIPD package version # and whatever else without interference from each other. From d67c95230de8010ac5bb716e178fc767a5d10fe0 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 3 Nov 2026 15:42:42 -0800 Subject: [PATCH 07/34] Add unit tests for FaxModule, IccTransform, FlateModule, and RLE Introduce unit tests for leaf-node codecs in core/fxcodec: - Add FaxModuleTest covering FaxModule::CreateDecoder boundary checks, Group 3 1-D decoding, BlackIs1 inverted buffers, Group 4 decoding, and FaxModule::FaxG4Decode. - Add IccTransformTest covering component validation, invalid profile handling, sRGB and Gray transforms, TranslateScanline, and Lab profile creation. - Add FlateModule.LZWDecode testing LZW stream decoding via FlateModule::FlateOrLZWDecode. - Add RLELeftoverCharacter and RLEScanlineDecoder testing trailing single-character run handling and scanline decoding offsets in BasicModule. AI-generated tests based upon analysis of x20 coverage data. Change-Id: I67320f04b397cb1ed907d8250c6dc238a957dd98 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163090 Commit-Queue: Tom Sepez Reviewed-by: Andy Phan --- core/fxcodec/BUILD.gn | 2 + core/fxcodec/basic/rle_unittest.cpp | 25 +++ core/fxcodec/fax/faxmodule_unittest.cpp | 172 ++++++++++++++++++++ core/fxcodec/flate/flatemodule_unittest.cpp | 14 ++ core/fxcodec/icc/icc_transform_unittest.cpp | 119 ++++++++++++++ 5 files changed, 332 insertions(+) create mode 100644 core/fxcodec/fax/faxmodule_unittest.cpp create mode 100644 core/fxcodec/icc/icc_transform_unittest.cpp diff --git a/core/fxcodec/BUILD.gn b/core/fxcodec/BUILD.gn index 36dad291b0..61205392fb 100644 --- a/core/fxcodec/BUILD.gn +++ b/core/fxcodec/BUILD.gn @@ -262,7 +262,9 @@ pdfium_unittest_source_set("unittests") { sources = [ "basic/a85_unittest.cpp", "basic/rle_unittest.cpp", + "fax/faxmodule_unittest.cpp", "flate/flatemodule_unittest.cpp", + "icc/icc_transform_unittest.cpp", "image_predictors_unittest.cpp", "jbig2/jbig2_bit_stream_unittest.cpp", "jbig2/jbig2_image_unittest.cpp", diff --git a/core/fxcodec/basic/rle_unittest.cpp b/core/fxcodec/basic/rle_unittest.cpp index b3434437e5..68d166418a 100644 --- a/core/fxcodec/basic/rle_unittest.cpp +++ b/core/fxcodec/basic/rle_unittest.cpp @@ -11,7 +11,9 @@ #include "core/fpdfapi/parser/fpdf_parser_decode.h" #include "core/fxcodec/basic/basicmodule.h" #include "core/fxcodec/data_and_bytes_consumed.h" +#include "core/fxcodec/scanlinedecoder.h" #include "core/fxcrt/data_vector.h" +#include "core/fxcrt/span.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -98,3 +100,26 @@ TEST(fxcodec, RLEFullLengthInputs) { EXPECT_THAT(result.data, ElementsAreArray(src_buf_4)); } } + +// Check that trailing leftover unmatched characters following matched runs +// are encoded properly. +TEST(fxcodec, RLELeftoverCharacter) { + const uint8_t src_buf[] = {1, 1, 2}; + DataVector dest_buf = BasicModule::RunLengthEncode(src_buf); + DataAndBytesConsumed result = RunLengthDecode(dest_buf); + EXPECT_THAT(result.data, ElementsAreArray(src_buf)); +} + +TEST(fxcodec, RLEScanlineDecoder) { + const uint8_t src_buf[] = {1, 1, 2}; + DataVector encoded = BasicModule::RunLengthEncode(src_buf); + std::unique_ptr decoder = + BasicModule::CreateRunLengthDecoder(encoded, 3, 1, 1, 8); + ASSERT_TRUE(decoder); + EXPECT_EQ(3, decoder->GetWidth()); + EXPECT_EQ(1, decoder->GetHeight()); + pdfium::span line = decoder->GetScanline(0); + ASSERT_FALSE(line.empty()); + EXPECT_THAT(line.first(3u), ElementsAreArray(src_buf)); + EXPECT_GT(decoder->GetSrcOffset(), 0u); +} diff --git a/core/fxcodec/fax/faxmodule_unittest.cpp b/core/fxcodec/fax/faxmodule_unittest.cpp new file mode 100644 index 0000000000..673e14d755 --- /dev/null +++ b/core/fxcodec/fax/faxmodule_unittest.cpp @@ -0,0 +1,172 @@ +// Copyright 2026 The PDFium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcodec/fax/faxmodule.h" + +#include + +#include +#include + +#include "core/fxcodec/scanlinedecoder.h" +#include "core/fxcrt/data_vector.h" +#include "core/fxcrt/span.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace fxcodec { + +namespace { + +// Real-world 44x46 1-D CCITT Group 3 encoded fax stream (containing 38 rows). +constexpr uint8_t kFax1DData[] = { + 0x26, 0xa0, 0x86, 0x80, 0xde, 0x08, 0x1e, 0x11, 0xe1, 0x87, 0x84, + 0x08, 0x30, 0xf4, 0x13, 0x7a, 0x4d, 0xf0, 0xef, 0x4d, 0xbe, 0xdb, + 0xff, 0xff, 0x6f, 0xfd, 0xaf, 0x0c, 0x2f, 0x1f, 0xff, 0x21, 0xb3, + 0x4c, 0x82, 0xe3, 0x30, 0x44, 0xe7, 0xc1, 0x1a, 0x09, 0xc2, 0x05, + 0xe1, 0x05, 0xe9, 0x7a, 0x5e, 0x97, 0xa5, 0xff, 0xa5, 0xfe, 0x0a, + 0x3f, 0x5f, 0xfa, 0xff, 0xfb, 0x5c, 0x9a, 0xaa, 0x30, 0xbe, 0xd2, + 0x7a, 0xdb, 0x4a, 0xd7, 0x0d, 0xa4, 0xda, 0x5b, 0x61, 0x28, 0xf0, + 0xc4, 0x26, 0xb2, 0x16, 0x81, 0x85, 0x00, 0x10, 0x01, 0x00, 0x0a, +}; + +// Real-world 548x238 2-D CCITT Group 4 encoded fax stream (containing +// 177 rows). +constexpr uint8_t kFaxG4Data[] = { + 0x2e, 0x5e, 0x46, 0xc0, 0xd2, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xc2, 0x20, 0x6c, 0x5e, 0x45, 0xc4, 0x5e, 0x3f, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xfc, 0x5e, 0x40, 0x40, 0x5e, 0x44, +}; + +} // namespace + +TEST(FaxModuleTest, BadDimensions) { + // Reject non-positive dimensions. + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 0, 10, 0, false, false, false, 0, 0)); + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 10, 0, 0, false, false, false, 0, 0)); + EXPECT_FALSE( + FaxModule::CreateDecoder({}, -1, 10, 0, false, false, false, 0, 0)); + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 10, -1, 0, false, false, false, 0, 0)); + + // Reject dimensions exceeding `kFaxMaxImageDimension`. + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 65536, 10, 0, false, false, false, 0, 0)); + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 10, 65536, 0, false, false, false, 0, 0)); + + // Reject invalid Columns and Rows overrides. + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 10, 10, 0, false, false, false, -1, 10)); + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 10, 10, 0, false, false, false, 65536, 10)); + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 10, 10, 0, false, false, false, 10, -1)); + EXPECT_FALSE( + FaxModule::CreateDecoder({}, 10, 10, 0, false, false, false, 10, 65536)); +} + +TEST(FaxModuleTest, EmptyInput) { + auto decoder = + FaxModule::CreateDecoder({}, 10, 10, 0, false, false, false, 0, 0); + ASSERT_TRUE(decoder); + EXPECT_EQ(10, decoder->GetWidth()); + EXPECT_EQ(10, decoder->GetHeight()); + EXPECT_TRUE(decoder->GetScanline(0).empty()); + EXPECT_EQ(0u, decoder->GetSrcOffset()); +} + +TEST(FaxModuleTest, Group3_1D_Decode) { + auto decoder = FaxModule::CreateDecoder(kFax1DData, 44, 46, /*K=*/0, + /*EndOfLine=*/false, + /*EncodedByteAlign=*/false, + /*BlackIs1=*/false, + /*Columns=*/44, /*Rows=*/46); + ASSERT_TRUE(decoder); + EXPECT_EQ(44, decoder->GetWidth()); + EXPECT_EQ(46, decoder->GetHeight()); + + // Data has 38 rows. + for (int i = 0; i < 38; ++i) { + pdfium::span line = decoder->GetScanline(i); + EXPECT_FALSE(line.empty()) << "Failed at line " << i; + EXPECT_GE(line.size(), 6u); + } + + // Subsequent rows past the end of stream data should be empty. + EXPECT_TRUE(decoder->GetScanline(38).empty()); + + EXPECT_GT(decoder->GetSrcOffset(), 0u); + + // Rewinding to line 0 should succeed and produce line 0 again. + pdfium::span line0 = decoder->GetScanline(0); + EXPECT_FALSE(line0.empty()); +} + +TEST(FaxModuleTest, Group3_1D_BlackIs1) { + auto normal_decoder = FaxModule::CreateDecoder( + kFax1DData, 44, 46, /*K=*/0, /*EndOfLine=*/false, + /*EncodedByteAlign=*/false, /*BlackIs1=*/false, /*Columns=*/44, + /*Rows=*/46); + auto black1_decoder = FaxModule::CreateDecoder( + kFax1DData, 44, 46, /*K=*/0, /*EndOfLine=*/false, + /*EncodedByteAlign=*/false, /*BlackIs1=*/true, /*Columns=*/44, + /*Rows=*/46); + ASSERT_TRUE(normal_decoder); + ASSERT_TRUE(black1_decoder); + + for (int i = 0; i < 38; ++i) { + pdfium::span normal_line = normal_decoder->GetScanline(i); + pdfium::span black1_line = black1_decoder->GetScanline(i); + ASSERT_FALSE(normal_line.empty()) << "Line " << i; + ASSERT_EQ(normal_line.size(), black1_line.size()) << "Line " << i; + for (size_t j = 0; j < normal_line.size(); ++j) { + EXPECT_EQ(normal_line[j], static_cast(~black1_line[j])) + << "Mismatch at line " << i << " byte " << j; + } + } +} + +TEST(FaxModuleTest, Group4_Decode) { + auto decoder = FaxModule::CreateDecoder(kFaxG4Data, 548, 238, /*K=*/-1, + /*EndOfLine=*/false, + /*EncodedByteAlign=*/false, + /*BlackIs1=*/false, + /*Columns=*/548, /*Rows=*/238); + ASSERT_TRUE(decoder); + EXPECT_EQ(548, decoder->GetWidth()); + EXPECT_EQ(238, decoder->GetHeight()); + + // Data has 177 rows. + for (int i = 0; i < 177; ++i) { + pdfium::span line = decoder->GetScanline(i); + EXPECT_FALSE(line.empty()) << "Failed at line " << i; + EXPECT_GE(line.size(), 69u); + } + + // Subsequent rows past the end of stream data should be empty. + EXPECT_TRUE(decoder->GetScanline(177).empty()); + + EXPECT_GT(decoder->GetSrcOffset(), 0u); +} + +TEST(FaxModuleTest, Group4_FaxG4Decode) { + constexpr int kWidth = 548; + constexpr int kHeight = 238; + constexpr int kPitch = (kWidth + 7) / 8; + DataVector dest_buf(kPitch * kHeight); + + uint32_t end_bitpos = FaxModule::FaxG4Decode( + kFaxG4Data, /*starting_bitpos=*/0, kWidth, kHeight, kPitch, dest_buf); + EXPECT_GT(end_bitpos, 0u); + EXPECT_LE(end_bitpos, sizeof(kFaxG4Data) * 8); +} + +} // namespace fxcodec diff --git a/core/fxcodec/flate/flatemodule_unittest.cpp b/core/fxcodec/flate/flatemodule_unittest.cpp index 480b8ba9a5..261a6f891d 100644 --- a/core/fxcodec/flate/flatemodule_unittest.cpp +++ b/core/fxcodec/flate/flatemodule_unittest.cpp @@ -72,3 +72,17 @@ TEST(FlateModule, Encode) { ++i; } } + +TEST(FlateModule, LZWDecode) { + // Encoded LZW stream for "AB": + // Codes: 256 (Clear Table), 65 ('A'), 66 ('B'), 257 (EOD) + static constexpr uint8_t kLzwInput[] = {0x80, 0x10, 0x48, 0x50, 0x10}; + static constexpr uint8_t kExpected[] = {'A', 'B'}; + + DataAndBytesConsumed result = FlateModule::FlateOrLZWDecode( + /*bLZW=*/true, kLzwInput, /*bEarlyChange=*/false, /*predictor=*/0, + /*Colors=*/0, /*BitsPerComponent=*/0, /*Columns=*/0, + /*estimated_size=*/0); + EXPECT_THAT(result.data, ElementsAreArray(kExpected)); + EXPECT_EQ(5u, result.bytes_consumed); +} diff --git a/core/fxcodec/icc/icc_transform_unittest.cpp b/core/fxcodec/icc/icc_transform_unittest.cpp new file mode 100644 index 0000000000..2dbb52d0d6 --- /dev/null +++ b/core/fxcodec/icc/icc_transform_unittest.cpp @@ -0,0 +1,119 @@ +// Copyright 2026 The PDFium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcodec/icc/icc_transform.h" + +#include + +#include +#include + +#include "core/fxcrt/data_vector.h" +#include "core/fxcrt/span.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace fxcodec { + +namespace { + +DataVector SerializeProfile(cmsHPROFILE profile) { + cmsUInt32Number bytes_needed = 0; + if (!cmsSaveProfileToMem(profile, nullptr, &bytes_needed) || + bytes_needed == 0) { + return {}; + } + DataVector buffer(bytes_needed); + if (!cmsSaveProfileToMem(profile, buffer.data(), &bytes_needed)) { + return {}; + } + return buffer; +} + +} // namespace + +TEST(IccTransformTest, IsValidIccComponents) { + EXPECT_TRUE(IccTransform::IsValidIccComponents(1)); + EXPECT_TRUE(IccTransform::IsValidIccComponents(3)); + EXPECT_TRUE(IccTransform::IsValidIccComponents(4)); + + EXPECT_FALSE(IccTransform::IsValidIccComponents(-1)); + EXPECT_FALSE(IccTransform::IsValidIccComponents(0)); + EXPECT_FALSE(IccTransform::IsValidIccComponents(2)); + EXPECT_FALSE(IccTransform::IsValidIccComponents(5)); + EXPECT_FALSE(IccTransform::IsValidIccComponents(16)); +} + +TEST(IccTransformTest, InvalidProfiles) { + EXPECT_FALSE(IccTransform::CreateTransformSRGB({})); + + static constexpr uint8_t kGarbage[] = {0x00, 0x01, 0x02, 0x03, 0x04}; + EXPECT_FALSE(IccTransform::CreateTransformSRGB(kGarbage)); +} + +TEST(IccTransformTest, SRGBProfile) { + cmsHPROFILE profile = cmsCreate_sRGBProfile(); + ASSERT_TRUE(profile); + DataVector profile_data = SerializeProfile(profile); + cmsCloseProfile(profile); + ASSERT_FALSE(profile_data.empty()); + + std::unique_ptr transform = + IccTransform::CreateTransformSRGB(profile_data); + ASSERT_TRUE(transform); + + // Translate single pixel float components. + static constexpr float kSrcRgb[3] = {1.0f, 0.0f, 0.0f}; + float dst_rgb[3] = {0.0f, 0.0f, 0.0f}; + transform->Translate(kSrcRgb, dst_rgb); + EXPECT_NEAR(dst_rgb[0], 1.0f, 0.05f); + EXPECT_NEAR(dst_rgb[1], 0.0f, 0.05f); + EXPECT_NEAR(dst_rgb[2], 0.0f, 0.05f); + + // Translate scanline with 8-bit samples. + // Note that destination is BGR_8 format per cmsCreateTransform. + static constexpr uint8_t kSrcPixel[3] = {255, 0, 0}; + uint8_t dst_pixel[3] = {0, 0, 0}; + transform->TranslateScanline(dst_pixel, kSrcPixel, 1); + EXPECT_EQ(dst_pixel[0], 0); // B + EXPECT_EQ(dst_pixel[1], 0); // G + EXPECT_EQ(dst_pixel[2], 255); // R +} + +TEST(IccTransformTest, LabProfile) { + // cmsCreateTransform currently fails for Lab profiles because + // IccTransform::CreateTransformSRGB constructs `srcFormat` with + // `BYTES_SH(0)` but omits `FLOAT_SH(1)`, so lcms2 does not recognize it + // as `TYPE_Lab_DBL`. + cmsHPROFILE profile = cmsCreateLab4Profile(nullptr); + ASSERT_TRUE(profile); + DataVector profile_data = SerializeProfile(profile); + cmsCloseProfile(profile); + ASSERT_FALSE(profile_data.empty()); + + EXPECT_FALSE(IccTransform::CreateTransformSRGB(profile_data)); +} + +TEST(IccTransformTest, GrayProfile) { + cmsToneCurve* gamma = cmsBuildGamma(nullptr, 2.2); + ASSERT_TRUE(gamma); + cmsHPROFILE profile = cmsCreateGrayProfile(nullptr, gamma); + cmsFreeToneCurve(gamma); + ASSERT_TRUE(profile); + DataVector profile_data = SerializeProfile(profile); + cmsCloseProfile(profile); + ASSERT_FALSE(profile_data.empty()); + + std::unique_ptr transform = + IccTransform::CreateTransformSRGB(profile_data); + ASSERT_TRUE(transform); + + static constexpr float kGrayWhite[1] = {1.0f}; + float dst_rgb[3] = {0.0f, 0.0f, 0.0f}; + transform->Translate(kGrayWhite, dst_rgb); + EXPECT_NEAR(dst_rgb[0], 1.0f, 0.05f); + EXPECT_NEAR(dst_rgb[1], 1.0f, 0.05f); + EXPECT_NEAR(dst_rgb[2], 1.0f, 0.05f); +} + +} // namespace fxcodec From 3fe8d0814a101347702dbf2c67cc8257101a1680 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 3 Nov 2026 16:22:44 -0800 Subject: [PATCH 08/34] Update siso_version to dfc32e4edca158cff2b64a1658ee3940f0116b3a Change-Id: I5bd21a1e8506d412ce112b5530b1fcfa439550ab Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163093 Commit-Queue: Andy Phan Auto-Submit: Lei Zhang Reviewed-by: Andy Phan --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index 55fde232fa..e51f3204e9 100644 --- a/DEPS +++ b/DEPS @@ -240,7 +240,7 @@ vars = { # Three lines of non-changing comments so that # the commit queue can handle CLs rolling siso # and whatever else without interference from each other. - 'siso_version': 'git_revision:c574896b272b40aac6e44c228ee0b471ea7e3d7d', + 'siso_version': 'git_revision:dfc32e4edca158cff2b64a1658ee3940f0116b3a', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling skia # and whatever else without interference from each other. From 02829e813fdc1b4c521611b70c27c616b510cb4b Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 4 Nov 2026 12:18:06 -0800 Subject: [PATCH 09/34] Hold CXFA_Event pointers via Persistent vector Hold individual cppgc::Persistent elements inside a standard vector. This avoids GC-heap allocations while still rooting each event across script execution in CXFA_Node::ProcessEvent(). Also rename GetEventByActivity() to GetEventsByActivity() to reflect returning a collection, and skip any events detached from the node during iteration. Fixed: 575091549 Change-Id: Ia42d22b237f497bd50da3aaad516c8e783346d64 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163151 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- xfa/fxfa/cxfa_ffwidgethandler.cpp | 2 +- xfa/fxfa/parser/cxfa_node.cpp | 15 +++++++++------ xfa/fxfa/parser/cxfa_node.h | 6 ++++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/xfa/fxfa/cxfa_ffwidgethandler.cpp b/xfa/fxfa/cxfa_ffwidgethandler.cpp index 92f6233646..3b447ebc8a 100644 --- a/xfa/fxfa/cxfa_ffwidgethandler.cpp +++ b/xfa/fxfa/cxfa_ffwidgethandler.cpp @@ -208,7 +208,7 @@ bool CXFA_FFWidgetHandler::HasEvent(CXFA_Node* pNode, default: break; } - return !pNode->GetEventByActivity(kXFAEventActivity[eEventType], false) + return !pNode->GetEventsByActivity(kXFAEventActivity[eEventType], false) .empty(); } diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp index 00c2641ace..303d1f910d 100644 --- a/xfa/fxfa/parser/cxfa_node.cpp +++ b/xfa/fxfa/parser/cxfa_node.cpp @@ -2565,13 +2565,16 @@ XFA_EventError CXFA_Node::ProcessEvent(CXFA_FFDocView* pDocView, return XFA_EventError::kNotExist; } - std::vector eventArray = - GetEventByActivity(iActivity, pEventParam->is_form_ready_); + std::vector> event_array = + GetEventsByActivity(iActivity, pEventParam->is_form_ready_); bool first = true; XFA_EventError iRet = XFA_EventError::kNotExist; - for (CXFA_Event* event : eventArray) { + for (const auto& event : event_array) { + if (event->GetParent() != this) { + continue; + } XFA_EventError result = - ProcessEventInternal(pDocView, iActivity, event, pEventParam); + ProcessEventInternal(pDocView, iActivity, event.Get(), pEventParam); if (first || result == XFA_EventError::kSuccess) { iRet = result; } @@ -3221,10 +3224,10 @@ CFX_RectF CXFA_Node::GetUIMargin() { bottom.value_or(0.0)); } -std::vector CXFA_Node::GetEventByActivity( +std::vector> CXFA_Node::GetEventsByActivity( XFA_AttributeValue iActivity, bool bIsFormReady) { - std::vector events; + std::vector> events; for (CXFA_Node* node : GetNodeListForType(XFA_Element::Event)) { auto* event = static_cast(node); if (event->GetActivity() != iActivity) { diff --git a/xfa/fxfa/parser/cxfa_node.h b/xfa/fxfa/parser/cxfa_node.h index 919e9f158b..cfaea5893f 100644 --- a/xfa/fxfa/parser/cxfa_node.h +++ b/xfa/fxfa/parser/cxfa_node.h @@ -25,6 +25,7 @@ #include "core/fxge/dib/fx_dib.h" #include "fxjs/gc/gced_tree_node_mixin.h" #include "v8/include/cppgc/member.h" +#include "v8/include/cppgc/persistent.h" #include "v8/include/cppgc/visitor.h" #include "xfa/fxfa/cxfa_ffwidget_type.h" #include "xfa/fxfa/fxfa.h" @@ -326,8 +327,9 @@ class CXFA_Node : public CXFA_Object, public GCedTreeNodeMixin { void SetWidgetReady() { is_widget_ready_ = true; } bool IsWidgetReady() const { return is_widget_ready_; } - std::vector GetEventByActivity(XFA_AttributeValue iActivity, - bool bIsFormReady); + std::vector> GetEventsByActivity( + XFA_AttributeValue iActivity, + bool bIsFormReady); void ResetData(); void StartWidgetLayout(CXFA_FFDoc* doc, From 08d3057179125d2df69268ec30ae9d0fd7e0053d Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 4 Nov 2026 12:55:43 -0800 Subject: [PATCH 10/34] Avoid an assert() in skia/BUILD.gn for riscv Add an extra no-op branch, like in Chromium's skia/BUILD.gn. Change-Id: I73b2031367328667efce0c65816d82040b15f913 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163190 Commit-Queue: Lei Zhang Reviewed-by: Tom Sepez --- skia/BUILD.gn | 2 ++ 1 file changed, 2 insertions(+) diff --git a/skia/BUILD.gn b/skia/BUILD.gn index 03345e474c..d174c8f363 100644 --- a/skia/BUILD.gn +++ b/skia/BUILD.gn @@ -584,6 +584,8 @@ skia_source_set("skia_opts") { # Conditional and empty body needed to avoid assert below } else if (current_cpu == "mipsel" || current_cpu == "mips64el") { cflags += [ "-fomit-frame-pointer" ] + } else if (current_cpu == "riscv64") { + # Conditional and empty body needed to avoid assert() below. } else { assert(false, "Unsupported target CPU " + current_cpu) } From 711dc444aaaca46c7331296b52bd2c60ebca1fb5 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 4 Nov 2026 15:58:15 -0800 Subject: [PATCH 11/34] Introduce FxDataPartitionType concept Define the FxDataPartitionType concept to de-duplicate type requirements across FxPartitionAllocAllocator, FxAllocAllocator, DataVector, and FixedSizeDataVector. Change-Id: I1b8474838a47d9643614622f9b3e49ebf9d91ee1 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163230 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- core/fxcrt/data_vector.h | 2 +- core/fxcrt/fixed_size_data_vector.h | 4 +--- core/fxcrt/fx_memory_wrappers.h | 11 ++++++----- core/fxcrt/fx_memory_wrappers_unittest.cpp | 5 +++++ 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/core/fxcrt/data_vector.h b/core/fxcrt/data_vector.h index d09cf2d14d..d51b7a2dac 100644 --- a/core/fxcrt/data_vector.h +++ b/core/fxcrt/data_vector.h @@ -13,7 +13,7 @@ namespace fxcrt { -template +template using DataVector = std::vector>; template diff --git a/core/fxcrt/fixed_size_data_vector.h b/core/fxcrt/fixed_size_data_vector.h index e72e88d6dd..409fb88259 100644 --- a/core/fxcrt/fixed_size_data_vector.h +++ b/core/fxcrt/fixed_size_data_vector.h @@ -22,9 +22,7 @@ namespace fxcrt { // accessible using spans. // It can either initialize its data with zeros, or leave its data // uninitialized. -template - requires(std::is_arithmetic::value || std::is_enum::value || - IsFXDataPartitionException::value) +template class FixedSizeDataVector { public: FixedSizeDataVector() = default; diff --git a/core/fxcrt/fx_memory_wrappers.h b/core/fxcrt/fx_memory_wrappers.h index e34af90f8d..2ad6700bf6 100644 --- a/core/fxcrt/fx_memory_wrappers.h +++ b/core/fxcrt/fx_memory_wrappers.h @@ -27,13 +27,16 @@ struct IsFXDataPartitionException : std::false_type {}; template <> \ struct IsFXDataPartitionException : std::true_type {} +template +concept FxDataPartitionType = std::is_arithmetic_v || std::is_enum_v || + IsFXDataPartitionException::value; + // Allocators for mapping STL containers onto Partition Alloc. // Otherwise, replacing e.g. the FX_AllocUninit/FX_Free pairs with STL may // undo some of the nice segregation that we get from PartitionAlloc. template struct FxPartitionAllocAllocator { - static_assert(std::is_arithmetic::value || std::is_enum::value || - IsFXDataPartitionException::value, + static_assert(FxDataPartitionType, "Only numeric types allowed in this partition"); using value_type = T; @@ -85,9 +88,7 @@ struct FxPartitionAllocAllocator { // Used to put backing store for std::vector<> and such into the // general partition, ensuring they contain data only. -template - requires(std::is_arithmetic::value || std::is_enum::value || - IsFXDataPartitionException::value) +template using FxAllocAllocator = FxPartitionAllocAllocator; diff --git a/core/fxcrt/fx_memory_wrappers_unittest.cpp b/core/fxcrt/fx_memory_wrappers_unittest.cpp index 763836116f..82d4f5e481 100644 --- a/core/fxcrt/fx_memory_wrappers_unittest.cpp +++ b/core/fxcrt/fx_memory_wrappers_unittest.cpp @@ -23,6 +23,11 @@ struct OnlyNumbers { FX_DATA_PARTITION_EXCEPTION(OnlyNumbers); +static_assert(FxDataPartitionType); +static_assert(FxDataPartitionType); +static_assert(FxDataPartitionType); +static_assert(!FxDataPartitionType); + TEST(fxcrt, FxFreeDeleter) { std::unique_ptr empty(nullptr); std::unique_ptr thing(FX_Alloc(int, 1)); From 1799c8d533c83a39ffb1af7a25e6a4463c685ed4 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 4 Nov 2026 16:50:36 -0800 Subject: [PATCH 12/34] Seed ObjectTreeTraverser from both trailer and catalog Traverse CPDF_Document::GetRoot() in addition to the trailer in ObjectTreeTraverser. The document root catalog can diverge from the trailer's /Root entry (such as after cross-reference rebuilds), which could otherwise cause objects reachable only from the catalog to be missed. Fixed: 581323820 Change-Id: Ic8f8292b61ba7c98ca82a5543e6e3b28b6d15e36 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163094 Commit-Queue: Tom Sepez Reviewed-by: Andy Phan --- .../parser/object_tree_traversal_util.cpp | 44 +++++++++++++------ .../parser/object_tree_traversal_util.h | 5 +-- ...bject_tree_traversal_util_embeddertest.cpp | 21 +++++++++ 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/core/fpdfapi/parser/object_tree_traversal_util.cpp b/core/fpdfapi/parser/object_tree_traversal_util.cpp index eebcff1e4e..e2118603d6 100644 --- a/core/fpdfapi/parser/object_tree_traversal_util.cpp +++ b/core/fpdfapi/parser/object_tree_traversal_util.cpp @@ -29,18 +29,10 @@ class ObjectTreeTraverser { : document_(document) { const CPDF_Parser* parser = document_->GetParser(); const CPDF_Dictionary* trailer = parser ? parser->GetTrailer() : nullptr; - const CPDF_Dictionary* root = trailer ? trailer : document_->GetRoot(); - const uint32_t root_object_number = - trailer ? parser->GetTrailerObjectNumber() : root->GetObjNum(); - // If `root` is a trailer, then it may not have an object number, as many - // trailers are inlined. - if (root_object_number) { - referenced_objects_[root_object_number] = 1; - object_number_map_[root] = root_object_number; + if (trailer) { + EnqueueRoot(pdfium::WrapRetain(trailer), + parser->GetTrailerObjectNumber()); } - - object_queue_.push(pdfium::WrapRetain(root)); - seen_objects_.insert(root); } ~ObjectTreeTraverser() = default; @@ -56,8 +48,19 @@ class ObjectTreeTraverser { uint32_t referenced_object_number; }; - std::vector GetReferenceEntries() { - std::vector reference_entries; + void EnqueueRoot(RetainPtr root, + uint32_t root_object_number) { + if (!root || !seen_objects_.insert(root.Get()).second) { + return; + } + if (root_object_number) { + referenced_objects_[root_object_number] = 1; + object_number_map_[root.Get()] = root_object_number; + } + object_queue_.push(std::move(root)); + } + + void DrainObjectQueue(std::vector* reference_entries) { while (!object_queue_.empty()) { RetainPtr current_object = object_queue_.front(); object_queue_.pop(); @@ -94,7 +97,7 @@ class ObjectTreeTraverser { // Unlike the other object types, CPDF_Reference can point at nullptr. if (referenced_object) { CHECK(referenced_object_number); - reference_entries.push_back( + reference_entries->push_back( {ref_object_number, referenced_object_number}); PushNewObject(ref_object, referenced_object); } @@ -115,6 +118,19 @@ class ObjectTreeTraverser { } } } + } + + std::vector GetReferenceEntries() { + std::vector reference_entries; + DrainObjectQueue(&reference_entries); + + // Also traverse the catalog if it was not reached from the trailer + // (e.g. if the trailer's `/Root` was missing, null, or diverged). + const CPDF_Dictionary* catalog = document_->GetRoot(); + if (catalog && !pdfium::Contains(seen_objects_, catalog)) { + EnqueueRoot(pdfium::WrapRetain(catalog), catalog->GetObjNum()); + DrainObjectQueue(&reference_entries); + } return reference_entries; } diff --git a/core/fpdfapi/parser/object_tree_traversal_util.h b/core/fpdfapi/parser/object_tree_traversal_util.h index e9db96dce9..f8f2ff9563 100644 --- a/core/fpdfapi/parser/object_tree_traversal_util.h +++ b/core/fpdfapi/parser/object_tree_traversal_util.h @@ -11,9 +11,8 @@ class CPDF_Document; -// Traverses `document` starting with its trailer, if it has one, or starting at -// the catalog, which always exists. The trailer should have a reference to the -// catalog. The traversal avoids cycles. +// Traverses `document` starting with its trailer, if it has one, and also from +// the catalog if not reached from the trailer. The traversal avoids cycles. // Returns all the PDF objects (not CPDF_Objects) the traversal reached as a set // of object numbers. std::set GetObjectsWithReferences(const CPDF_Document* document); diff --git a/core/fpdfapi/parser/object_tree_traversal_util_embeddertest.cpp b/core/fpdfapi/parser/object_tree_traversal_util_embeddertest.cpp index ed0c8e1697..a129c4f06e 100644 --- a/core/fpdfapi/parser/object_tree_traversal_util_embeddertest.cpp +++ b/core/fpdfapi/parser/object_tree_traversal_util_embeddertest.cpp @@ -9,6 +9,8 @@ #include #include "core/fpdfapi/parser/cpdf_document.h" +#include "core/fpdfapi/parser/cpdf_null.h" +#include "core/fpdfapi/parser/cpdf_parser.h" #include "testing/embedder_test.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -110,3 +112,22 @@ TEST_F(ObjectTreeTraversalUtilEmbedderTest, std::set referenced_objects = GetObjectsWithMultipleReferences(doc); EXPECT_TRUE(referenced_objects.empty()); } + +TEST_F(ObjectTreeTraversalUtilEmbedderTest, + GetObjectsWithReferencesTrailerWithoutRoot) { + ASSERT_TRUE(OpenDocument("hello_world.pdf")); + CPDF_Document* doc = GetCPDFDocument(document()); + doc->GetParser()->GetMutableTrailerForTesting()->SetNewFor("Root"); + std::set referenced_objects = GetObjectsWithReferences(doc); + EXPECT_THAT(referenced_objects, + UnorderedElementsAreArray({1, 2, 3, 4, 5, 6})); +} + +TEST_F(ObjectTreeTraversalUtilEmbedderTest, + GetObjectsWithMultipleReferencesTrailerWithoutRoot) { + ASSERT_TRUE(OpenDocument("hello_world_2_pages.pdf")); + CPDF_Document* doc = GetCPDFDocument(document()); + doc->GetParser()->GetMutableTrailerForTesting()->SetNewFor("Root"); + std::set referenced_objects = GetObjectsWithMultipleReferences(doc); + EXPECT_THAT(referenced_objects, UnorderedElementsAreArray({5, 6, 7})); +} From 6d464783524ed6a75d3d7cf9f32a524bfbd5cb33 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 4 Nov 2026 17:00:16 -0800 Subject: [PATCH 13/34] Use CJS_Runtime::BlockingScope to handle nested blocking calls CJS_Runtime::~CJS_Runtime() guards against the runtime being destroyed while a blocking call-out (app.alert(), app.response(), ...) is still on the stack with CHECK(!IsBlocking()). Nested blocking call-outs (for example, /AA /WC WillClose JavaScript executed during teardown that calls app.beep()) previously caused the inner EndBlock() call to reset the blocking state, allowing the runtime to be destroyed prematurely and leading to use-after-free conditions. Replace BeginBlock() and EndBlock() with a CJS_Runtime::BlockingScope class backed by AutoRestorer. This restores the prior blocking state upon leaving each scope, ensuring outer blocking states remain active until the outermost call completes. Fixed: 581296829 Change-Id: If91c82a4cedba263bdf77d6fa6ce6c3e07324c5e Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163150 Reviewed-by: Andy Phan Commit-Queue: Tom Sepez --- fxjs/cjs_app.cpp | 12 ++++-------- fxjs/cjs_document.cpp | 15 +++++---------- fxjs/cjs_publicmethods_embeddertest.cpp | 24 ++++++++++++++++++++++++ fxjs/cjs_runtime.cpp | 5 +++++ fxjs/cjs_runtime.h | 16 ++++++++++++++-- 5 files changed, 52 insertions(+), 20 deletions(-) diff --git a/fxjs/cjs_app.cpp b/fxjs/cjs_app.cpp index d18663d595..39d2ae6358 100644 --- a/fxjs/cjs_app.cpp +++ b/fxjs/cjs_app.cpp @@ -262,11 +262,10 @@ CJS_Result CJS_App::alert(CJS_Runtime* pRuntime, swTitle = JSGetStringFromID(JSMessage::kAlert); } - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); pFormFillEnv->KillFocusAnnot({}); v8::Local ret = pRuntime->NewNumber( pFormFillEnv->JS_appAlert(swMsg, swTitle, iType, iIcon)); - pRuntime->EndBlock(); return CJS_Result::Success(ret); } @@ -282,9 +281,8 @@ CJS_Result CJS_App::beep(CJS_Runtime* pRuntime, type = pRuntime->ToInt32Reentrant(params[0]); } - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); pRuntime->GetFormFillEnv()->JS_appBeep(type); - pRuntime->EndBlock(); return CJS_Result::Success(); } @@ -488,10 +486,9 @@ CJS_Result CJS_App::mailMsg(CJS_Runtime* pRuntime, cMsg = pRuntime->ToWideStringReentrant(newParams[5]); } - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); pRuntime->GetFormFillEnv()->JS_docmailForm(pdfium::span(), bUI, cTo, cSubject, cCc, cBcc, cMsg); - pRuntime->EndBlock(); return CJS_Result::Success(); } @@ -583,10 +580,9 @@ CJS_Result CJS_App::response(CJS_Runtime* pRuntime, static constexpr int kMaxBytes = kMaxWideChars * sizeof(uint16_t); auto buffer = FixedSizeDataVector::Zeroed(kMaxBytes); - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); int byte_length = pRuntime->GetFormFillEnv()->JS_appResponse( swQuestion, swTitle, swDefault, swLabel, bPassword, buffer.span()); - pRuntime->EndBlock(); if (byte_length < 0 || byte_length > kMaxBytes) { return CJS_Result::Failure(JSMessage::kParamTooLongError); diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp index ee73eac1c9..1086e6cc82 100644 --- a/fxjs/cjs_document.cpp +++ b/fxjs/cjs_document.cpp @@ -364,10 +364,9 @@ CJS_Result CJS_Document::mailDoc(CJS_Runtime* pRuntime, return CJS_Result::Failure(JSMessage::kBadObjectError); } - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); form_fill_env_->JS_docmailForm(pdfium::span(), bUI, cTo, cSubject, cCc, cBcc, cMsg); - pRuntime->EndBlock(); return CJS_Result::Success(); } @@ -424,10 +423,9 @@ CJS_Result CJS_Document::mailForm(CJS_Runtime* pRuntime, return CJS_Result::Failure(WideString::FromASCII("Bad FDF format.")); } - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); form_fill_env_->JS_docmailForm(sTextBuf.unsigned_span(), bUI, cTo, cSubject, cCc, cBcc, cMsg); - pRuntime->EndBlock(); return CJS_Result::Success(); } @@ -657,9 +655,8 @@ CJS_Result CJS_Document::submitForm(CJS_Runtime* pRuntime, CPDF_InteractiveForm* pPDFForm = GetCoreInteractiveForm(); if (pRuntime->GetArrayLength(aFields) == 0 && bEmpty) { if (pPDFForm->CheckRequiredFields(nullptr, true)) { - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); GetSDKInteractiveForm()->SubmitForm(strURL); - pRuntime->EndBlock(); } return CJS_Result::Success(); } @@ -679,9 +676,8 @@ CJS_Result CJS_Document::submitForm(CJS_Runtime* pRuntime, } if (pPDFForm->CheckRequiredFields(&fieldObjects, true)) { - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); GetSDKInteractiveForm()->SubmitFields(strURL, fieldObjects, true, !bFDF); - pRuntime->EndBlock(); } return CJS_Result::Success(); } @@ -1494,10 +1490,9 @@ CJS_Result CJS_Document::gotoNamedDest( CPDF_Dest dest(std::move(dest_array)); std::vector positions = dest.GetScrollPositionArray(); - pRuntime->BeginBlock(); + CJS_Runtime::BlockingScope scope(pRuntime); form_fill_env_->DoGoToAction(dest.GetDestPageIndex(document), dest.GetZoomMode(), positions); - pRuntime->EndBlock(); return CJS_Result::Success(); } diff --git a/fxjs/cjs_publicmethods_embeddertest.cpp b/fxjs/cjs_publicmethods_embeddertest.cpp index 3c99d765f3..037ee10973 100644 --- a/fxjs/cjs_publicmethods_embeddertest.cpp +++ b/fxjs/cjs_publicmethods_embeddertest.cpp @@ -256,3 +256,27 @@ TEST_F(CJSPublicMethodsEmbedderTest, AFNumberKeystroke) { handler->SetRCForTest(nullptr); handler->SetStrChangeForTest(nullptr); } + +TEST_F(CJSPublicMethodsEmbedderTest, BlockingScope) { + v8::Isolate::Scope isolate_scope(isolate()); + v8::HandleScope handle_scope(isolate()); + v8::Context::Scope context_scope(GetV8Context()); + + ASSERT_TRUE(OpenDocument("calculate.pdf")); + ScopedPage page = LoadScopedPage(0); + ASSERT_TRUE(page); + + CJS_Runtime runtime( + CPDFSDKFormFillEnvironmentFromFPDFFormHandle(form_handle())); + EXPECT_FALSE(runtime.IsBlocking()); + { + CJS_Runtime::BlockingScope scope1(&runtime); + EXPECT_TRUE(runtime.IsBlocking()); + { + CJS_Runtime::BlockingScope scope2(&runtime); + EXPECT_TRUE(runtime.IsBlocking()); + } + EXPECT_TRUE(runtime.IsBlocking()); + } + EXPECT_FALSE(runtime.IsBlocking()); +} diff --git a/fxjs/cjs_runtime.cpp b/fxjs/cjs_runtime.cpp index dc0403ad41..47b7938b7d 100644 --- a/fxjs/cjs_runtime.cpp +++ b/fxjs/cjs_runtime.cpp @@ -94,6 +94,11 @@ CJS_Runtime::~CJS_Runtime() { } } +CJS_Runtime::BlockingScope::BlockingScope(CJS_Runtime* runtime) + : restorer_(&runtime->blocking_, true) {} + +CJS_Runtime::BlockingScope::~BlockingScope() = default; + void CJS_Runtime::DefineJSObjects() { v8::Isolate::Scope isolate_scope(GetIsolate()); v8::HandleScope handle_scope(GetIsolate()); diff --git a/fxjs/cjs_runtime.h b/fxjs/cjs_runtime.h index 84cf6f3fd9..6489985896 100644 --- a/fxjs/cjs_runtime.h +++ b/fxjs/cjs_runtime.h @@ -12,6 +12,7 @@ #include #include +#include "core/fxcrt/autorestorer.h" #include "core/fxcrt/cfx_timer.h" #include "core/fxcrt/observed_ptr.h" #include "fxjs/cfxjs_engine.h" @@ -44,8 +45,19 @@ class CJS_Runtime final : public IJS_Runtime, bool AddEventToSet(const FieldEvent& event); void RemoveEventFromSet(const FieldEvent& event); - void BeginBlock() { blocking_ = true; } - void EndBlock() { blocking_ = false; } + class [[maybe_unused, nodiscard]] BlockingScope { + public: + FX_STACK_ALLOCATED(); + + explicit BlockingScope(CJS_Runtime* runtime); + BlockingScope(const BlockingScope&) = delete; + BlockingScope& operator=(const BlockingScope&) = delete; + ~BlockingScope(); + + private: + AutoRestorer restorer_; + }; + bool IsBlocking() const { return blocking_; } // Attempt to convert the |value| into a number. If successful the number From 892b0f37e9da2d89a922652f0ddfe5183ad69c98 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 4 Nov 2026 17:41:11 -0800 Subject: [PATCH 14/34] Support in-class IsDataPartitionSafe member in FxDataPartitionType Allow types to opt into FxDataPartitionType by declaring an in-class static constexpr bool IsDataPartitionSafe. This is easier to use with class templates. -- Opt in template FX_LAB_STRUCT for arithmetic types T. Change-Id: I50652e55abce24694e7ce0ddad3be757834c5f06 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163250 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- core/fxcrt/fx_memory_wrappers.h | 3 ++- core/fxcrt/fx_memory_wrappers_unittest.cpp | 14 ++++++++++++++ core/fxge/dib/fx_dib.h | 3 +++ core/fxge/dib/fx_dib_unittest.cpp | 4 ++++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/core/fxcrt/fx_memory_wrappers.h b/core/fxcrt/fx_memory_wrappers.h index 2ad6700bf6..e834ff24d6 100644 --- a/core/fxcrt/fx_memory_wrappers.h +++ b/core/fxcrt/fx_memory_wrappers.h @@ -29,7 +29,8 @@ struct IsFXDataPartitionException : std::false_type {}; template concept FxDataPartitionType = std::is_arithmetic_v || std::is_enum_v || - IsFXDataPartitionException::value; + IsFXDataPartitionException::value || + requires { requires T::kIsDataPartitionSafe; }; // Allocators for mapping STL containers onto Partition Alloc. // Otherwise, replacing e.g. the FX_AllocUninit/FX_Free pairs with STL may diff --git a/core/fxcrt/fx_memory_wrappers_unittest.cpp b/core/fxcrt/fx_memory_wrappers_unittest.cpp index 82d4f5e481..03e11af286 100644 --- a/core/fxcrt/fx_memory_wrappers_unittest.cpp +++ b/core/fxcrt/fx_memory_wrappers_unittest.cpp @@ -19,6 +19,17 @@ struct OnlyNumbers { int y; }; +struct TaggedType { + static constexpr bool kIsDataPartitionSafe = true; + int a; +}; + +template +struct TemplatedTaggedType { + static constexpr bool kIsDataPartitionSafe = std::is_arithmetic_v; + T a; +}; + } // namespace FX_DATA_PARTITION_EXCEPTION(OnlyNumbers); @@ -26,6 +37,9 @@ FX_DATA_PARTITION_EXCEPTION(OnlyNumbers); static_assert(FxDataPartitionType); static_assert(FxDataPartitionType); static_assert(FxDataPartitionType); +static_assert(FxDataPartitionType); +static_assert(FxDataPartitionType>); +static_assert(!FxDataPartitionType>); static_assert(!FxDataPartitionType); TEST(fxcrt, FxFreeDeleter) { diff --git a/core/fxge/dib/fx_dib.h b/core/fxge/dib/fx_dib.h index c74a3e9409..9914f1031a 100644 --- a/core/fxge/dib/fx_dib.h +++ b/core/fxge/dib/fx_dib.h @@ -9,6 +9,7 @@ #include +#include #include #include "core/fxcrt/compiler_specific.h" @@ -105,6 +106,8 @@ struct FX_CMYK_STRUCT { template struct FX_LAB_STRUCT { + static constexpr bool kIsDataPartitionSafe = std::is_arithmetic_v; + T lightness_star = 0; T a_star = 0; T b_star = 0; diff --git a/core/fxge/dib/fx_dib_unittest.cpp b/core/fxge/dib/fx_dib_unittest.cpp index 4063f83df8..d9cc5f031f 100644 --- a/core/fxge/dib/fx_dib_unittest.cpp +++ b/core/fxge/dib/fx_dib_unittest.cpp @@ -6,8 +6,12 @@ #include +#include "core/fxcrt/fx_memory_wrappers.h" #include "testing/gtest/include/gtest/gtest.h" +static_assert(FxDataPartitionType>); +static_assert(FxDataPartitionType>); + TEST(FxDibTest, ArgbToBGRAStruct) { FX_BGRA_STRUCT white = ArgbToBGRAStruct(0xffffffff); EXPECT_EQ(0xff, white.blue); From c75f0bd272822d6d86e7f350484c9136d4a53949 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 4 Nov 2026 17:49:22 -0800 Subject: [PATCH 15/34] Rework FX_DATA_PARTITION_EXCEPTION macro Migrate all internal structs to the kIsDataPartitionSafe mechanism. Rename to macro to FX_EXTERNAL_DATA_PARTITION_SAFE to provide an escape hatch for third-party types. -- Test with an incomplete forward-declared type. Change-Id: Ida14b7a8940c8afd6503d1acbfaf61985fa90901 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163251 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- core/fpdfapi/page/cpdf_indexedcs.h | 4 ++-- core/fpdftext/cpdf_textpage.h | 5 ++--- core/fxcodec/jbig2/jbig2_arith_decoder.h | 4 ++-- core/fxcrt/fx_memory_wrappers.h | 9 ++++----- core/fxcrt/fx_memory_wrappers_unittest.cpp | 16 +++++++--------- core/fxge/apple/cquartz_2d.cpp | 2 +- core/fxge/dib/cstretchengine.h | 7 ++++--- core/fxge/skia/fx_skia_device.h | 6 +++--- 8 files changed, 25 insertions(+), 28 deletions(-) diff --git a/core/fpdfapi/page/cpdf_indexedcs.h b/core/fpdfapi/page/cpdf_indexedcs.h index 917bd999c5..3052d6eefd 100644 --- a/core/fpdfapi/page/cpdf_indexedcs.h +++ b/core/fpdfapi/page/cpdf_indexedcs.h @@ -11,16 +11,16 @@ #include "core/fpdfapi/page/cpdf_basedcs.h" #include "core/fxcrt/data_vector.h" -#include "core/fxcrt/fx_memory_wrappers.h" #include "core/fxcrt/retain_ptr.h" class CPDF_Document; struct IndexedColorMinMax { + static constexpr bool kIsDataPartitionSafe = true; + float min; float max; }; -FX_DATA_PARTITION_EXCEPTION(IndexedColorMinMax); class CPDF_IndexedCS final : public CPDF_BasedCS { public: diff --git a/core/fpdftext/cpdf_textpage.h b/core/fpdftext/cpdf_textpage.h index e941a19b52..9269509297 100644 --- a/core/fpdftext/cpdf_textpage.h +++ b/core/fpdftext/cpdf_textpage.h @@ -16,7 +16,6 @@ #include "core/fpdfapi/page/cpdf_pageobjectholder.h" #include "core/fxcrt/data_vector.h" #include "core/fxcrt/fx_coordinates.h" -#include "core/fxcrt/fx_memory_wrappers.h" #include "core/fxcrt/unowned_ptr.h" #include "core/fxcrt/widestring.h" #include "core/fxcrt/widetext_buffer.h" @@ -26,12 +25,12 @@ class CPDF_Page; class CPDF_TextObject; struct TextPageCharSegment { + static constexpr bool kIsDataPartitionSafe = true; + int index; int count; }; -FX_DATA_PARTITION_EXCEPTION(TextPageCharSegment); - class CPDF_TextPage { public: enum class CharType : uint8_t { diff --git a/core/fxcodec/jbig2/jbig2_arith_decoder.h b/core/fxcodec/jbig2/jbig2_arith_decoder.h index 7d0d8180e8..9c441856b9 100644 --- a/core/fxcodec/jbig2/jbig2_arith_decoder.h +++ b/core/fxcodec/jbig2/jbig2_arith_decoder.h @@ -9,13 +9,14 @@ #include -#include "core/fxcrt/fx_memory_wrappers.h" #include "core/fxcrt/unowned_ptr.h" class CJBig2_BitStream; class JBig2ArithCtx { public: + static constexpr bool kIsDataPartitionSafe = true; + struct JBig2ArithQe { uint16_t Qe; uint8_t NMPS; @@ -35,7 +36,6 @@ class JBig2ArithCtx { bool mps_ = false; uint8_t i_ = 0; }; -FX_DATA_PARTITION_EXCEPTION(JBig2ArithCtx); class CJBig2_ArithDecoder { public: diff --git a/core/fxcrt/fx_memory_wrappers.h b/core/fxcrt/fx_memory_wrappers.h index e834ff24d6..0bd5e5f3f0 100644 --- a/core/fxcrt/fx_memory_wrappers.h +++ b/core/fxcrt/fx_memory_wrappers.h @@ -20,11 +20,10 @@ struct FxFreeDeleter { template struct IsFXDataPartitionException : std::false_type {}; -// Use with caution. No further checks are made to see if `T` is appropriate -// for the Data Partition (e.g. no pointers, strings, vtables, etc.). This -// declaration must occur in the top-level namespace. -#define FX_DATA_PARTITION_EXCEPTION(T) \ - template <> \ +// Use ONLY for external / third-party types (e.g. Skia, Apple CoreGraphics) +// whose definitions cannot include `kIsDataPartitionSafe`. +#define FX_EXTERNAL_DATA_PARTITION_SAFE(T) \ + template <> \ struct IsFXDataPartitionException : std::true_type {} template diff --git a/core/fxcrt/fx_memory_wrappers_unittest.cpp b/core/fxcrt/fx_memory_wrappers_unittest.cpp index 03e11af286..91e4338025 100644 --- a/core/fxcrt/fx_memory_wrappers_unittest.cpp +++ b/core/fxcrt/fx_memory_wrappers_unittest.cpp @@ -14,14 +14,10 @@ namespace { -struct OnlyNumbers { - int x; - int y; -}; - struct TaggedType { static constexpr bool kIsDataPartitionSafe = true; - int a; + int x; + int y; }; template @@ -30,13 +26,15 @@ struct TemplatedTaggedType { T a; }; +struct IncompleteType; + } // namespace -FX_DATA_PARTITION_EXCEPTION(OnlyNumbers); +FX_EXTERNAL_DATA_PARTITION_SAFE(IncompleteType); static_assert(FxDataPartitionType); static_assert(FxDataPartitionType); -static_assert(FxDataPartitionType); +static_assert(FxDataPartitionType); static_assert(FxDataPartitionType); static_assert(FxDataPartitionType>); static_assert(!FxDataPartitionType>); @@ -78,7 +76,7 @@ TEST(fxcrt, FxStringAllocator) { } TEST(fxcrt, FxAllocAllocatorStructException) { - std::vector> vec; + std::vector> vec; vec.push_back({42, 73}); EXPECT_EQ(vec.back().x, 42); EXPECT_EQ(vec.back().y, 73); diff --git a/core/fxge/apple/cquartz_2d.cpp b/core/fxge/apple/cquartz_2d.cpp index 1bf7f02dd5..401fb8dc47 100644 --- a/core/fxge/apple/cquartz_2d.cpp +++ b/core/fxge/apple/cquartz_2d.cpp @@ -21,7 +21,7 @@ #error Expected CGFLOAT_IS_DOUBLE to be defined by CoreGraphics headers #endif -FX_DATA_PARTITION_EXCEPTION(CGPoint); +FX_EXTERNAL_DATA_PARTITION_SAFE(CGPoint); void* CQuartz2D::CreateGraphics(const RetainPtr& pBitmap) { if (!pBitmap) { diff --git a/core/fxge/dib/cstretchengine.h b/core/fxge/dib/cstretchengine.h index b75c5a5edc..147fb180c7 100644 --- a/core/fxge/dib/cstretchengine.h +++ b/core/fxge/dib/cstretchengine.h @@ -17,7 +17,6 @@ #include "core/fxcrt/data_vector.h" #include "core/fxcrt/fixed_size_data_vector.h" #include "core/fxcrt/fx_coordinates.h" -#include "core/fxcrt/fx_memory_wrappers.h" #include "core/fxcrt/fx_system.h" #include "core/fxcrt/raw_span.h" #include "core/fxcrt/retain_ptr.h" @@ -41,6 +40,8 @@ class ScanlineComposerIface; // count, a short run, or a group whose window does not fit -- take the // unchanged per-column path. struct StretchHorzGroup { + static constexpr bool kIsDataPartitionSafe = true; + size_t src_byte_base; // Byte offset of the source window in the row. // Per-tap gather indices into the window; see the kernel. std::array, 2> idx; @@ -49,7 +50,6 @@ struct StretchHorzGroup { uint32_t taps; // 1 or 2. bool wide; // Whether the source window needs 32 bytes, not 16. }; -FX_DATA_PARTITION_EXCEPTION(StretchHorzGroup); // A run of consecutive groups covering 4 * group_count consecutive // destination columns from dest_col (an absolute column index, the same @@ -57,11 +57,12 @@ FX_DATA_PARTITION_EXCEPTION(StretchHorzGroup); // [first_group, first_group + group_count) of horz_groups_. The kernel // handles a whole run per call. struct StretchHorzRun { + static constexpr bool kIsDataPartitionSafe = true; + int dest_col; size_t first_group; size_t group_count; }; -FX_DATA_PARTITION_EXCEPTION(StretchHorzRun); class CStretchEngine { public: diff --git a/core/fxge/skia/fx_skia_device.h b/core/fxge/skia/fx_skia_device.h index e29f76876d..cf78f5b8de 100644 --- a/core/fxge/skia/fx_skia_device.h +++ b/core/fxge/skia/fx_skia_device.h @@ -33,9 +33,9 @@ struct CFX_TextRenderOptions; struct TextCharPos; // Assumes Skia is not going to add non-data members to its fundamental types. -FX_DATA_PARTITION_EXCEPTION(SkColor4f); -FX_DATA_PARTITION_EXCEPTION(SkPoint); -FX_DATA_PARTITION_EXCEPTION(SkRSXform); +FX_EXTERNAL_DATA_PARTITION_SAFE(SkColor4f); +FX_EXTERNAL_DATA_PARTITION_SAFE(SkPoint); +FX_EXTERNAL_DATA_PARTITION_SAFE(SkRSXform); class CFX_SkiaDeviceDriver final : public RenderDeviceDriverIface { public: From 13af78b6e94df06c7e9969cbad0c4a5a59385d39 Mon Sep 17 00:00:00 2001 From: Levi Zim Date: Wed, 4 Nov 2026 18:38:38 -0800 Subject: [PATCH 16/34] Fix riscv64 build error for cstretchengine_hwy.cpp This CL fixes a build error of pdfium on riscv64. Use Rebind so the store descriptor matches the fractional-LMUL vector returned by U8FromU32 on RVV. Bug: 559426535 Change-Id: I6b067bc8d56b4212ed8291d49f9ce2fb0a8a73c9 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/161950 Commit-Queue: Lei Zhang Reviewed-by: Lei Zhang Reviewed-by: Andy Phan --- AUTHORS | 1 + core/fxge/dib/cstretchengine_hwy.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 3c4802c4ae..a7bfeb888c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -38,6 +38,7 @@ Justin Pierce Ke Liu Kei Kamikawa LN Liberda +Levi Zim Luật Nguyễn Manuel Geißer Matt Jolly diff --git a/core/fxge/dib/cstretchengine_hwy.cpp b/core/fxge/dib/cstretchengine_hwy.cpp index 064c618fbf..0a39493033 100644 --- a/core/fxge/dib/cstretchengine_hwy.cpp +++ b/core/fxge/dib/cstretchengine_hwy.cpp @@ -171,7 +171,7 @@ inline void DoGroupImpl(const StretchHorzGroup& group, // arm of `if constexpr` is still instantiated for every kBpp, so the // kBpp == 4 arm's static_assert would otherwise fire for kBpp == 1 too. if constexpr (kBpp == 1) { - const hn::Full32 d8x4; + const hn::Rebind d8x4; hn::StoreU(hn::U8FromU32(b16), d8x4, dest.data()); } else { const auto g16 = hn::And(hn::ShiftRight<16>(g), m255); From 9976ca5f8d1dc7e2a1571a468be9251e775cc815 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 4 Nov 2026 19:24:34 -0800 Subject: [PATCH 17/34] Add BlockingScope in AlertIfPossible() AlertIfPossible() calls the embedder's JS_appAlert() without entering a blocking scope. In Chrome, the alert forces a style and layout update which can destroy the PDF plug-in and CJS_Runtime. Because blocking_ was not set, the CHECK(!IsBlocking()) guard in ~CJS_Runtime() did not fire, allowing the runtime to be freed while script frames remain. Fixed: 580846556 Change-Id: I974fa5124419fd6a702c26caf22b1c6dbd298d36 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163095 Reviewed-by: Andy Phan Commit-Queue: Tom Sepez --- fxjs/cjs_event_context.h | 1 + fxjs/cjs_publicmethods.cpp | 1 + fxjs/cjs_publicmethods_embeddertest.cpp | 43 +++++++++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/fxjs/cjs_event_context.h b/fxjs/cjs_event_context.h index 3b43ad83ad..f78b3ed6ca 100644 --- a/fxjs/cjs_event_context.h +++ b/fxjs/cjs_event_context.h @@ -109,6 +109,7 @@ class CJS_EventContext final : public IJS_EventContext { CPDFSDK_FormFillEnvironment* GetFormFillEnv() const { return form_fill_env_.Get(); } + CJS_Runtime* GetRuntime() const { return runtime_.get(); } CJS_Field* SourceField(); CJS_Field* TargetField(); diff --git a/fxjs/cjs_publicmethods.cpp b/fxjs/cjs_publicmethods.cpp index 92ee10d1b2..bcd974a1a1 100644 --- a/fxjs/cjs_publicmethods.cpp +++ b/fxjs/cjs_publicmethods.cpp @@ -96,6 +96,7 @@ void AlertIfPossible(CJS_EventContext* context, const WideString& wsMsg) { CPDFSDK_FormFillEnvironment* pFormFillEnv = context->GetFormFillEnv(); if (pFormFillEnv) { + CJS_Runtime::BlockingScope scope(context->GetRuntime()); pFormFillEnv->JS_appAlert(wsMsg, wsCaller, JSPLATFORM_ALERT_BUTTON_OK, JSPLATFORM_ALERT_ICON_STATUS); } diff --git a/fxjs/cjs_publicmethods_embeddertest.cpp b/fxjs/cjs_publicmethods_embeddertest.cpp index 037ee10973..1d56261c39 100644 --- a/fxjs/cjs_publicmethods_embeddertest.cpp +++ b/fxjs/cjs_publicmethods_embeddertest.cpp @@ -8,7 +8,9 @@ #include "fpdfsdk/cpdfsdk_helpers.h" #include "fxjs/cjs_event_context.h" #include "fxjs/cjs_publicmethods.h" +#include "testing/embedder_test_mock_delegate.h" #include "testing/external_engine_embedder_test.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "v8/include/v8-container.h" #include "v8/include/v8-context.h" @@ -280,3 +282,44 @@ TEST_F(CJSPublicMethodsEmbedderTest, BlockingScope) { } EXPECT_FALSE(runtime.IsBlocking()); } + +TEST_F(CJSPublicMethodsEmbedderTest, AlertIfPossibleBlocking) { + v8::Isolate::Scope isolate_scope(isolate()); + v8::HandleScope handle_scope(isolate()); + v8::Context::Scope context_scope(GetV8Context()); + + ASSERT_TRUE(OpenDocument("calculate.pdf")); + ScopedPage page = LoadScopedPage(0); + ASSERT_TRUE(page); + + CJS_Runtime runtime( + CPDFSDKFormFillEnvironmentFromFPDFFormHandle(form_handle())); + auto& delegate = SetOwnedDelegate(); + EXPECT_CALL(delegate, Alert).WillOnce([&] { + EXPECT_TRUE(runtime.IsBlocking()); + return 0; + }); + + runtime.NewEventContext(); + auto* handler = runtime.GetCurrentEventContext(); + + WideString val = L"50"; + bool valid = true; + handler->SetValueForTest(&val); + handler->SetRCForTest(&valid); + + v8::LocalVector params(runtime.GetIsolate()); + params.push_back(runtime.NewBoolean(true)); + params.push_back(runtime.NewNumber(100)); + params.push_back(runtime.NewBoolean(true)); + params.push_back(runtime.NewNumber(200)); + + EXPECT_FALSE(runtime.IsBlocking()); + CJS_Result ret = CJS_PublicMethods::AFRange_Validate(&runtime, params); + EXPECT_FALSE(runtime.IsBlocking()); + EXPECT_FALSE(valid); + EXPECT_FALSE(ret.HasError()); + + handler->SetValueForTest(nullptr); + handler->SetRCForTest(nullptr); +} From 77b522a2c7c7d63068e9d03114663c4774318760 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 5 Nov 2026 08:51:45 -0800 Subject: [PATCH 18/34] Use FixedSizeDataVector in cfx_dibbase_skia.cpp Add FixedSizeDataVector::release() to transfer ownership of the underlying allocation to the caller and reset the vector to be empty. Use FixedSizeDataVector in CreateSkiaImageFrom24BppDib() and CreateSkiaImageFromTransformedDib() instead of manually allocating raw buffers with FX_TryAlloc() and constructing an UNSAFE_BUFFERS() span. Along the way, avoid unnecessary buffer zeroing and buffer size variables. Also add comments to better explain memory management. This CL is AI generated. Change-Id: Iad60969662116270d3e11b987402c6debc5a8a4b Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163270 Reviewed-by: Tom Sepez Commit-Queue: Lei Zhang --- core/fxcrt/fixed_size_data_vector.h | 8 +++++ .../fxcrt/fixed_size_data_vector_unittest.cpp | 16 +++++++++ core/fxge/skia/cfx_dibbase_skia.cpp | 36 +++++++++---------- 3 files changed, 41 insertions(+), 19 deletions(-) diff --git a/core/fxcrt/fixed_size_data_vector.h b/core/fxcrt/fixed_size_data_vector.h index 409fb88259..fcda7691f3 100644 --- a/core/fxcrt/fixed_size_data_vector.h +++ b/core/fxcrt/fixed_size_data_vector.h @@ -94,6 +94,14 @@ class FixedSizeDataVector { bool empty() const { return size_ == 0; } size_t size() const { return size_; } + // Releases ownership of the underlying data buffer and resets this vector to + // be empty. The caller is responsible for freeing the returned pointer with + // FX_Free(). + [[nodiscard]] T* release() { + size_ = 0; + return data_.release(); + } + // Implicit access to data via span. operator pdfium::span() { return span(); } operator pdfium::span() const { return span(); } diff --git a/core/fxcrt/fixed_size_data_vector_unittest.cpp b/core/fxcrt/fixed_size_data_vector_unittest.cpp index 2b0a5c7e4f..3e939087bd 100644 --- a/core/fxcrt/fixed_size_data_vector_unittest.cpp +++ b/core/fxcrt/fixed_size_data_vector_unittest.cpp @@ -5,9 +5,11 @@ #include "core/fxcrt/fixed_size_data_vector.h" #include +#include #include #include +#include "core/fxcrt/fx_memory_wrappers.h" #include "core/fxcrt/span.h" #include "core/fxcrt/stl_util.h" #include "testing/gmock/include/gmock/gmock.h" @@ -180,3 +182,17 @@ TEST(FixedSizeDataVector, Last) { EXPECT_EQ(some[0], 0u); EXPECT_EQ(some[1], 1u); } + +TEST(FixedSizeDataVector, Release) { + auto vec = FixedSizeDataVector::Uninit(4); + ASSERT_EQ(4u, vec.span().size()); + const int* const original_data_ptr = vec.span().data(); + + std::unique_ptr released(vec.release()); + EXPECT_EQ(released.get(), original_data_ptr); + EXPECT_EQ(0u, vec.size()); + EXPECT_TRUE(vec.empty()); + EXPECT_TRUE(vec.span().empty()); + EXPECT_EQ(nullptr, vec.span().data()); + EXPECT_EQ(nullptr, vec.release()); +} diff --git a/core/fxge/skia/cfx_dibbase_skia.cpp b/core/fxge/skia/cfx_dibbase_skia.cpp index ab8a4cf514..2364757d41 100644 --- a/core/fxge/skia/cfx_dibbase_skia.cpp +++ b/core/fxge/skia/cfx_dibbase_skia.cpp @@ -7,15 +7,14 @@ #include #include -#include #include #include #include "core/fxcrt/check_op.h" #include "core/fxcrt/compiler_specific.h" +#include "core/fxcrt/fixed_size_data_vector.h" #include "core/fxcrt/fx_2d_size.h" #include "core/fxcrt/fx_memory.h" -#include "core/fxcrt/fx_memory_wrappers.h" #include "core/fxcrt/fx_safe_types.h" #include "core/fxcrt/notreached.h" #include "core/fxcrt/numerics/safe_conversions.h" @@ -148,10 +147,9 @@ sk_sp CreateSkiaImageFrom24BppDib(const CFX_DIBBase& source) { SkImageInfo info = SkImageInfo::Make(width, height, kBGRA_8888_SkColorType, kOpaque_SkAlphaType); const size_t dst_row_bytes = info.minRowBytes(); - size_t output_size = Fx2DSizeOrDie(dst_row_bytes, height); - std::unique_ptr output( - FX_TryAlloc(uint8_t, output_size)); - if (!output) { + auto output = FixedSizeDataVector::TryUninit( + Fx2DSizeOrDie(dst_row_bytes, height)); + if (output.empty()) { return nullptr; } @@ -160,22 +158,22 @@ sk_sp CreateSkiaImageFrom24BppDib(const CFX_DIBBase& source) { DCHECK_LE(min_row_bytes, source.GetPitch()); if (!source.GetBuffer().empty()) { - libyuv::RGB24ToARGB(source.GetBuffer().data(), source.GetPitch(), - output.get(), pdfium::checked_cast(dst_row_bytes), - width, height); + libyuv::RGB24ToARGB( + source.GetBuffer().data(), source.GetPitch(), output.span().data(), + pdfium::checked_cast(dst_row_bytes), width, height); } else { - // SAFETY: `output` allocated with `output_size` bytes above. - auto output_span = UNSAFE_BUFFERS(pdfium::span(output.get(), output_size)); for (int row = 0; row < height; ++row) { pdfium::span scanline = source.GetScanline(row); ValidateScanlineSize(scanline, min_row_bytes); libyuv::RGB24ToARGB( scanline.data(), 0, - output_span.subspan(Fx2DSizeOrDie(row, dst_row_bytes)).data(), 0, - width, 1); + output.subspan(Fx2DSizeOrDie(row, dst_row_bytes)).data(), 0, width, + 1); } } + // "Leak" allocated memory in `output` to `SkImage`, which will eventually be + // freed by ReleaseAllocatedHeldBySkImage(). return SkImages::RasterFromPixmap( SkPixmap(info, output.release(), dst_row_bytes), /*rasterReleaseProc=*/ReleaseAllocatedHeldBySkImage, @@ -202,15 +200,14 @@ sk_sp CreateSkiaImageFromTransformedDib( SkImageInfo info = SkImageInfo::Make(width, height, color_type, alpha_type); DCHECK_EQ(info.minRowBytes(), width * sizeof(Result)); - size_t output_size = Fx2DSizeOrDie(info.minRowBytes(), height); - std::unique_ptr output( - FX_TryAlloc(uint8_t, output_size)); - if (!output) { + auto output = FixedSizeDataVector::TryUninit( + Fx2DSizeOrDie(info.minRowBytes(), height)); + if (output.empty()) { return nullptr; } // Transform source pixels to output pixels. Iterate by individual scanline. - Result* output_cursor = reinterpret_cast(output.get()); + Result* output_cursor = reinterpret_cast(output.span().data()); const size_t min_row_bytes = fxge::CalculatePitch8OrDie(source.GetBPP(), /*components=*/1, width); DCHECK_LE(min_row_bytes, source.GetPitch()); @@ -226,7 +223,8 @@ sk_sp CreateSkiaImageFromTransformedDib( } } - // "Leak" allocated memory to `SkImage`. + // "Leak" allocated memory in `output` to `SkImage`, which will eventually be + // freed by ReleaseAllocatedHeldBySkImage(). return SkImages::RasterFromPixmap( SkPixmap(info, output.release(), info.minRowBytes()), /*rasterReleaseProc=*/ReleaseAllocatedHeldBySkImage, From 82a14a9c3d2339bfeda997ae6e9d567417a17f09 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 5 Nov 2026 10:36:06 -0800 Subject: [PATCH 19/34] Remove src_left and src_top from CFX_DIBBase::ConvertBuffer() All callers of CFX_DIBBase::ConvertBuffer() pass 0 for `src_left` and `src_top`: - CFX_DIBBase::ConvertTo() and CFX_DIBitmap::ConvertFormat() pass 0 directly. - CFX_DIBitmap::TransferWithUnequalFormats() is only called from CFX_DIBitmap::TransferBitmap() after CFX_DIBBase::GetOverlapRect(), where every caller with unequal formats passes non-positive offsets that GetOverlapRect() shifts into `dest_left` and `dest_top`, leaving `src_left` and `src_top` at 0. Check that `src_left` and `src_top` are 0 in CFX_DIBitmap::TransferBitmap() before calling TransferWithUnequalFormats(), and remove `src_left` and `src_top` from TransferWithUnequalFormats(), CFX_DIBBase::ConvertBuffer(), and all ConvertBuffer_*() helpers. This also fixes ConvertBuffer_RgbToArgbPremul() and ConvertBuffer_32bppRgbToRgb24() passing a single-scanline span to multi-row libyuv calls by using CFX_DIBBase::GetBuffer() directly. This CL is AI generated. Change-Id: I468478e73b6010aa630d03cbfc34a635b279b1ee Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163351 Reviewed-by: Tom Sepez Commit-Queue: Lei Zhang --- core/fxge/dib/cfx_dibbase.cpp | 274 ++++++++++++--------------------- core/fxge/dib/cfx_dibbase.h | 4 +- core/fxge/dib/cfx_dibitmap.cpp | 17 +- core/fxge/dib/cfx_dibitmap.h | 4 +- 4 files changed, 108 insertions(+), 191 deletions(-) diff --git a/core/fxge/dib/cfx_dibbase.cpp b/core/fxge/dib/cfx_dibbase.cpp index cfa77d7f9e..bed4d2363e 100644 --- a/core/fxge/dib/cfx_dibbase.cpp +++ b/core/fxge/dib/cfx_dibbase.cpp @@ -45,32 +45,25 @@ void ConvertBuffer_RgbToArgbPremul( int dest_pitch, int width, int height, - const RetainPtr& src_bitmap, - int src_left, - int src_top) { + const RetainPtr& src_bitmap) { #if defined(PDF_USE_LIBYUV) - if (!src_bitmap->GetBuffer().empty()) { - const uint8_t* src_buf = src_bitmap->GetScanline(src_top) - .subspan(Fx2DSizeOrDie(src_left, 3)) - .data(); - libyuv::RGB24ToARGB(src_buf, src_bitmap->GetPitch(), dest_buf.data(), - dest_pitch, width, height); + pdfium::span src_buffer = src_bitmap->GetBuffer(); + if (!src_buffer.empty()) { + libyuv::RGB24ToARGB(src_buffer.data(), src_bitmap->GetPitch(), + dest_buf.data(), dest_pitch, width, height); return; } for (int row = 0; row < height; ++row) { auto dest_span = dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)); - auto src_span = src_bitmap->GetScanline(src_top + row) - .subspan(Fx2DSizeOrDie(src_left, 3)); + auto src_span = src_bitmap->GetScanline(row); libyuv::RGB24ToARGB(src_span.data(), 0, dest_span.data(), 0, width, 1); } #else for (int row = 0; row < height; ++row) { auto dest_span = fxcrt::reinterpret_span>( dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); - auto src_span = - src_bitmap->GetScanlineAs>(src_top + row) - .subspan(static_cast(src_left)); + auto src_span = src_bitmap->GetScanlineAs>(row); for (auto [input, output] : fxcrt::Zip(src_span, dest_span)) { // Avoid interleaved load/stores. const uint8_t blue = input.blue; @@ -90,15 +83,11 @@ void ConvertBuffer_ArgbPremulToRgb( int dest_pitch, int width, int height, - const RetainPtr& src_bitmap, - int src_left, - int src_top) { + const RetainPtr& src_bitmap) { for (int row = 0; row < height; ++row) { auto dest_span = fxcrt::reinterpret_span>( dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); - auto src_span = - src_bitmap->GetScanlineAs>(src_top + row) - .subspan(static_cast(src_left)); + auto src_span = src_bitmap->GetScanlineAs>(row); for (auto [input, output] : fxcrt::Zip(src_span, dest_span)) { auto unpremultiplied_input = UnPreMultiplyColor(input); output.blue = unpremultiplied_input.blue; @@ -112,16 +101,14 @@ void ConvertBuffer_ArgbPremul(pdfium::span dest_buf, int dest_pitch, int width, int height, - const RetainPtr& src_bitmap, - int src_left, - int src_top) { + const RetainPtr& src_bitmap) { switch (src_bitmap->GetBPP()) { case 8: // TODO(crbug.com/42271020): Determine if this ever happens. NOTREACHED(); case 24: ConvertBuffer_RgbToArgbPremul(dest_buf, dest_pitch, width, height, - src_bitmap, src_left, src_top); + src_bitmap); break; case 32: // TODO(crbug.com/42271020): Determine if this ever happens. @@ -137,21 +124,18 @@ void ConvertBuffer_1bppMaskToGray( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { static constexpr uint8_t kSetGray = 0xff; static constexpr uint8_t kResetGray = 0x00; for (int row = 0; row < height; ++row) { pdfium::span dest_span = dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)); - pdfium::span src_span = - pSrcBitmap->GetScanline(src_top + row); + pdfium::span src_span = pSrcBitmap->GetScanline(row); std::ranges::fill(dest_span.first(static_cast(width)), kResetGray); uint8_t* dest_scan = dest_span.data(); const uint8_t* src_scan = src_span.data(); UNSAFE_TODO({ - for (int col = src_left; col < src_left + width; ++col) { + for (int col = 0; col < width; ++col) { if (src_scan[col / 8] & (1 << (7 - col % 8))) { *dest_scan = kSetGray; } @@ -166,14 +150,10 @@ void ConvertBuffer_8bppMaskToGray( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { for (int row = 0; row < height; ++row) { - fxcrt::Copy( - pSrcBitmap->GetScanline(src_top + row) - .subspan(static_cast(src_left), static_cast(width)), - dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); + fxcrt::Copy(pSrcBitmap->GetScanline(row).first(static_cast(width)), + dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); } } @@ -181,9 +161,7 @@ void ConvertBuffer_8bppPltToGray(pdfium::span dest_buf, int dest_pitch, int width, int height, - const RetainPtr& src, - int src_left, - int src_top) { + const RetainPtr& src) { pdfium::span src_palette = src->GetPaletteSpan(); CHECK_EQ(256u, src_palette.size()); std::array gray; @@ -192,9 +170,7 @@ void ConvertBuffer_8bppPltToGray(pdfium::span dest_buf, } for (int row = 0; row < height; ++row) { auto dest_scan = dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)); - auto src_scan = - src->GetScanline(src_top + row) - .subspan(static_cast(src_left), static_cast(width)); + auto src_scan = src->GetScanline(row).first(static_cast(width)); for (auto [input, output] : fxcrt::Zip(src_scan, dest_scan)) { output = gray[input]; } @@ -206,15 +182,12 @@ void ConvertBuffer_RgbToGray(pdfium::span dest_buf, int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { CHECK_EQ(pSrcBitmap->GetBPP(), 8 * sizeof(SrcType)); for (int row = 0; row < height; ++row) { auto dest_scan = dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)); - auto src_scan = - pSrcBitmap->GetScanlineAs(src_top + row) - .subspan(static_cast(src_left), static_cast(width)); + auto src_scan = pSrcBitmap->GetScanlineAs(row).first( + static_cast(width)); for (auto [src_pix, dest_pix] : fxcrt::Zip(src_scan, dest_scan)) { dest_pix = FXRGB2GRAY(src_pix.red, src_pix.green, src_pix.blue); } @@ -225,9 +198,7 @@ void ConvertBuffer_IndexCopy(pdfium::span dest_buf, int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { if (pSrcBitmap->GetBPP() == 1) { for (int row = 0; row < height; ++row) { pdfium::span dest_span = @@ -235,9 +206,9 @@ void ConvertBuffer_IndexCopy(pdfium::span dest_buf, // Set all destination pixels to be white initially. std::ranges::fill(dest_span.first(static_cast(width)), 255); uint8_t* dest_scan = dest_span.data(); - const uint8_t* src_scan = pSrcBitmap->GetScanline(src_top + row).data(); + const uint8_t* src_scan = pSrcBitmap->GetScanline(row).data(); UNSAFE_TODO({ - for (int col = src_left; col < src_left + width; ++col) { + for (int col = 0; col < width; ++col) { // If the source bit is set, then set the destination pixel to be // black. if (src_scan[col / 8] & (1 << (7 - col % 8))) { @@ -250,10 +221,9 @@ void ConvertBuffer_IndexCopy(pdfium::span dest_buf, } } else { for (int row = 0; row < height; ++row) { - fxcrt::Copy(pSrcBitmap->GetScanline(src_top + row) - .subspan(static_cast(src_left), - static_cast(width)), - dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); + fxcrt::Copy( + pSrcBitmap->GetScanline(row).first(static_cast(width)), + dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); } } } @@ -264,30 +234,26 @@ DataVector ConvertBuffer_PltToPltRgb8( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { - ConvertBuffer_IndexCopy(dest_buf, dest_pitch, width, height, pSrcBitmap, - src_left, src_top); + const RetainPtr& pSrcBitmap) { + ConvertBuffer_IndexCopy(dest_buf, dest_pitch, width, height, pSrcBitmap); const size_t plt_size = pSrcBitmap->GetRequiredPaletteSize(); return ToDataVector(pSrcBitmap->GetPaletteSpan().first(plt_size)); } -void ConvertBuffer_1bppMaskToRgb(pdfium::span dest_buf, - int dest_pitch, - int width, - int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { +void ConvertBuffer_1bppMaskToRgb( + pdfium::span dest_buf, + int dest_pitch, + int width, + int height, + const RetainPtr& pSrcBitmap) { static constexpr uint8_t kSetGray = 0xff; static constexpr uint8_t kResetGray = 0x00; for (int row = 0; row < height; ++row) { uint8_t* dest_scan = dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)).data(); - const uint8_t* src_scan = pSrcBitmap->GetScanline(src_top + row).data(); + const uint8_t* src_scan = pSrcBitmap->GetScanline(row).data(); UNSAFE_TODO({ - for (int col = src_left; col < src_left + width; ++col) { + for (int col = 0; col < width; ++col) { uint8_t value = (src_scan[col / 8] & (1 << (7 - col % 8))) ? kSetGray : kResetGray; FXSYS_memset(dest_scan, value, 3); @@ -302,15 +268,12 @@ void ConvertBuffer_8bppMaskToRgb24( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { for (int row = 0; row < height; ++row) { auto dest_scan = fxcrt::reinterpret_span>( dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); auto src_scan = - pSrcBitmap->GetScanline(src_top + row) - .subspan(static_cast(src_left), static_cast(width)); + pSrcBitmap->GetScanline(row).first(static_cast(width)); for (auto [src_pix, dest_pix] : fxcrt::Zip(src_scan, dest_scan)) { dest_pix = {src_pix, src_pix, src_pix}; } @@ -322,15 +285,12 @@ void ConvertBuffer_8bppMaskToRgb32( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { for (int row = 0; row < height; ++row) { auto dest_scan = fxcrt::reinterpret_span>( dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); auto src_scan = - pSrcBitmap->GetScanline(src_top + row) - .subspan(static_cast(src_left), static_cast(width)); + pSrcBitmap->GetScanline(row).first(static_cast(width)); for (auto [src_pix, dest_pix] : fxcrt::Zip(src_scan, dest_scan)) { // This should allow the compiler to assemble a 32-bit word in // registers followed by a 32-bit store, likely more efficient @@ -340,13 +300,12 @@ void ConvertBuffer_8bppMaskToRgb32( } } -void ConvertBuffer_1bppPltToRgb(pdfium::span dest_buf, - int dest_pitch, - int width, - int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { +void ConvertBuffer_1bppPltToRgb( + pdfium::span dest_buf, + int dest_pitch, + int width, + int height, + const RetainPtr& pSrcBitmap) { pdfium::span src_palette = pSrcBitmap->GetPaletteSpan(); const uint8_t dst_palette[6] = { FXARGB_B(src_palette[0]), FXARGB_G(src_palette[0]), @@ -355,9 +314,9 @@ void ConvertBuffer_1bppPltToRgb(pdfium::span dest_buf, for (int row = 0; row < height; ++row) { uint8_t* dest_scan = dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)).data(); - const uint8_t* src_scan = pSrcBitmap->GetScanline(src_top + row).data(); + const uint8_t* src_scan = pSrcBitmap->GetScanline(row).data(); UNSAFE_TODO({ - for (int col = src_left; col < src_left + width; ++col) { + for (int col = 0; col < width; ++col) { size_t offset = (src_scan[col / 8] & (1 << (7 - col % 8))) ? 3 : 0; FXSYS_memcpy(dest_scan, dst_palette + offset, 3); dest_scan += 3; @@ -372,15 +331,12 @@ void ConvertBuffer_8bppPltToRgb24( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { for (int row = 0; row < height; ++row) { auto dest_scan = fxcrt::reinterpret_span>( dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); auto src_scan = - pSrcBitmap->GetScanline(src_top + row) - .subspan(static_cast(src_left), static_cast(width)); + pSrcBitmap->GetScanline(row).first(static_cast(width)); for (auto [src_index, dest_pix] : fxcrt::Zip(src_scan, dest_scan)) { dest_pix = dst_palette[src_index]; } @@ -393,15 +349,12 @@ void ConvertBuffer_8bppPltToRgb32( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { for (int row = 0; row < height; ++row) { auto dest_scan = fxcrt::reinterpret_span>( dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); auto src_scan = - pSrcBitmap->GetScanline(src_top + row) - .subspan(static_cast(src_left), static_cast(width)); + pSrcBitmap->GetScanline(row).first(static_cast(width)); for (auto [src_index, dest_pix] : fxcrt::Zip(src_scan, dest_scan)) { const auto& pal_pix = dst_palette[src_index]; // This should allow the compiler to assemble a 32-bit word in @@ -416,15 +369,12 @@ void ConvertBuffer_1bppToRgb(pdfium::span dest_buf, int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { if (pSrcBitmap->HasPalette()) { - ConvertBuffer_1bppPltToRgb(dest_buf, dest_pitch, width, height, pSrcBitmap, - src_left, src_top); + ConvertBuffer_1bppPltToRgb(dest_buf, dest_pitch, width, height, pSrcBitmap); } else { - ConvertBuffer_1bppMaskToRgb(dest_buf, dest_pitch, width, height, pSrcBitmap, - src_left, src_top); + ConvertBuffer_1bppMaskToRgb(dest_buf, dest_pitch, width, height, + pSrcBitmap); } } @@ -433,9 +383,7 @@ void ConvertBuffer_8bppToRgb(FXDIB_Format dest_format, int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { if (pSrcBitmap->HasPalette()) { pdfium::span src_palette = pSrcBitmap->GetPaletteSpan(); CHECK_EQ(256u, src_palette.size()); @@ -446,18 +394,18 @@ void ConvertBuffer_8bppToRgb(FXDIB_Format dest_format, } if (GetCompsFromFormat(dest_format) == 3) { ConvertBuffer_8bppPltToRgb24(dst_palette, dest_buf, dest_pitch, width, - height, pSrcBitmap, src_left, src_top); + height, pSrcBitmap); } else { ConvertBuffer_8bppPltToRgb32(dst_palette, dest_buf, dest_pitch, width, - height, pSrcBitmap, src_left, src_top); + height, pSrcBitmap); } } else { if (GetCompsFromFormat(dest_format) == 3) { ConvertBuffer_8bppMaskToRgb24(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); } else { ConvertBuffer_8bppMaskToRgb32(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); } } } @@ -467,15 +415,11 @@ void ConvertBuffer_24bppRgbToRgb24( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { - const size_t x_offset = Fx2DSizeOrDie(src_left, 3); + const RetainPtr& pSrcBitmap) { const size_t byte_count = Fx2DSizeOrDie(width, 3); for (int row = 0; row < height; ++row) { - fxcrt::Copy( - pSrcBitmap->GetScanline(src_top + row).subspan(x_offset, byte_count), - dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); + fxcrt::Copy(pSrcBitmap->GetScanline(row).first(byte_count), + dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); } } @@ -484,16 +428,12 @@ void ConvertBuffer_32bppRgbToRgb24( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { - const size_t x_offset = Fx2DSizeOrDie(src_left, 4); + const RetainPtr& pSrcBitmap) { #if defined(PDF_USE_LIBYUV) - if (!pSrcBitmap->GetBuffer().empty()) { - const uint8_t* src_buf = - pSrcBitmap->GetScanline(src_top).subspan(x_offset).data(); - libyuv::ARGBToRGB24(src_buf, pSrcBitmap->GetPitch(), dest_buf.data(), - dest_pitch, width, height); + pdfium::span src_buffer = pSrcBitmap->GetBuffer(); + if (!src_buffer.empty()) { + libyuv::ARGBToRGB24(src_buffer.data(), pSrcBitmap->GetPitch(), + dest_buf.data(), dest_pitch, width, height); return; } #endif @@ -501,8 +441,7 @@ void ConvertBuffer_32bppRgbToRgb24( for (int row = 0; row < height; ++row) { uint8_t* dest_scan = dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch)).data(); - const uint8_t* src_scan = - pSrcBitmap->GetScanline(src_top + row).subspan(x_offset).data(); + const uint8_t* src_scan = pSrcBitmap->GetScanline(row).data(); #if defined(PDF_USE_LIBYUV) libyuv::ARGBToRGB24(src_scan, 0, dest_scan, 0, width, 1); #else @@ -522,16 +461,13 @@ void ConvertBuffer_RgbToRgb32(pdfium::span dest_buf, int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { CHECK_EQ(pSrcBitmap->GetBPP(), 8 * sizeof(SrcType)); for (int row = 0; row < height; ++row) { auto dest_scan = fxcrt::reinterpret_span>( dest_buf.subspan(Fx2DSizeOrDie(row, dest_pitch))); - auto src_scan = - pSrcBitmap->GetScanlineAs(src_top + row) - .subspan(static_cast(src_left), static_cast(width)); + auto src_scan = pSrcBitmap->GetScanlineAs(row).first( + static_cast(width)); for (auto [src_pix, dest_pix] : fxcrt::Zip(src_scan, dest_scan)) { // This should allow the compiler to assemble a 32-bit word in // registers followed by a 32-bit store, likely more efficient @@ -545,27 +481,25 @@ void ConvertBuffer_8bppMask(pdfium::span dest_buf, int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { switch (pSrcBitmap->GetBPP()) { case 1: CHECK(!pSrcBitmap->HasPalette()); ConvertBuffer_1bppMaskToGray(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); break; case 8: if (pSrcBitmap->HasPalette()) { ConvertBuffer_8bppPltToGray(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); } else { ConvertBuffer_8bppMaskToGray(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); } break; case 24: ConvertBuffer_RgbToGray>( - dest_buf, dest_pitch, width, height, pSrcBitmap, src_left, src_top); + dest_buf, dest_pitch, width, height, pSrcBitmap); break; case 32: #if defined(PDF_USE_SKIA) @@ -573,7 +507,7 @@ void ConvertBuffer_8bppMask(pdfium::span dest_buf, CHECK_NE(pSrcBitmap->GetFormat(), FXDIB_Format::kBgraPremul); #endif ConvertBuffer_RgbToGray>( - dest_buf, dest_pitch, width, height, pSrcBitmap, src_left, src_top); + dest_buf, dest_pitch, width, height, pSrcBitmap); break; default: NOTREACHED(); @@ -585,32 +519,29 @@ void ConvertBuffer_Rgb(FXDIB_Format dest_format, int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { switch (pSrcBitmap->GetBPP()) { case 1: - ConvertBuffer_1bppToRgb(dest_buf, dest_pitch, width, height, pSrcBitmap, - src_left, src_top); + ConvertBuffer_1bppToRgb(dest_buf, dest_pitch, width, height, pSrcBitmap); break; case 8: ConvertBuffer_8bppToRgb(dest_format, dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); break; case 24: ConvertBuffer_24bppRgbToRgb24(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); break; case 32: #if defined(PDF_USE_SKIA) if (pSrcBitmap->GetFormat() == FXDIB_Format::kBgraPremul) { ConvertBuffer_ArgbPremulToRgb(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); break; } #endif ConvertBuffer_32bppRgbToRgb24(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); break; default: NOTREACHED(); @@ -622,17 +553,15 @@ void ConvertBuffer_Argb(FXDIB_Format dest_format, int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { switch (pSrcBitmap->GetBPP()) { case 8: ConvertBuffer_8bppToRgb(dest_format, dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); break; case 24: ConvertBuffer_RgbToRgb32>( - dest_buf, dest_pitch, width, height, pSrcBitmap, src_left, src_top); + dest_buf, dest_pitch, width, height, pSrcBitmap); break; case 32: #if defined(PDF_USE_SKIA) @@ -640,7 +569,7 @@ void ConvertBuffer_Argb(FXDIB_Format dest_format, CHECK_NE(pSrcBitmap->GetFormat(), FXDIB_Format::kBgraPremul); #endif ConvertBuffer_RgbToRgb32>( - dest_buf, dest_pitch, width, height, pSrcBitmap, src_left, src_top); + dest_buf, dest_pitch, width, height, pSrcBitmap); break; default: NOTREACHED(); @@ -1125,7 +1054,7 @@ RetainPtr CFX_DIBBase::ConvertTo(FXDIB_Format dest_format) const { RetainPtr holder(this); DataVector pal_8bpp = ConvertBuffer(dest_format, pClone->GetWritableBuffer(), - pClone->GetPitch(), GetWidth(), GetHeight(), holder, 0, 0); + pClone->GetPitch(), GetWidth(), GetHeight(), holder); if (!pal_8bpp.empty()) { pClone->TakePalette(std::move(pal_8bpp)); } @@ -1333,9 +1262,7 @@ DataVector CFX_DIBBase::ConvertBuffer( int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top) { + const RetainPtr& pSrcBitmap) { switch (dest_format) { case FXDIB_Format::kInvalid: case FXDIB_Format::k1bppRgb: @@ -1343,8 +1270,7 @@ DataVector CFX_DIBBase::ConvertBuffer( NOTREACHED(); } case FXDIB_Format::k8bppMask: { - ConvertBuffer_8bppMask(dest_buf, dest_pitch, width, height, pSrcBitmap, - src_left, src_top); + ConvertBuffer_8bppMask(dest_buf, dest_pitch, width, height, pSrcBitmap); return {}; } case FXDIB_Format::k8bppRgb: { @@ -1352,27 +1278,25 @@ DataVector CFX_DIBBase::ConvertBuffer( CHECK(src_bpp == 1 || src_bpp == 8); if (pSrcBitmap->HasPalette()) { return ConvertBuffer_PltToPltRgb8(dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); } - ConvertBuffer_8bppMask(dest_buf, dest_pitch, width, height, pSrcBitmap, - src_left, src_top); + ConvertBuffer_8bppMask(dest_buf, dest_pitch, width, height, pSrcBitmap); return {}; } case FXDIB_Format::kBgr: { ConvertBuffer_Rgb(dest_format, dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); return {}; } case FXDIB_Format::kBgra: case FXDIB_Format::kBgrx: { ConvertBuffer_Argb(dest_format, dest_buf, dest_pitch, width, height, - pSrcBitmap, src_left, src_top); + pSrcBitmap); return {}; } #if defined(PDF_USE_SKIA) case FXDIB_Format::kBgraPremul: { - ConvertBuffer_ArgbPremul(dest_buf, dest_pitch, width, height, pSrcBitmap, - src_left, src_top); + ConvertBuffer_ArgbPremul(dest_buf, dest_pitch, width, height, pSrcBitmap); return {}; } #endif diff --git a/core/fxge/dib/cfx_dibbase.h b/core/fxge/dib/cfx_dibbase.h index 99d7224b88..57d20df0f7 100644 --- a/core/fxge/dib/cfx_dibbase.h +++ b/core/fxge/dib/cfx_dibbase.h @@ -146,9 +146,7 @@ class CFX_DIBBase : public Retainable { int dest_pitch, int width, int height, - const RetainPtr& pSrcBitmap, - int src_left, - int src_top); + const RetainPtr& pSrcBitmap); RetainPtr ClipToInternal(const FX_RECT* pClip) const; void BuildPalette(); diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp index 4836116712..116fcdc6b4 100644 --- a/core/fxge/dib/cfx_dibitmap.cpp +++ b/core/fxge/dib/cfx_dibitmap.cpp @@ -221,9 +221,10 @@ bool CFX_DIBitmap::TransferBitmap(int width, FXDIB_Format dest_format = GetFormat(); FXDIB_Format src_format = source->GetFormat(); if (dest_format != src_format) { + CHECK_EQ(src_left, 0); + CHECK_EQ(src_top, 0); return TransferWithUnequalFormats(dest_format, dest_left, dest_top, width, - height, std::move(source), src_left, - src_top); + height, std::move(source)); } if (GetBPP() != 1) { @@ -243,9 +244,7 @@ bool CFX_DIBitmap::TransferWithUnequalFormats( int dest_top, int width, int height, - RetainPtr source, - int src_left, - int src_top) { + RetainPtr source) { if (HasPalette()) { return false; } @@ -263,9 +262,8 @@ bool CFX_DIBitmap::TransferWithUnequalFormats( pdfium::span dest_buf = GetWritableBuffer().subspan( dest_top * GetPitch() + static_cast(offset.ValueOrDie())); - DataVector dest_palette = - ConvertBuffer(dest_format, dest_buf, GetPitch(), width, height, source, - src_left, src_top); + DataVector dest_palette = ConvertBuffer( + dest_format, dest_buf, GetPitch(), width, height, std::move(source)); CHECK(dest_palette.empty()); return true; } @@ -1024,8 +1022,7 @@ bool CFX_DIBitmap::ConvertFormat(FXDIB_Format dest_format) { RetainPtr holder(this); palette_ = ConvertBuffer(dest_format, dest_span, dest_pitch, GetWidth(), - GetHeight(), holder, /*src_left=*/0, - /*src_top=*/0); + GetHeight(), holder); #if defined(PDF_USE_LIBYUV) } #endif diff --git a/core/fxge/dib/cfx_dibitmap.h b/core/fxge/dib/cfx_dibitmap.h index 5c1f7f38ad..bc0fb6e7cb 100644 --- a/core/fxge/dib/cfx_dibitmap.h +++ b/core/fxge/dib/cfx_dibitmap.h @@ -212,9 +212,7 @@ class CFX_DIBitmap final : public CFX_DIBBase { int dest_top, int width, int height, - RetainPtr source, - int src_left, - int src_top); + RetainPtr source); void TransferWithMultipleBPP(int dest_left, int dest_top, int width, From c7348a4dd7463bf07d4a3e1bd20bd20d4ceb094c Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 5 Nov 2026 11:29:01 -0800 Subject: [PATCH 20/34] Guard JSPLATFORM dialog call-outs with in_active_ffi_callback_ CPDFSDK_FormFillEnvironment::JS_appAlert(), JS_appResponse(), and JS_appBeep() invoke embedder callbacks via IPDF_JSPLATFORM without setting in_active_ffi_callback_. In Chrome, embedder dialogs (such as alert, confirm, and prompt) force a style and layout update before opening. When the plugin element has been detached or hidden, that update can run synchronously while an XFA script remains. Fixed: 580848937 Change-Id: Ie95b280942bb21003c4d9f43cbf33d915d04fad8 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163350 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- fpdfsdk/cpdfsdk_formfillenvironment.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fpdfsdk/cpdfsdk_formfillenvironment.cpp b/fpdfsdk/cpdfsdk_formfillenvironment.cpp index e85a9bf5c6..124f504e19 100644 --- a/fpdfsdk/cpdfsdk_formfillenvironment.cpp +++ b/fpdfsdk/cpdfsdk_formfillenvironment.cpp @@ -219,6 +219,7 @@ int CPDFSDK_FormFillEnvironment::JS_appAlert(const WideString& Msg, ByteString bsMsg = Msg.ToUTF16LE(); ByteString bsTitle = Title.ToUTF16LE(); + AutoRestorer restorer(&in_active_ffi_callback_, true); return js_platform->app_alert(js_platform, AsFPDFWideString(&bsMsg), AsFPDFWideString(&bsTitle), Type, Icon); } @@ -239,6 +240,7 @@ int CPDFSDK_FormFillEnvironment::JS_appResponse( ByteString bsTitle = Title.ToUTF16LE(); ByteString bsDefault = Default.ToUTF16LE(); ByteString bsLabel = Label.ToUTF16LE(); + AutoRestorer restorer(&in_active_ffi_callback_, true); return js_platform->app_response( js_platform, AsFPDFWideString(&bsQuestion), AsFPDFWideString(&bsTitle), AsFPDFWideString(&bsDefault), AsFPDFWideString(&bsLabel), bPassword, @@ -251,6 +253,7 @@ void CPDFSDK_FormFillEnvironment::JS_appBeep(int nType) { return; } + AutoRestorer restorer(&in_active_ffi_callback_, true); js_platform->app_beep(js_platform, nType); } From b3c6c6f4b0e7d0b25c9b98b6eab6eae667f8d217 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 5 Nov 2026 11:51:39 -0800 Subject: [PATCH 21/34] Fix UNSAFE_TODO() in CreateSkiaImageFromTransformedDib() Change `output` in CreateSkiaImageFromTransformedDib() from FixedSizeDataVector to FixedSizeDataVector. Index into its span by `column` and advance `output_span` by `width` on each row instead of incrementing a raw `output_cursor` pointer inside an UNSAFE_TODO() block. Also remove the redundant `line` variable and promote the row byte and pitch assertions to CHECKs. This CL is AI generated. Change-Id: I90dfd08e55eb69488a928184a6e0d114128c6bc2 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163290 Reviewed-by: Tom Sepez Commit-Queue: Lei Zhang --- core/fxge/skia/cfx_dibbase_skia.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/core/fxge/skia/cfx_dibbase_skia.cpp b/core/fxge/skia/cfx_dibbase_skia.cpp index 2364757d41..71d06fd4e9 100644 --- a/core/fxge/skia/cfx_dibbase_skia.cpp +++ b/core/fxge/skia/cfx_dibbase_skia.cpp @@ -11,7 +11,6 @@ #include #include "core/fxcrt/check_op.h" -#include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/fixed_size_data_vector.h" #include "core/fxcrt/fx_2d_size.h" #include "core/fxcrt/fx_memory.h" @@ -198,29 +197,29 @@ sk_sp CreateSkiaImageFromTransformedDib( const int width = source.GetWidth(); const int height = source.GetHeight(); SkImageInfo info = SkImageInfo::Make(width, height, color_type, alpha_type); - DCHECK_EQ(info.minRowBytes(), width * sizeof(Result)); + CHECK_EQ(info.minRowBytes(), width * sizeof(Result)); - auto output = FixedSizeDataVector::TryUninit( - Fx2DSizeOrDie(info.minRowBytes(), height)); + auto output = + FixedSizeDataVector::TryUninit(Fx2DSizeOrDie(width, height)); if (output.empty()) { return nullptr; } // Transform source pixels to output pixels. Iterate by individual scanline. - Result* output_cursor = reinterpret_cast(output.span().data()); + pdfium::span output_span = output.span(); const size_t min_row_bytes = fxge::CalculatePitch8OrDie(source.GetBPP(), /*components=*/1, width); - DCHECK_LE(min_row_bytes, source.GetPitch()); + CHECK_LE(min_row_bytes, source.GetPitch()); - int line = 0; for (int row = 0; row < height; ++row) { - pdfium::span scanline = source.GetScanline(line++); + pdfium::span scanline = source.GetScanline(row); ValidateScanlineSize(scanline, min_row_bytes); for (int column = 0; column < width; ++column) { - UNSAFE_TODO(*output_cursor++) = Traits::Invoke( + output_span[column] = Traits::Invoke( std::forward(pixel_transform), scanline, column); } + output_span = output_span.subspan(static_cast(width)); } // "Leak" allocated memory in `output` to `SkImage`, which will eventually be From 5a31c5c34384dfdf6290575a19c768dac92df4a9 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 5 Nov 2026 11:59:27 -0800 Subject: [PATCH 22/34] Improve libyuv path in CFX_DIBBase::SwapXY() - Check CFX_DIBBase::GetBuffer() directly instead of calling CFX_DIBBase::RealizeIfNeeded(). When a DIB is not backed by a contiguous buffer, fall back to the scanline loop instead of allocating and copying into an intermediate CFX_DIBitmap. This also allows reverting `defined(PDF_USE_LIBYUV)` from the RealizeIfNeeded() guards. - Use libyuv::RotatePlane() for 1-byte-per-pixel bitmaps to match libyuv::ARGBRotate() and avoid passing a single-scanline span with a negative stride to the deprecated libyuv::RotatePlane270() function. This CL is AI generated. Change-Id: If83bd73ee0023794ab1f0c3962817ad5918ac116 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163352 Reviewed-by: Tom Sepez Commit-Queue: Lei Zhang --- core/fxge/dib/cfx_dibbase.cpp | 40 ++++++++++++---------------------- core/fxge/dib/cfx_dibbase.h | 2 +- core/fxge/dib/cfx_dibitmap.cpp | 2 +- core/fxge/dib/cfx_dibitmap.h | 2 +- 4 files changed, 17 insertions(+), 29 deletions(-) diff --git a/core/fxge/dib/cfx_dibbase.cpp b/core/fxge/dib/cfx_dibbase.cpp index bed4d2363e..39a5744de4 100644 --- a/core/fxge/dib/cfx_dibbase.cpp +++ b/core/fxge/dib/cfx_dibbase.cpp @@ -594,7 +594,7 @@ size_t CFX_DIBBase::GetEstimatedImageMemoryBurden() const { return GetRequiredPaletteSize() * sizeof(uint32_t); } -#if BUILDFLAG(IS_WIN) || defined(PDF_USE_SKIA) || defined(PDF_USE_LIBYUV) +#if BUILDFLAG(IS_WIN) || defined(PDF_USE_SKIA) RetainPtr CFX_DIBBase::RealizeIfNeeded() const { return Realize(); } @@ -1111,33 +1111,21 @@ RetainPtr CFX_DIBBase::SwapXY(bool bXFlip, bool bYFlip) const { const int bytes_per_pixel = GetBPP() / 8; #if defined(PDF_USE_LIBYUV) if (bytes_per_pixel == 1 || bytes_per_pixel == 4) { - RetainPtr realized = RealizeIfNeeded(); - if (realized) { - const uint8_t* src_buf = realized->GetBuffer().data(); - const int src_pitch = realized->GetPitch(); - uint8_t* dst_buf = dest_span.data(); + pdfium::span src_buffer = GetBuffer(); + if (!src_buffer.empty()) { + const int height = bXFlip == bYFlip ? -GetHeight() : GetHeight(); + const libyuv::RotationMode mode = + bYFlip ? libyuv::kRotate270 : libyuv::kRotate90; if (bytes_per_pixel == 1) { - if (bXFlip && bYFlip) { - libyuv::RotatePlane270(realized->GetScanline(GetHeight() - 1).data(), - -src_pitch, dst_buf, dest_pitch, GetWidth(), - GetHeight()); - } else if (bXFlip) { - libyuv::RotatePlane90(src_buf, src_pitch, dst_buf, dest_pitch, - GetWidth(), GetHeight()); - } else if (bYFlip) { - libyuv::RotatePlane270(src_buf, src_pitch, dst_buf, dest_pitch, - GetWidth(), GetHeight()); - } else { - libyuv::TransposePlane(src_buf, src_pitch, dst_buf, dest_pitch, - GetWidth(), GetHeight()); + if (libyuv::RotatePlane(src_buffer.data(), GetPitch(), dest_span.data(), + dest_pitch, GetWidth(), height, mode) == 0) { + return pTransBitmap; + } + } else { + if (libyuv::ARGBRotate(src_buffer.data(), GetPitch(), dest_span.data(), + dest_pitch, GetWidth(), height, mode) == 0) { + return pTransBitmap; } - return pTransBitmap; - } - if (libyuv::ARGBRotate( - src_buf, src_pitch, dst_buf, dest_pitch, GetWidth(), - bXFlip == bYFlip ? -GetHeight() : GetHeight(), - bYFlip ? libyuv::kRotate270 : libyuv::kRotate90) == 0) { - return pTransBitmap; } } } diff --git a/core/fxge/dib/cfx_dibbase.h b/core/fxge/dib/cfx_dibbase.h index 57d20df0f7..4502bb5daf 100644 --- a/core/fxge/dib/cfx_dibbase.h +++ b/core/fxge/dib/cfx_dibbase.h @@ -52,7 +52,7 @@ class CFX_DIBBase : public Retainable { virtual pdfium::span GetScanline(int line) const = 0; virtual bool SkipToScanline(int line, PauseIndicatorIface* pPause) const; virtual size_t GetEstimatedImageMemoryBurden() const; -#if BUILDFLAG(IS_WIN) || defined(PDF_USE_SKIA) || defined(PDF_USE_LIBYUV) +#if BUILDFLAG(IS_WIN) || defined(PDF_USE_SKIA) // Calls Realize() if needed. Otherwise, return `this`. virtual RetainPtr RealizeIfNeeded() const; #endif diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp index 116fcdc6b4..6bc06c7e61 100644 --- a/core/fxge/dib/cfx_dibitmap.cpp +++ b/core/fxge/dib/cfx_dibitmap.cpp @@ -131,7 +131,7 @@ size_t CFX_DIBitmap::GetEstimatedImageMemoryBurden() const { return result; } -#if BUILDFLAG(IS_WIN) || defined(PDF_USE_SKIA) || defined(PDF_USE_LIBYUV) +#if BUILDFLAG(IS_WIN) || defined(PDF_USE_SKIA) RetainPtr CFX_DIBitmap::RealizeIfNeeded() const { if (GetBuffer().empty()) { return Realize(); diff --git a/core/fxge/dib/cfx_dibitmap.h b/core/fxge/dib/cfx_dibitmap.h index bc0fb6e7cb..b442316345 100644 --- a/core/fxge/dib/cfx_dibitmap.h +++ b/core/fxge/dib/cfx_dibitmap.h @@ -63,7 +63,7 @@ class CFX_DIBitmap final : public CFX_DIBBase { pdfium::span GetBuffer() const override; pdfium::span GetScanline(int line) const override; size_t GetEstimatedImageMemoryBurden() const override; -#if BUILDFLAG(IS_WIN) || defined(PDF_USE_SKIA) || defined(PDF_USE_LIBYUV) +#if BUILDFLAG(IS_WIN) || defined(PDF_USE_SKIA) RetainPtr RealizeIfNeeded() const override; #endif From 4f93283fe3564aa8e5ef77f1b98969b1d9e2e5ac Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 5 Nov 2026 14:02:36 -0800 Subject: [PATCH 23/34] Enforce geometry invariants in LibjpegScanlineDecoder::Rewind Validate that re-parsing a JPEG header upon rewind preserves the dimensions, component count, downscale denominator, and color order established during decoder initialization. Rejecting altered headers prevents desynchronization between libjpeg decompressed rows and the pre-allocated scanline buffer. Additionally, verify that libjpeg's reported output width and row byte requirement do not exceed the scanline buffer allocation, destroying the decompressor and aborting decode cleanly on failure. Add unit tests covering dimension, component count, and header corruption upon decoder rewind. Fixed: 580846890 Change-Id: I687635a3841d4d4dfa48763c37057cba191afdb8 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163170 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- core/fxcodec/jpeg/jpegmodule_unittest.cpp | 61 +++++++++++++++++++ .../fxcodec/jpeg/libjpeg_scanline_decoder.cpp | 48 ++++++++++++--- 2 files changed, 102 insertions(+), 7 deletions(-) diff --git a/core/fxcodec/jpeg/jpegmodule_unittest.cpp b/core/fxcodec/jpeg/jpegmodule_unittest.cpp index 2589d06f85..54fcff6b18 100644 --- a/core/fxcodec/jpeg/jpegmodule_unittest.cpp +++ b/core/fxcodec/jpeg/jpegmodule_unittest.cpp @@ -83,6 +83,67 @@ TEST(JpegModuleTest, CreateDecoderValid) { EXPECT_GE(scanline0.size(), 360u); } +#if !defined(PDF_ENABLE_RUST_JPEG) +TEST(JpegModuleTest, RewindMismatchedDimensionsFails) { + std::vector jpeg_data = LoadMonaLisa(); + ASSERT_FALSE(jpeg_data.empty()); + + std::unique_ptr decoder = + JpegModule::CreateDecoder(jpeg_data, 120, 120, 3, true, 1, false); + ASSERT_TRUE(decoder); + EXPECT_FALSE(decoder->GetScanline(0).empty()); + EXPECT_FALSE(decoder->GetScanline(1).empty()); + + // Find SOF0 marker (0xFF, 0xC0) and mutate the width. + ASSERT_EQ(jpeg_data[242], 0xFF); + ASSERT_EQ(jpeg_data[243], 0xC0); + EXPECT_EQ(jpeg_data[249], 0x00); + EXPECT_EQ(jpeg_data[250], 0x78); + jpeg_data[249] = 0x01; + jpeg_data[250] = 0xF0; + + // Rewind should reject the altered width and fail rather than overflowing. + EXPECT_TRUE(decoder->GetScanline(0).empty()); +} + +TEST(JpegModuleTest, RewindMismatchedComponentsFails) { + std::vector jpeg_data = LoadMonaLisa(); + ASSERT_FALSE(jpeg_data.empty()); + + std::unique_ptr decoder = + JpegModule::CreateDecoder(jpeg_data, 120, 120, 3, true, 1, false); + ASSERT_TRUE(decoder); + EXPECT_FALSE(decoder->GetScanline(0).empty()); + EXPECT_FALSE(decoder->GetScanline(1).empty()); + + // Find SOF0 marker (0xFF, 0xC0) and mutate the component count. + ASSERT_EQ(jpeg_data[242], 0xFF); + ASSERT_EQ(jpeg_data[243], 0xC0); + EXPECT_EQ(jpeg_data[251], 0x03); + jpeg_data[251] = 0x01; + + // Rewind should reject the altered component count. + EXPECT_TRUE(decoder->GetScanline(0).empty()); +} + +TEST(JpegModuleTest, RewindCorruptedDataFails) { + std::vector jpeg_data = LoadMonaLisa(); + ASSERT_FALSE(jpeg_data.empty()); + + std::unique_ptr decoder = + JpegModule::CreateDecoder(jpeg_data, 120, 120, 3, true, 1, false); + ASSERT_TRUE(decoder); + EXPECT_FALSE(decoder->GetScanline(0).empty()); + EXPECT_FALSE(decoder->GetScanline(1).empty()); + + // Corrupt the header bytes in the backing buffer. + std::ranges::fill(pdfium::span(jpeg_data).first<10>(), 0); + + // Rewind should fail cleanly. + EXPECT_TRUE(decoder->GetScanline(0).empty()); +} +#endif // !defined(PDF_ENABLE_RUST_JPEG) + TEST(JpegModuleTest, CreateDecoderDownscaled) { std::vector jpeg_data = LoadMonaLisa(); ASSERT_FALSE(jpeg_data.empty()); diff --git a/core/fxcodec/jpeg/libjpeg_scanline_decoder.cpp b/core/fxcodec/jpeg/libjpeg_scanline_decoder.cpp index 6e05c67386..f89240cf7c 100644 --- a/core/fxcodec/jpeg/libjpeg_scanline_decoder.cpp +++ b/core/fxcodec/jpeg/libjpeg_scanline_decoder.cpp @@ -235,25 +235,59 @@ bool LibjpegScanlineDecoder::CreateImpl(pdfium::span src_span, } bool LibjpegScanlineDecoder::Rewind() { - if (started_) { - jpeg_common_destroy_decompress(&common_); + if (started_ || !decompress_created_) { + const int expected_orig_width = orig_width_; + const int expected_orig_height = orig_height_; + const int expected_output_width = output_width_; + const int expected_output_height = output_height_; + const int expected_comps = comps_; + const uint32_t expected_scale_denom = scale_denom_; + const bool expected_bgr = scanlines_are_bgr_; + + if (decompress_created_) { + jpeg_common_destroy_decompress(&common_); + decompress_created_ = false; + } if (!InitDecode(/*bAcceptKnownBadHeader=*/false)) { return false; } + if (orig_width_ != expected_orig_width || + orig_height_ != expected_orig_height || + output_width_ != expected_output_width || + output_height_ != expected_output_height || + common_.cinfo.num_components != expected_comps || + scale_denom_ != expected_scale_denom || + scanlines_are_bgr_ != expected_bgr) { + jpeg_common_destroy_decompress(&common_); + decompress_created_ = false; + started_ = false; + return false; + } } + // `CreateImpl()` validated `scale_denom_` and, for non-MCU-aligned images, // reset it to 1 in `InitDecode()` (called from `CreateImpl()`, and again just // above when re-decoding), so it is safe to use as-is here. common_.cinfo.scale_denom = scale_denom_; - output_width_ = ScaledJpegSize(orig_width_, scale_denom_); - output_height_ = ScaledJpegSize(orig_height_, scale_denom_); if (!jpeg_common_start_decompress(&common_)) { jpeg_common_destroy_decompress(&common_); + decompress_created_ = false; + started_ = false; return false; } - // `CalcPitch()` sized the scanline buffer for `output_width_` columns, - // so libjpeg must not produce more than that per scanline. - CHECK_LE(static_cast(common_.cinfo.output_width), output_width_); + + // Ensure libjpeg does not produce more data than `scanline_buf_` can hold. + FX_SAFE_SIZE_T scanline_bytes = common_.cinfo.output_width; + scanline_bytes *= common_.cinfo.out_color_components; + if (!scanline_bytes.IsValid() || + scanline_bytes.ValueOrDie() > scanline_buf_.size() || + static_cast(common_.cinfo.output_width) > output_width_) { + jpeg_common_destroy_decompress(&common_); + decompress_created_ = false; + started_ = false; + return false; + } + started_ = true; return true; } From 9cbde18f9de7c12f57b35c1ed2d66cb157981603 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 5 Nov 2026 14:14:49 -0800 Subject: [PATCH 24/34] Expand unit tests for xfa/fgas/crt - Test CFGAS_Decimal integer conversion, string parsing with whitespace and sign prefixes, scale modifications with rounding, negation, float and double conversion, multiplication, and division. - Test FX_DateFromCanonical and FX_TimeFromCanonical parsing and validation edge cases. - Test CFGAS_StringFormatter date, time, and datetime subcategory formatting (e.g. short, medium, long, full). - Test CFGAS_StringFormatter time parsing with meridiem and timezone specifiers. - Test CFGAS_StringFormatter numeric formatting and parsing edge cases including negative integer formatting, overflow digit handling, and literal string escapes. - Test null and zero formatting and parsing with quoted literals and unquoted strings. - Test text formatting and parsing character validation. Change-Id: I0c44a22a582f405c4e8b557c284787b7cb0c7a98 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163430 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- xfa/fgas/crt/cfgas_decimal_unittest.cpp | 157 +++++++- .../crt/cfgas_stringformatter_unittest.cpp | 372 +++++++++++++++++- 2 files changed, 519 insertions(+), 10 deletions(-) diff --git a/xfa/fgas/crt/cfgas_decimal_unittest.cpp b/xfa/fgas/crt/cfgas_decimal_unittest.cpp index e31a5e0e14..197bc7ed3f 100644 --- a/xfa/fgas/crt/cfgas_decimal_unittest.cpp +++ b/xfa/fgas/crt/cfgas_decimal_unittest.cpp @@ -19,9 +19,19 @@ TEST(CFGASDecimalTest, Empty) { TEST(CFGASDecimalTest, FromInt32) { CFGAS_Decimal big(std::numeric_limits::max()); - CFGAS_Decimal small(std::numeric_limits::min()); EXPECT_EQ(L"2147483647", big.ToWideString()); + + CFGAS_Decimal small(std::numeric_limits::min()); EXPECT_EQ(L"-2147483648", small.ToWideString()); + + CFGAS_Decimal neg(-12345); + EXPECT_EQ(L"-12345", neg.ToWideString()); + + CFGAS_Decimal pos(12345); + EXPECT_EQ(L"12345", pos.ToWideString()); + + CFGAS_Decimal zero(0); + EXPECT_EQ(L"0", zero.ToWideString()); } TEST(CFGASDecimalTest, FromUint32) { @@ -68,12 +78,155 @@ TEST(CFGASDecimalTest, FromFloatFractional) { TEST(CFGASDecimalTest, FromString) { CFGAS_Decimal big(L"100000000000000000000000000"); - CFGAS_Decimal small(L"-1000000000000000000000000"); EXPECT_EQ(L"100000000000000000000000000", big.ToWideString()); + + CFGAS_Decimal small(L"-1000000000000000000000000"); EXPECT_EQ(L"-1000000000000000000000000", small.ToWideString()); + + CFGAS_Decimal leading_spaces(L" 456"); + EXPECT_EQ(L"456", leading_spaces.ToWideString()); + + CFGAS_Decimal leading_plus(L"+789"); + EXPECT_EQ(L"789", leading_plus.ToWideString()); + + CFGAS_Decimal spaces_plus(L" +123.45"); + EXPECT_EQ(L"123.45", spaces_plus.ToWideString()); + + CFGAS_Decimal spaces_minus(L" -123.45"); + EXPECT_EQ(L"-123.45", spaces_minus.ToWideString()); + + CFGAS_Decimal leading_dot(L".5"); + EXPECT_EQ(L"0.5", leading_dot.ToWideString()); + + CFGAS_Decimal trailing_dot(L"42."); + EXPECT_EQ(L"42", trailing_dot.ToWideString()); + + CFGAS_Decimal non_digit_term(L"123abc456"); + EXPECT_EQ(L"123", non_digit_term.ToWideString()); + + CFGAS_Decimal empty_str(L""); + EXPECT_EQ(L"0", empty_str.ToWideString()); } TEST(CFGASDecimalTest, FromString28Digits) { CFGAS_Decimal frac(L"32109876543210.0123456890123"); EXPECT_EQ(L"32109876543210.0123456890123", frac.ToWideString()); } + +TEST(CFGASDecimalTest, ScaleAndNegate) { + CFGAS_Decimal val(L"12.34"); + EXPECT_TRUE(val.IsNotZero()); + EXPECT_EQ(2, val.GetScale()); + + // Same scale is a no-op. + val.SetScale(2); + EXPECT_EQ(2, val.GetScale()); + EXPECT_EQ(L"12.34", val.ToWideString()); + + // Increase scale. + val.SetScale(4); + EXPECT_EQ(4, val.GetScale()); + EXPECT_EQ(L"12.3400", val.ToWideString()); + + // Decrease scale with rounding. + val.SetScale(1); + EXPECT_EQ(1, val.GetScale()); + EXPECT_EQ(L"12.3", val.ToWideString()); + + val.SetScale(0); + EXPECT_EQ(0, val.GetScale()); + EXPECT_EQ(L"12", val.ToWideString()); + + // Negate non-zero. + val.SetNegate(); + EXPECT_EQ(L"-12", val.ToWideString()); + val.SetNegate(); + EXPECT_EQ(L"12", val.ToWideString()); + + // Negate zero. + CFGAS_Decimal zero; + EXPECT_FALSE(zero.IsNotZero()); + zero.SetNegate(); + EXPECT_FALSE(zero.IsNotZero()); + EXPECT_EQ(L"0", zero.ToWideString()); +} + +TEST(CFGASDecimalTest, FloatDoubleConversion) { + CFGAS_Decimal int_val(123); + EXPECT_FLOAT_EQ(123.0f, int_val.ToFloat()); + EXPECT_DOUBLE_EQ(123.0, int_val.ToDouble()); + + CFGAS_Decimal pos(L"123.5"); + EXPECT_FLOAT_EQ(123.5f, pos.ToFloat()); + // TODO(tsepez): powf in ToDouble() truncates precision, causing 123.5 to + // return 123.50000184029341 rather than 123.5. + EXPECT_NE(123.5, pos.ToDouble()); + EXPECT_NEAR(123.5, pos.ToDouble(), 1e-4); + + CFGAS_Decimal neg(L"-123.5"); + EXPECT_FLOAT_EQ(-123.5f, neg.ToFloat()); + // TODO(tsepez): Same precision truncation for negative numbers. + EXPECT_NE(-123.5, neg.ToDouble()); + EXPECT_NEAR(-123.5, neg.ToDouble(), 1e-4); +} + +TEST(CFGASDecimalTest, Multiply) { + CFGAS_Decimal zero; + CFGAS_Decimal num(L"42"); + EXPECT_EQ(L"0", (zero * num).ToWideString()); + EXPECT_EQ(L"0", (num * zero).ToWideString()); + + CFGAS_Decimal a(12); + CFGAS_Decimal b(34); + EXPECT_EQ(L"408", (a * b).ToWideString()); + + CFGAS_Decimal c(-12); + EXPECT_EQ(L"-408", (c * b).ToWideString()); + EXPECT_EQ(L"408", (c * CFGAS_Decimal(-34)).ToWideString()); + + CFGAS_Decimal frac1(L"1.5"); + CFGAS_Decimal frac2(L"2.5"); + EXPECT_EQ(L"3.75", (frac1 * frac2).ToWideString()); + + // Multiplying numbers with high precision triggers scale shrinking. + CFGAS_Decimal hi1(L"12345678901234.12345678901234"); + CFGAS_Decimal hi2(L"98765432109876.54321098765432"); + CFGAS_Decimal product = hi1 * hi2; + EXPECT_TRUE(product.IsNotZero()); + EXPECT_LE(product.GetScale(), 28); +} + +TEST(CFGASDecimalTest, Divide) { + CFGAS_Decimal zero; + CFGAS_Decimal num(100); + + // Division by zero. + EXPECT_EQ(L"0", (num / zero).ToWideString()); + + // Zero divided by non-zero. + EXPECT_EQ(L"0", (zero / num).ToWideString()); + + // Integer division. + EXPECT_EQ(L"25", (num / CFGAS_Decimal(4)).ToWideString()); + + // Smaller scale divided by larger scale. + CFGAS_Decimal d1(10); + CFGAS_Decimal d2(L"2.5"); + EXPECT_EQ(L"4", (d1 / d2).ToWideString()); + + // Larger scale divided by smaller scale maintains scale difference. + CFGAS_Decimal d3(L"10.0"); + CFGAS_Decimal d4(2); + EXPECT_EQ(L"5.0", (d3 / d4).ToWideString()); + + // Repeating fraction. + CFGAS_Decimal one(1); + CFGAS_Decimal three(3); + CFGAS_Decimal div_result = one / three; + EXPECT_EQ(L"0.3333333333333333333333333333", div_result.ToWideString()); + + // Negative operands. + EXPECT_EQ(L"-5", (CFGAS_Decimal(-10) / CFGAS_Decimal(2)).ToWideString()); + EXPECT_EQ(L"-5", (CFGAS_Decimal(10) / CFGAS_Decimal(-2)).ToWideString()); + EXPECT_EQ(L"5", (CFGAS_Decimal(-10) / CFGAS_Decimal(-2)).ToWideString()); +} diff --git a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp index f657c07ca1..cfebcc7e9d 100644 --- a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp +++ b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp @@ -691,7 +691,12 @@ TEST_F(CFGASStringFormatterTest, NullFormat) { static const struct { const wchar_t* pattern; const wchar_t* output; - } kTests[] = {{L"null{'n/a'}", L"n/a"}, {L"null{}", L""}}; + } kTests[] = { + {L"null{'n/a'}", L"n/a"}, + {L"null{}", L""}, + {L"null{None}", L"None"}, + {L"null{'Pre'Post}", L"PrePost"}, + }; size_t i = 0; for (const auto& test : kTests) { @@ -701,6 +706,9 @@ TEST_F(CFGASStringFormatterTest, NullFormat) { EXPECT_EQ(test.output, result) << " TEST: " << i; ++i; } + + WideString empty_result; + EXPECT_FALSE(CFGAS_StringFormatter(L"").FormatNull(&empty_result)); } TEST_F(CFGASStringFormatterTest, ZeroParse) { @@ -715,19 +723,20 @@ TEST_F(CFGASStringFormatterTest, ZeroParse) { EXPECT_TRUE(fmt.ParseZero(test.input)) << " TEST: " << i; ++i; } + + EXPECT_FALSE(CFGAS_StringFormatter(L"zero{'0'}").ParseZero(L"1")); + EXPECT_FALSE(CFGAS_StringFormatter(L"zero{0}").ParseZero(L"1")); } TEST_F(CFGASStringFormatterTest, ZeroFormat) { static const struct { - const wchar_t* input; const wchar_t* pattern; const wchar_t* output; - } kTests[] = {// TODO(dsinclair): The zero format can take a number specifier - // which we don't take into account. - // {L"", L"zero {9}", L""}, - // {L"0", L"zero {9}", L"0"}, - // {L"0.0", L"zero{9}", L"0"}, - {L"0", L"zero{}", L""}}; + } kTests[] = { + {L"zero{}", L""}, + {L"zero{'Zero'}", L"Zero"}, + {L"zero{Zero}", L"Zero"}, + }; size_t i = 0; for (const auto& test : kTests) { @@ -737,6 +746,9 @@ TEST_F(CFGASStringFormatterTest, ZeroFormat) { EXPECT_EQ(test.output, result) << " TEST: " << i; ++i; } + + WideString empty_result; + EXPECT_FALSE(CFGAS_StringFormatter(L"").FormatZero(&empty_result)); } TEST_F(CFGASStringFormatterTest, GetCategory) { @@ -765,3 +777,347 @@ TEST_F(CFGASStringFormatterTest, GetCategory) { EXPECT_EQ(CFGAS_StringFormatter::Category::kDate, CFGAS_StringFormatter(L"date.long{}").GetCategory()); } + +TEST_F(CFGASStringFormatterTest, CanonicalDate) { + CFX_DateTime dt; + + // Valid dates. + EXPECT_TRUE(FX_DateFromCanonical(WideStringView(L"2020-01-15").span(), &dt)); + EXPECT_EQ(2020, dt.GetYear()); + EXPECT_EQ(1, dt.GetMonth()); + EXPECT_EQ(15, dt.GetDay()); + + // Year only defaults to month 1, day 1. + EXPECT_TRUE(FX_DateFromCanonical(WideStringView(L"2020").span(), &dt)); + EXPECT_EQ(2020, dt.GetYear()); + EXPECT_EQ(1, dt.GetMonth()); + EXPECT_EQ(1, dt.GetDay()); + + // Year-month defaults to day 1. + EXPECT_TRUE(FX_DateFromCanonical(WideStringView(L"2020-05").span(), &dt)); + EXPECT_EQ(2020, dt.GetYear()); + EXPECT_EQ(5, dt.GetMonth()); + EXPECT_EQ(1, dt.GetDay()); + + // Leap year Feb 29. + EXPECT_TRUE(FX_DateFromCanonical(WideStringView(L"2020-02-29").span(), &dt)); + EXPECT_EQ(29, dt.GetDay()); + + // Non-leap year Feb 28. + EXPECT_TRUE(FX_DateFromCanonical(WideStringView(L"2021-02-28").span(), &dt)); + EXPECT_EQ(28, dt.GetDay()); + + // Invalid: length > 10. + EXPECT_FALSE( + FX_DateFromCanonical(WideStringView(L"2020-01-01-extra").span(), &dt)); + + // Invalid: year < 1900. + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"1899-01-01").span(), &dt)); + + // Invalid: non-numeric year. + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"abcd-01-01").span(), &dt)); + + // Invalid: month out of range. + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"2020-00-01").span(), &dt)); + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"2020-13-01").span(), &dt)); + + // Invalid: day out of range. + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"2020-01-00").span(), &dt)); + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"2020-01-32").span(), &dt)); + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"2020-04-31").span(), &dt)); + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"2021-02-29").span(), &dt)); + EXPECT_FALSE(FX_DateFromCanonical(WideStringView(L"2020-02-30").span(), &dt)); +} + +TEST_F(CFGASStringFormatterTest, CanonicalTime) { + LocaleIface* locale = Mgr(L"en")->GetDefLocale(); + CFX_DateTime dt; + + // Valid times. + EXPECT_TRUE( + FX_TimeFromCanonical(locale, WideStringView(L"14:30:15").span(), &dt)); + EXPECT_EQ(14, dt.GetHour()); + EXPECT_EQ(30, dt.GetMinute()); + EXPECT_EQ(15, dt.GetSecond()); + EXPECT_EQ(0, dt.GetMillisecond()); + + // Hour only. + EXPECT_TRUE(FX_TimeFromCanonical(locale, WideStringView(L"14").span(), &dt)); + EXPECT_EQ(14, dt.GetHour()); + EXPECT_EQ(0, dt.GetMinute()); + + // Hour and minute only. + EXPECT_TRUE( + FX_TimeFromCanonical(locale, WideStringView(L"14:30").span(), &dt)); + EXPECT_EQ(14, dt.GetHour()); + EXPECT_EQ(30, dt.GetMinute()); + + // With milliseconds. + EXPECT_TRUE(FX_TimeFromCanonical( + locale, WideStringView(L"14:30:15.123").span(), &dt)); + EXPECT_EQ(123, dt.GetMillisecond()); + + // With time zone offsets. + EXPECT_TRUE( + FX_TimeFromCanonical(locale, WideStringView(L"14:30:15Z").span(), &dt)); + EXPECT_TRUE(FX_TimeFromCanonical( + locale, WideStringView(L"14:30:15+02:00").span(), &dt)); + EXPECT_TRUE(FX_TimeFromCanonical( + locale, WideStringView(L"14:30:15-05:00").span(), &dt)); + + // Invalid: empty. + EXPECT_FALSE(FX_TimeFromCanonical(locale, WideStringView(L"").span(), &dt)); + + // Invalid: hour >= 24. + EXPECT_FALSE( + FX_TimeFromCanonical(locale, WideStringView(L"24:00:00").span(), &dt)); + + // Invalid: minute >= 60. + EXPECT_FALSE( + FX_TimeFromCanonical(locale, WideStringView(L"14:60:00").span(), &dt)); + + // Invalid: second >= 60. + EXPECT_FALSE( + FX_TimeFromCanonical(locale, WideStringView(L"14:30:60").span(), &dt)); + + // Invalid: incomplete milliseconds. + EXPECT_FALSE( + FX_TimeFromCanonical(locale, WideStringView(L"14:30:15.12").span(), &dt)); +} + +TEST_F(CFGASStringFormatterTest, TimeParse) { + struct { + const wchar_t* locale; + const wchar_t* input; + const wchar_t* pattern; + CFX_DateTime output; + } kTests[] = { + {L"en", L"18:30:45", L"time{HH:MM:SS}", + CFX_DateTime(0, 0, 0, 18, 30, 45, 0)}, + {L"en", L"8:5:6", L"time{H:M:S}", CFX_DateTime(0, 0, 0, 8, 5, 6, 0)}, + {L"en", L"1:05:10 PM", L"time{h:MM:SS A}", + CFX_DateTime(0, 0, 0, 13, 5, 10, 0)}, + {L"en", L"12:05:10 AM", L"time{h:MM:SS A}", + CFX_DateTime(0, 0, 0, 0, 5, 10, 0)}, + {L"en", L"12:05:10 PM", L"time{h:MM:SS A}", + CFX_DateTime(0, 0, 0, 12, 5, 10, 0)}, + {L"en", L"14:30:45.123", L"time{HH:MM:SS.FFF}", + CFX_DateTime(0, 0, 0, 14, 30, 45, 123)}, + {L"en", L"At 14:30:45", L"time{'At 'HH:MM:SS}", + CFX_DateTime(0, 0, 0, 14, 30, 45, 0)}, + {L"en", L"14:30:45 Z", L"time{HH:MM:SS z}", + CFX_DateTime(0, 0, 0, 14, 30, 45, 0)}, + {L"en", L"14:30:45 EST", L"time{HH:MM:SS Z}", + CFX_DateTime(0, 0, 0, 9, 30, 45, 0)}, + }; + + for (const auto& test : kTests) { + CFX_DateTime result; + CFGAS_StringFormatter fmt(test.pattern); + EXPECT_TRUE(fmt.ParseDateTime(Mgr(test.locale), test.input, + CFGAS_StringFormatter::DateTimeType::kTime, + &result)) + << " TEST: " << test.locale << ", " << test.input; + EXPECT_EQ(test.output, result) + << " TEST: " << test.locale << ", " << test.input; + } +} + +TEST_F(CFGASStringFormatterTest, DateTimeSubcategoryFormat) { + WideString result; + + // Date subcategories. + CFGAS_StringFormatter fmt_date_short(L"date.short{}"); + EXPECT_TRUE(fmt_date_short.FormatDateTime( + Mgr(L"en"), L"2002-10-25", CFGAS_StringFormatter::DateTimeType::kDate, + &result)); + EXPECT_EQ(L"10/25/02", result); + + CFGAS_StringFormatter fmt_date_med(L"date.medium{}"); + EXPECT_TRUE(fmt_date_med.FormatDateTime( + Mgr(L"en"), L"2002-10-25", CFGAS_StringFormatter::DateTimeType::kDate, + &result)); + EXPECT_EQ(L"Oct 25, 2002", result); + + CFGAS_StringFormatter fmt_date_long(L"date.long{}"); + EXPECT_TRUE(fmt_date_long.FormatDateTime( + Mgr(L"en"), L"2002-10-25", CFGAS_StringFormatter::DateTimeType::kDate, + &result)); + EXPECT_EQ(L"October 25, 2002", result); + + CFGAS_StringFormatter fmt_date_full(L"date.full{}"); + EXPECT_TRUE(fmt_date_full.FormatDateTime( + Mgr(L"en"), L"2002-10-25", CFGAS_StringFormatter::DateTimeType::kDate, + &result)); + EXPECT_EQ(L"Friday, October 25, 2002", result); + + // Time subcategories. + CFGAS_StringFormatter fmt_time_short(L"time.short{}"); + EXPECT_TRUE(fmt_time_short.FormatDateTime( + Mgr(L"en"), L"14:30:00", CFGAS_StringFormatter::DateTimeType::kTime, + &result)); + EXPECT_EQ(L"2:30 PM", result); + + CFGAS_StringFormatter fmt_time_med(L"time.medium{}"); + EXPECT_TRUE(fmt_time_med.FormatDateTime( + Mgr(L"en"), L"14:30:00", CFGAS_StringFormatter::DateTimeType::kTime, + &result)); + EXPECT_EQ(L"2:30:00 PM", result); + + // DateTime subcategories format with directly concatenated patterns. + CFGAS_StringFormatter fmt_dt_short(L"datetime.short{}"); + EXPECT_TRUE(fmt_dt_short.FormatDateTime( + Mgr(L"en"), L"2002-10-25T14:30:00", + CFGAS_StringFormatter::DateTimeType::kDateTime, &result)); + EXPECT_EQ(L"10/25/022:30 PM", result); + + CFGAS_StringFormatter fmt_dt_med(L"datetime.medium{}"); + EXPECT_TRUE(fmt_dt_med.FormatDateTime( + Mgr(L"en"), L"2002-10-25T14:30:00", + CFGAS_StringFormatter::DateTimeType::kDateTime, &result)); + EXPECT_EQ(L"Oct 25, 20022:30:00 PM", result); +} + +TEST_F(CFGASStringFormatterTest, TextFormatAndParseEdgeCases) { + WideString result; + + // FormatText empty cases. + EXPECT_FALSE(CFGAS_StringFormatter(L"").FormatText(L"abc", &result)); + EXPECT_FALSE(CFGAS_StringFormatter(L"text{AAA}").FormatText(L"", &result)); + + // FormatText character validation. + // 'A' requires alpha. + EXPECT_FALSE(CFGAS_StringFormatter(L"text{AAA}").FormatText(L"123", &result)); + // '9' requires digit. + EXPECT_FALSE(CFGAS_StringFormatter(L"text{999}").FormatText(L"abc", &result)); + // 'O' requires alnum. + EXPECT_FALSE(CFGAS_StringFormatter(L"text{OOO}").FormatText(L"...", &result)); + // Length mismatch (input too long). + EXPECT_FALSE(CFGAS_StringFormatter(L"text{A}").FormatText(L"AB", &result)); + // Length mismatch (input too short). + EXPECT_FALSE(CFGAS_StringFormatter(L"text{AAA}").FormatText(L"A", &result)); + + // ParseText empty cases. + EXPECT_FALSE(CFGAS_StringFormatter(L"").ParseText(L"abc", &result)); + EXPECT_FALSE(CFGAS_StringFormatter(L"text{AAA}").ParseText(L"", &result)); + EXPECT_FALSE(CFGAS_StringFormatter(L"num{999}").ParseText(L"123", &result)); + + // ParseText literal mismatch. + EXPECT_FALSE(CFGAS_StringFormatter(L"text{'prefix 'AAA}") + .ParseText(L"other ABC", &result)); + + // ParseText non-symbol mismatch. + EXPECT_FALSE( + CFGAS_StringFormatter(L"text{AAA-999}").ParseText(L"ABC:123", &result)); +} + +TEST_F(CFGASStringFormatterTest, NumFormatAndParseEdgeCases) { + WideString result; + + // FormatNum empty cases. + EXPECT_FALSE( + CFGAS_StringFormatter(L"").FormatNum(Mgr(L"en"), L"123", &result)); + EXPECT_FALSE( + CFGAS_StringFormatter(L"num{999}").FormatNum(Mgr(L"en"), L"", &result)); + + // ParseNum empty cases. + EXPECT_FALSE( + CFGAS_StringFormatter(L"").ParseNum(Mgr(L"en"), L"123", &result)); + EXPECT_FALSE( + CFGAS_StringFormatter(L"num{999}").ParseNum(Mgr(L"en"), L"", &result)); + + // Integer negative formatting without +/- in pattern. + result.clear(); + EXPECT_TRUE( + CFGAS_StringFormatter(L"999").FormatNum(Mgr(L"en"), L"-123", &result)); + EXPECT_EQ(WideString(L"-123"), result); + + // Integer negative with 'z' suppression. + result.clear(); + EXPECT_TRUE( + CFGAS_StringFormatter(L"zzz").FormatNum(Mgr(L"en"), L"-45", &result)); + EXPECT_EQ(WideString(L"-45"), result); + + // Overflow digits before decimal dot rounds to integer. + result.clear(); + EXPECT_TRUE( + CFGAS_StringFormatter(L"99").FormatNum(Mgr(L"en"), L"1234.56", &result)); + EXPECT_EQ(WideString(L"1,235"), result); + + // Overflow negative before decimal dot. + result.clear(); + EXPECT_TRUE( + CFGAS_StringFormatter(L"99").FormatNum(Mgr(L"en"), L"-1234.56", &result)); + EXPECT_EQ(WideString(L"-1,235"), result); + + // Literal in format pattern with quotes and unicode escapes. + result.clear(); + EXPECT_TRUE(CFGAS_StringFormatter(L"num{'Cost: \\u0024'999.99}") + .FormatNum(Mgr(L"en"), L"123.45", &result)); + EXPECT_EQ(WideString(L"Cost: $123.45"), result); + + // ParseNum with leading negative sign. + result.clear(); + EXPECT_TRUE( + CFGAS_StringFormatter(L"999").ParseNum(Mgr(L"en"), L"-123", &result)); + EXPECT_EQ(WideString(L"-123"), result); + + // ParseNum with currency symbol. + result.clear(); + EXPECT_TRUE( + CFGAS_StringFormatter(L"$999").ParseNum(Mgr(L"en"), L"$123", &result)); + EXPECT_EQ(WideString(L"123"), result); + + // ParseNum with literal in pattern. + result.clear(); + EXPECT_TRUE(CFGAS_StringFormatter(L"num{'Amt: '999}") + .ParseNum(Mgr(L"en"), L"Amt: 123", &result)); + EXPECT_EQ(WideString(L"123"), result); +} + +TEST_F(CFGASStringFormatterTest, DateTimeParse) { + CFX_DateTime dt; + + // TimeDate parses time first, then date. + EXPECT_TRUE(CFGAS_StringFormatter(L"time{HH:MM:SS} date{YYYY-MM-DD}") + .ParseDateTime(Mgr(L"en"), L"14:30:00 2020-05-15", + CFGAS_StringFormatter::DateTimeType::kTimeDate, + &dt)); + EXPECT_EQ(CFX_DateTime(2020, 5, 15, 14, 30, 0, 0), dt); + + // DateTime parses date first, then time. + // TODO(tsepez): ParseDateTime() inverts wsDatePattern and wsTimePattern for + // DateTimeType::kDateTime, passing wsTimePattern to ParseLocaleDate() and + // wsDatePattern to ParseLocaleTime(), which causes parsing to fail. + EXPECT_FALSE( + CFGAS_StringFormatter(L"date{YYYY-MM-DD} time{HH:MM:SS}") + .ParseDateTime(Mgr(L"en"), L"2020-05-15 14:30:00", + CFGAS_StringFormatter::DateTimeType::kDateTime, &dt)); +} + +TEST_F(CFGASStringFormatterTest, FormatDateTimeEdgeCases) { + WideString result; + + // Empty pattern. + EXPECT_FALSE(CFGAS_StringFormatter(L"").FormatDateTime( + Mgr(L"en"), L"2020-01-01", CFGAS_StringFormatter::DateTimeType::kDate, + &result)); + + // Unknown category. + EXPECT_FALSE(CFGAS_StringFormatter(L"unknown{}") + .FormatDateTime( + Mgr(L"en"), L"2020-01-01", + CFGAS_StringFormatter::DateTimeType::kUnknown, &result)); + + // ParseDateTime empty pattern. + CFX_DateTime dt; + EXPECT_FALSE(CFGAS_StringFormatter(L"").ParseDateTime( + Mgr(L"en"), L"2020-01-01", CFGAS_StringFormatter::DateTimeType::kDate, + &dt)); + + // ParseDateTime unknown category. + EXPECT_FALSE(CFGAS_StringFormatter(L"unknown{}") + .ParseDateTime(Mgr(L"en"), L"2020-01-01", + CFGAS_StringFormatter::DateTimeType::kUnknown, + &dt)); +} From b1530663588836705d61a9fb6476ae8a4124c547 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 5 Nov 2026 15:05:31 -0800 Subject: [PATCH 25/34] Improve unit tests for core/fxge Add unit tests to improve code coverage across fxge modules: - cfx_color: Add test coverage for ConvertColorType() covering conversions among Transparent, Gray, RGB, CMYK, and out-of-range inputs. - fx_freetype: Add unit tests for FXFT_unicode_from_adobe_name(), FXFT_adobe_name_from_unicode(), and FreeTypeVersionSupportsHinting() guarded under pdf_enable_freetype. - cfx_cttnametable: Add unit tests for standalone TrueType name table parsing guarded under pdf_enable_xfa. Change-Id: I563ee66d2ba9973fc53ea695d151515207f4c946 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163110 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- core/fxge/BUILD.gn | 8 ++ core/fxge/cfx_color_unittest.cpp | 86 +++++++++++++++++++++ core/fxge/cfx_cttnametable_unittest.cpp | 82 ++++++++++++++++++++ core/fxge/freetype/fx_freetype_unittest.cpp | 81 +++++++++++++++++++ 4 files changed, 257 insertions(+) create mode 100644 core/fxge/cfx_cttnametable_unittest.cpp create mode 100644 core/fxge/freetype/fx_freetype_unittest.cpp diff --git a/core/fxge/BUILD.gn b/core/fxge/BUILD.gn index f8c85ead16..93de99d1b7 100644 --- a/core/fxge/BUILD.gn +++ b/core/fxge/BUILD.gn @@ -334,6 +334,14 @@ pdfium_unittest_source_set("unittests") { sources += [ "agg/cfx_agg_cliprgn_unittest.cpp" ] } + if (pdf_enable_freetype) { + sources += [ "freetype/fx_freetype_unittest.cpp" ] + } + + if (pdf_enable_xfa) { + sources += [ "cfx_cttnametable_unittest.cpp" ] + } + if (pdf_enable_fontations) { sources += [ "skrifa/fx_skrifa_unittest.cpp" ] deps += [ diff --git a/core/fxge/cfx_color_unittest.cpp b/core/fxge/cfx_color_unittest.cpp index 6adc04355e..f442f5e34a 100644 --- a/core/fxge/cfx_color_unittest.cpp +++ b/core/fxge/cfx_color_unittest.cpp @@ -59,3 +59,89 @@ TEST(CFX_ColorTest, OperatorSlashUnusedComponents) { CFX_Color c3 = CFX_Color::MakeGray(0.4f); EXPECT_EQ(c2, c3); } + +TEST(CFX_ColorTest, ConvertColorType) { + // Transparent + CFX_Color trans; + EXPECT_EQ(CFX_Color::Type::kTransparent, + trans.ConvertColorType(CFX_Color::Type::kTransparent).nColorType); + EXPECT_EQ(CFX_Color::Type::kTransparent, + trans.ConvertColorType(CFX_Color::Type::kGray).nColorType); + EXPECT_EQ(CFX_Color::Type::kTransparent, + trans.ConvertColorType(CFX_Color::Type::kRGB).nColorType); + EXPECT_EQ(CFX_Color::Type::kTransparent, + trans.ConvertColorType(CFX_Color::Type::kCMYK).nColorType); + + // Gray + CFX_Color gray = CFX_Color::MakeGray(0.6f); + EXPECT_EQ(CFX_Color::Type::kTransparent, + gray.ConvertColorType(CFX_Color::Type::kTransparent).nColorType); + EXPECT_EQ(gray, gray.ConvertColorType(CFX_Color::Type::kGray)); + + CFX_Color gray_to_rgb = gray.ConvertColorType(CFX_Color::Type::kRGB); + EXPECT_EQ(CFX_Color::Type::kRGB, gray_to_rgb.nColorType); + EXPECT_FLOAT_EQ(0.6f, gray_to_rgb.fColor1); + EXPECT_FLOAT_EQ(0.6f, gray_to_rgb.fColor2); + EXPECT_FLOAT_EQ(0.6f, gray_to_rgb.fColor3); + + CFX_Color gray_to_cmyk = gray.ConvertColorType(CFX_Color::Type::kCMYK); + EXPECT_EQ(CFX_Color::Type::kCMYK, gray_to_cmyk.nColorType); + EXPECT_FLOAT_EQ(0.0f, gray_to_cmyk.fColor1); + EXPECT_FLOAT_EQ(0.0f, gray_to_cmyk.fColor2); + EXPECT_FLOAT_EQ(0.0f, gray_to_cmyk.fColor3); + EXPECT_FLOAT_EQ(0.4f, gray_to_cmyk.fColor4); + + // RGB + CFX_Color rgb = CFX_Color::MakeRGB(0.2f, 0.4f, 0.6f); + EXPECT_EQ(CFX_Color::Type::kTransparent, + rgb.ConvertColorType(CFX_Color::Type::kTransparent).nColorType); + EXPECT_EQ(rgb, rgb.ConvertColorType(CFX_Color::Type::kRGB)); + + CFX_Color rgb_to_gray = rgb.ConvertColorType(CFX_Color::Type::kGray); + EXPECT_EQ(CFX_Color::Type::kGray, rgb_to_gray.nColorType); + EXPECT_FLOAT_EQ(0.3f * 0.2f + 0.59f * 0.4f + 0.11f * 0.6f, + rgb_to_gray.fColor1); + + CFX_Color rgb_to_cmyk = rgb.ConvertColorType(CFX_Color::Type::kCMYK); + EXPECT_EQ(CFX_Color::Type::kCMYK, rgb_to_cmyk.nColorType); + EXPECT_FLOAT_EQ(0.8f, rgb_to_cmyk.fColor1); + EXPECT_FLOAT_EQ(0.6f, rgb_to_cmyk.fColor2); + EXPECT_FLOAT_EQ(0.4f, rgb_to_cmyk.fColor3); + EXPECT_FLOAT_EQ(0.4f, rgb_to_cmyk.fColor4); + + // CMYK + CFX_Color cmyk = CFX_Color::MakeCMYK(0.1f, 0.2f, 0.3f, 0.4f); + EXPECT_EQ(CFX_Color::Type::kTransparent, + cmyk.ConvertColorType(CFX_Color::Type::kTransparent).nColorType); + EXPECT_EQ(cmyk, cmyk.ConvertColorType(CFX_Color::Type::kCMYK)); + + CFX_Color cmyk_to_gray = cmyk.ConvertColorType(CFX_Color::Type::kGray); + EXPECT_EQ(CFX_Color::Type::kGray, cmyk_to_gray.nColorType); + EXPECT_FLOAT_EQ(1.0f - (0.3f * 0.1f + 0.59f * 0.2f + 0.11f * 0.3f + 0.4f), + cmyk_to_gray.fColor1); + + CFX_Color cmyk_to_rgb = cmyk.ConvertColorType(CFX_Color::Type::kRGB); + EXPECT_EQ(CFX_Color::Type::kRGB, cmyk_to_rgb.nColorType); + EXPECT_FLOAT_EQ(0.5f, cmyk_to_rgb.fColor1); + EXPECT_FLOAT_EQ(0.4f, cmyk_to_rgb.fColor2); + EXPECT_FLOAT_EQ(0.3f, cmyk_to_rgb.fColor3); + + // Out-of-range inputs exercise fallback paths. + CFX_Color invalid_gray = CFX_Color::MakeGray(-0.5f); + EXPECT_EQ(CFX_Color::MakeRGB(0.0f, 0.0f, 0.0f), + invalid_gray.ConvertColorType(CFX_Color::Type::kRGB)); + EXPECT_EQ(CFX_Color::MakeCMYK(0.0f, 0.0f, 0.0f, 0.0f), + invalid_gray.ConvertColorType(CFX_Color::Type::kCMYK)); + + CFX_Color invalid_rgb = CFX_Color::MakeRGB(2.0f, 0.0f, 0.0f); + EXPECT_EQ(CFX_Color::MakeGray(0.0f), + invalid_rgb.ConvertColorType(CFX_Color::Type::kGray)); + EXPECT_EQ(CFX_Color::MakeCMYK(0.0f, 0.0f, 0.0f, 0.0f), + invalid_rgb.ConvertColorType(CFX_Color::Type::kCMYK)); + + CFX_Color invalid_cmyk = CFX_Color::MakeCMYK(0.0f, -1.0f, 0.0f, 0.0f); + EXPECT_EQ(CFX_Color::MakeGray(0.0f), + invalid_cmyk.ConvertColorType(CFX_Color::Type::kGray)); + EXPECT_EQ(CFX_Color::MakeRGB(0.0f, 0.0f, 0.0f), + invalid_cmyk.ConvertColorType(CFX_Color::Type::kRGB)); +} diff --git a/core/fxge/cfx_cttnametable_unittest.cpp b/core/fxge/cfx_cttnametable_unittest.cpp new file mode 100644 index 0000000000..1958d69bb6 --- /dev/null +++ b/core/fxge/cfx_cttnametable_unittest.cpp @@ -0,0 +1,82 @@ +// Copyright 2026 The PDFium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxge/cfx_cttnametable.h" + +#include + +#include + +#include "core/fxcrt/widestring.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::ElementsAre; + +TEST(CFX_CTTNameTableTest, Empty) { + CFX_CTTNameTable table; + EXPECT_TRUE(table.GetFamilyNames().empty()); + + CFX_CTTNameTable empty_table(pdfium::span{}); + EXPECT_TRUE(empty_table.GetFamilyNames().empty()); +} + +TEST(CFX_CTTNameTableTest, AddFamilyName) { + CFX_CTTNameTable table; + table.AddFamilyName(L"First"); + table.AddFamilyName(L"Second"); + ASSERT_THAT(table.GetFamilyNames(), ElementsAre(L"First", L"Second")); +} + +TEST(CFX_CTTNameTableTest, ParseMacintoshRecord) { + // Construct a minimal TT name table with 1 name record for Macintosh + // (Platform 1). + // Header: + // format: 0 (2 bytes) + // count: 1 (2 bytes) + // string_offset: 18 (2 bytes) + // NameRecord 0 (12 bytes at offset 6): + // platform_id: 1 (2 bytes) + // encoding_id: 0 (2 bytes) + // language_id: 0 (2 bytes) + // name_id: 1 (2 bytes) + // length: 4 (2 bytes) + // offset: 0 (2 bytes) + // String storage (starts at offset 18): + // "Test\0" (5 bytes, extra byte to satisfy >= bound check) + static constexpr uint8_t kTableData[] = { + 0x00, 0x00, // format + 0x00, 0x01, // count = 1 + 0x00, 0x12, // string_offset = 18 + 0x00, 0x01, // platform_id = 1 + 0x00, 0x00, // encoding_id = 0 + 0x00, 0x00, // language_id = 0 + 0x00, 0x01, // name_id = 1 + 0x00, 0x04, // length = 4 + 0x00, 0x00, // offset = 0 + 'T', 'e', 's', 't', 0 // string data + }; + + CFX_CTTNameTable table(kTableData); + EXPECT_THAT(table.GetFamilyNames(), ElementsAre(L"Test")); +} + +TEST(CFX_CTTNameTableTest, ParseOutOfBoundsRecord) { + // Same table, but offset points beyond string storage. + static constexpr uint8_t kTableData[] = { + 0x00, 0x00, // format + 0x00, 0x01, // count = 1 + 0x00, 0x12, // string_offset = 18 + 0x00, 0x01, // platform_id = 1 + 0x00, 0x00, // encoding_id = 0 + 0x00, 0x00, // language_id = 0 + 0x00, 0x01, // name_id = 1 + 0x00, 0x04, // length = 4 + 0x00, 0x20, // offset = 32 (out of bounds) + 'T', 'e', 's', 't', 0 // string data + }; + + CFX_CTTNameTable table(kTableData); + EXPECT_TRUE(table.GetFamilyNames().empty()); +} diff --git a/core/fxge/freetype/fx_freetype_unittest.cpp b/core/fxge/freetype/fx_freetype_unittest.cpp new file mode 100644 index 0000000000..2740b35592 --- /dev/null +++ b/core/fxge/freetype/fx_freetype_unittest.cpp @@ -0,0 +1,81 @@ +// Copyright 2026 The PDFium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxge/freetype/fx_freetype.h" + +#include + +#include + +#include "testing/gtest/include/gtest/gtest.h" + +TEST(FXFreeTypeTest, VersionSupportsHinting) { + ScopedFXFTLibraryRec library = InitializeFreeType(); + ASSERT_TRUE(library); + EXPECT_TRUE(FreeTypeVersionSupportsHinting(library.get())); +} + +TEST(FXFreeTypeTest, UnicodeFromAdobeName) { + // Empty or invalid names. + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("")); + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("not_a_valid_name")); + EXPECT_EQ(0, FXFT_unicode_from_adobe_name(".notdef")); + + // uniXXXX format. + EXPECT_EQ(0x0041, FXFT_unicode_from_adobe_name("uni0041")); + EXPECT_EQ(0x0020, FXFT_unicode_from_adobe_name("uni0020")); + EXPECT_EQ(0xFFFF, FXFT_unicode_from_adobe_name("uniFFFF")); + // Invalid uni formats (non-hex, wrong length). + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("uni")); + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("uni004")); + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("uni004G")); + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("uni0041extra")); + + // uniXXXX with dot variant (bit 31 set). + static constexpr uint32_t kVariantBit = 1U << 31; + EXPECT_EQ(static_cast(0x0041 | kVariantBit), + FXFT_unicode_from_adobe_name("uni0041.swash")); + + // uXXXX format (4 to 6 hex digits). + EXPECT_EQ(0x0041, FXFT_unicode_from_adobe_name("u0041")); + EXPECT_EQ(0x1F600, FXFT_unicode_from_adobe_name("u1F600")); + EXPECT_EQ(0x10FFFF, FXFT_unicode_from_adobe_name("u10FFFF")); + // Invalid u formats. + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("u1")); + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("u12")); + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("u123")); + EXPECT_EQ(0, FXFT_unicode_from_adobe_name("u0041G")); + + // uXXXX with dot variant. + EXPECT_EQ(static_cast(0x0041 | kVariantBit), + FXFT_unicode_from_adobe_name("u0041.alt")); + EXPECT_EQ(static_cast(0x1F600 | kVariantBit), + FXFT_unicode_from_adobe_name("u1F600.variant")); + + // Standard Adobe glyph names. + EXPECT_EQ(0x0041, FXFT_unicode_from_adobe_name("A")); + EXPECT_EQ(0x0020, FXFT_unicode_from_adobe_name("space")); + EXPECT_EQ(0x0026, FXFT_unicode_from_adobe_name("ampersand")); + + // Standard Adobe glyph names with dot variant. + EXPECT_EQ(static_cast(0x0041 | kVariantBit), + FXFT_unicode_from_adobe_name("A.swash")); +} + +TEST(FXFreeTypeTest, AdobeNameFromUnicode) { + std::array buffer; + + FXFT_adobe_name_from_unicode(buffer, L'A'); + EXPECT_STREQ("A", buffer.data()); + + FXFT_adobe_name_from_unicode(buffer, L' '); + EXPECT_STREQ("space", buffer.data()); + + FXFT_adobe_name_from_unicode(buffer, L'&'); + EXPECT_STREQ("ampersand", buffer.data()); + + // Non-existent unicode point. + FXFT_adobe_name_from_unicode(buffer, 0xFFFF); + EXPECT_EQ('\0', buffer[0]); +} From 21d5499474663618afa756e0b8f1649cb47b21b2 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 5 Nov 2026 15:34:04 -0800 Subject: [PATCH 26/34] Fix CFGAS_Decimal::ToDouble() precision and ParseDateTime() arguments - Use pow() instead of powf() in CFGAS_Decimal::ToDouble() to avoid truncating double precision down to single-precision float mantissa. Also remove the C-style cast when scaling 32-bit limbs. - Fix transposed pattern arguments in CFGAS_StringFormatter::ParseDateTime() for DateTimeType::kDateTime, where wsTimePattern was passed to ParseLocaleDate() and wsDatePattern was passed to ParseLocaleTime(). - Allow ParseLocaleDate() to leave unconsumed characters for subsequent time parsing in combined date-time strings, and check that the full input string was consumed upon completing ParseDateTime(). - Update test expectations to verify the corrected behaviors. Change-Id: I89a5265fb9d7e91e9dd3c8a3386b1dc995294ac7 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163450 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- xfa/fgas/crt/cfgas_decimal.cpp | 8 ++--- xfa/fgas/crt/cfgas_decimal_unittest.cpp | 9 ++--- xfa/fgas/crt/cfgas_stringformatter.cpp | 33 ++++++++++--------- .../crt/cfgas_stringformatter_unittest.cpp | 12 +++---- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/xfa/fgas/crt/cfgas_decimal.cpp b/xfa/fgas/crt/cfgas_decimal.cpp index 309aad762e..a1a6c4bf89 100644 --- a/xfa/fgas/crt/cfgas_decimal.cpp +++ b/xfa/fgas/crt/cfgas_decimal.cpp @@ -400,10 +400,10 @@ float CFGAS_Decimal::ToFloat() const { } double CFGAS_Decimal::ToDouble() const { - double pow = (double)(1 << 16) * (1 << 16); - double base = static_cast(hi_) * pow * pow + - static_cast(mid_) * pow + static_cast(lo_); - return (neg_ ? -1 : 1) * base * powf(10.0f, -u_scale_); + constexpr double kPow32 = static_cast(uint64_t{1} << 32); + double base = static_cast(hi_) * kPow32 * kPow32 + + static_cast(mid_) * kPow32 + static_cast(lo_); + return (neg_ ? -1 : 1) * base * pow(10.0, -u_scale_); } void CFGAS_Decimal::SetScale(uint8_t newscale) { diff --git a/xfa/fgas/crt/cfgas_decimal_unittest.cpp b/xfa/fgas/crt/cfgas_decimal_unittest.cpp index 197bc7ed3f..cbe4723531 100644 --- a/xfa/fgas/crt/cfgas_decimal_unittest.cpp +++ b/xfa/fgas/crt/cfgas_decimal_unittest.cpp @@ -158,16 +158,11 @@ TEST(CFGASDecimalTest, FloatDoubleConversion) { CFGAS_Decimal pos(L"123.5"); EXPECT_FLOAT_EQ(123.5f, pos.ToFloat()); - // TODO(tsepez): powf in ToDouble() truncates precision, causing 123.5 to - // return 123.50000184029341 rather than 123.5. - EXPECT_NE(123.5, pos.ToDouble()); - EXPECT_NEAR(123.5, pos.ToDouble(), 1e-4); + EXPECT_DOUBLE_EQ(123.5, pos.ToDouble()); CFGAS_Decimal neg(L"-123.5"); EXPECT_FLOAT_EQ(-123.5f, neg.ToFloat()); - // TODO(tsepez): Same precision truncation for negative numbers. - EXPECT_NE(-123.5, neg.ToDouble()); - EXPECT_NEAR(-123.5, neg.ToDouble(), 1e-4); + EXPECT_DOUBLE_EQ(-123.5, neg.ToDouble()); } TEST(CFGASDecimalTest, Multiply) { diff --git a/xfa/fgas/crt/cfgas_stringformatter.cpp b/xfa/fgas/crt/cfgas_stringformatter.cpp index 3ebe43413c..a28507baeb 100644 --- a/xfa/fgas/crt/cfgas_stringformatter.cpp +++ b/xfa/fgas/crt/cfgas_stringformatter.cpp @@ -359,9 +359,6 @@ bool ParseLocaleDate(const WideString& wsDate, *cc += symbol.GetLength(); } } - if (*cc < spDate.size()) { - return false; - } datetime->SetDate(year, month, day); return !!(*cc); @@ -1860,26 +1857,32 @@ bool CFGAS_StringFormatter::ParseDateTime(LocaleMgrIface* pLocaleMgr, } size_t iStart = 0; + bool success = false; switch (eCategory) { case DateTimeType::kDate: - return ParseLocaleDate(wsSrcDateTime, wsDatePattern, pLocale, dtValue, - &iStart); + success = ParseLocaleDate(wsSrcDateTime, wsDatePattern, pLocale, dtValue, + &iStart); + break; case DateTimeType::kTime: - return ParseLocaleTime(wsSrcDateTime, wsTimePattern, pLocale, dtValue, - &iStart); + success = ParseLocaleTime(wsSrcDateTime, wsTimePattern, pLocale, dtValue, + &iStart); + break; case DateTimeType::kDateTime: - return ParseLocaleDate(wsSrcDateTime, wsTimePattern, pLocale, dtValue, - &iStart) && - ParseLocaleTime(wsSrcDateTime, wsDatePattern, pLocale, dtValue, - &iStart); + success = ParseLocaleDate(wsSrcDateTime, wsDatePattern, pLocale, dtValue, + &iStart) && + ParseLocaleTime(wsSrcDateTime, wsTimePattern, pLocale, dtValue, + &iStart); + break; case DateTimeType::kTimeDate: - return ParseLocaleTime(wsSrcDateTime, wsTimePattern, pLocale, dtValue, - &iStart) && - ParseLocaleDate(wsSrcDateTime, wsDatePattern, pLocale, dtValue, - &iStart); + success = ParseLocaleTime(wsSrcDateTime, wsTimePattern, pLocale, dtValue, + &iStart) && + ParseLocaleDate(wsSrcDateTime, wsDatePattern, pLocale, dtValue, + &iStart); + break; case DateTimeType::kUnknown: return false; } + return success && iStart == wsSrcDateTime.GetLength(); } bool CFGAS_StringFormatter::ParseZero(const WideString& wsSrcText) const { diff --git a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp index cfebcc7e9d..8cb3167868 100644 --- a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp +++ b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp @@ -1086,13 +1086,11 @@ TEST_F(CFGASStringFormatterTest, DateTimeParse) { EXPECT_EQ(CFX_DateTime(2020, 5, 15, 14, 30, 0, 0), dt); // DateTime parses date first, then time. - // TODO(tsepez): ParseDateTime() inverts wsDatePattern and wsTimePattern for - // DateTimeType::kDateTime, passing wsTimePattern to ParseLocaleDate() and - // wsDatePattern to ParseLocaleTime(), which causes parsing to fail. - EXPECT_FALSE( - CFGAS_StringFormatter(L"date{YYYY-MM-DD} time{HH:MM:SS}") - .ParseDateTime(Mgr(L"en"), L"2020-05-15 14:30:00", - CFGAS_StringFormatter::DateTimeType::kDateTime, &dt)); + EXPECT_TRUE(CFGAS_StringFormatter(L"date{YYYY-MM-DD} time{HH:MM:SS}") + .ParseDateTime(Mgr(L"en"), L"2020-05-15 14:30:00", + CFGAS_StringFormatter::DateTimeType::kDateTime, + &dt)); + EXPECT_EQ(CFX_DateTime(2020, 5, 15, 14, 30, 0, 0), dt); } TEST_F(CFGASStringFormatterTest, FormatDateTimeEdgeCases) { From 87989d71daa80568e9ba0a8f4225ccf4d789064c Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 5 Nov 2026 17:50:50 -0800 Subject: [PATCH 27/34] Replace CFGAS_Decimal::operator/ with DivBy10 and DivBy100 In production code, division on CFGAS_Decimal is only ever performed to divide by 10 and 100 in CFGAS_StringFormatter. Replace the arbitrary division operator with dedicated DivBy10() and DivBy100() methods. Remove the 1,024-bit binary search multi-limb division helpers and associated unused array routines from cfgas_decimal.cpp. Improve CFGAS_Decimal::ToDouble() precision by using static_cast for the 2^32 factor and double-precision pow(), resolving precision truncation on fractional values. Update tests accordingly. AI-generated patch. Change-Id: I0f805fc4efafeb962ddeae7a16db9a919e1eae9e Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163431 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- xfa/fgas/crt/cfgas_decimal.cpp | 169 +++--------------- xfa/fgas/crt/cfgas_decimal.h | 3 +- xfa/fgas/crt/cfgas_decimal_unittest.cpp | 72 ++++---- xfa/fgas/crt/cfgas_stringformatter.cpp | 7 +- .../crt/cfgas_stringformatter_unittest.cpp | 12 ++ 5 files changed, 89 insertions(+), 174 deletions(-) diff --git a/xfa/fgas/crt/cfgas_decimal.cpp b/xfa/fgas/crt/cfgas_decimal.cpp index a1a6c4bf89..c6b32e71f8 100644 --- a/xfa/fgas/crt/cfgas_decimal.cpp +++ b/xfa/fgas/crt/cfgas_decimal.cpp @@ -64,15 +64,6 @@ inline void decimal_helper_mul10(uint64_t& phi, uint64_t& pmid, uint64_t& plo) { pmid = static_cast(pmid); } -inline void decimal_helper_mul10_any(uint64_t nums[], uint8_t numcount) { - nums[0] *= 0xA; - UNSAFE_TODO({ - for (int i = 1; i < numcount; i++) { - nums[i] = nums[i] * 0xA + Rshift32Bit(nums[i - 1]); - nums[i - 1] = static_cast(nums[i - 1]); - } - }); -} inline void decimal_helper_normalize(uint64_t& phi, uint64_t& pmid, @@ -98,32 +89,6 @@ inline void decimal_helper_normalize_any(uint64_t nums[], uint8_t len) { }); } -inline int8_t decimal_helper_raw_compare_any(uint64_t a[], - uint8_t al, - uint64_t b[], - uint8_t bl) { - int8_t retVal = 0; - UNSAFE_TODO({ - for (int i = std::max(al - 1, bl - 1); i >= 0; i--) { - uint64_t l = (i >= al ? 0 : a[i]), r = (i >= bl ? 0 : b[i]); - retVal += (l > r ? 1 : (l < r ? -1 : 0)); - if (retVal) { - return retVal; - } - } - }); - return retVal; -} - -inline void decimal_helper_dec_any(uint64_t a[], uint8_t al) { - UNSAFE_TODO({ - for (int i = 0; i < al; i++) { - if (a[i]--) { - return; - } - } - }); -} inline void decimal_helper_inc_any(uint64_t a[], uint8_t al) { UNSAFE_TODO({ @@ -166,67 +131,6 @@ inline void decimal_helper_raw_mul(uint64_t a[], }); } -inline void decimal_helper_raw_div(uint64_t a[], - uint8_t al, - uint64_t b[], - uint8_t bl, - uint64_t c[], - uint8_t cl) { - UNSAFE_TODO({ - for (int i = 0; i < cl; i++) { - c[i] = 0; - } - - uint64_t left[16] = {0}; - uint64_t right[16] = {0}; - left[0] = 0; - for (int i = 0; i < al; i++) { - right[i] = a[i]; - } - - uint64_t tmp[16]; - while (decimal_helper_raw_compare_any(left, al, right, al) <= 0) { - uint64_t cur[16]; - for (int i = 0; i < al; i++) { - cur[i] = left[i] + right[i]; - } - - for (int i = al - 1; i >= 0; i--) { - if (i) { - cur[i - 1] += Lshift32Bit(cur[i] % 2); - } - cur[i] /= 2; - } - - decimal_helper_raw_mul(cur, al, b, bl, tmp, 16); - switch (decimal_helper_raw_compare_any(tmp, 16, a, al)) { - case -1: - for (int i = 0; i < 16; i++) { - left[i] = cur[i]; - } - - left[0]++; - decimal_helper_normalize_any(left, al); - break; - case 1: - for (int i = 0; i < 16; i++) { - right[i] = cur[i]; - } - decimal_helper_dec_any(right, al); - break; - case 0: - for (int i = 0; i < std::min(al, cl); i++) { - c[i] = cur[i]; - } - return; - } - } - for (int i = 0; i < std::min(al, cl); i++) { - c[i] = left[i]; - } - }); -} - inline bool decimal_helper_outofrange(uint64_t a[], uint8_t al, uint8_t goal) { UNSAFE_TODO({ for (int i = goal; i < al; i++) { @@ -254,24 +158,6 @@ inline void decimal_helper_shrinkintorange(uint64_t a[], } } -inline void decimal_helper_truncate(uint64_t& phi, - uint64_t& pmid, - uint64_t& plo, - uint8_t& scale, - uint8_t minscale = 0) { - while (scale > minscale) { - uint64_t thi = phi, tmid = pmid, tlo = plo; - if (decimal_helper_div10(thi, tmid, tlo) != 0) { - break; - } - - phi = thi; - pmid = tmid; - plo = tlo; - scale--; - } -} - } // namespace CFGAS_Decimal::CFGAS_Decimal() = default; @@ -465,38 +351,41 @@ CFGAS_Decimal CFGAS_Decimal::operator*(const CFGAS_Decimal& val) const { static_cast(c[2]), neg, scale); } -CFGAS_Decimal CFGAS_Decimal::operator/(const CFGAS_Decimal& val) const { - if (!val.IsNotZero()) { - return CFGAS_Decimal(); +void CFGAS_Decimal::DivBy10() { + if (!IsNotZero()) { + return; } - bool neg = neg_ ^ val.neg_; - uint64_t a[7] = {lo_, mid_, hi_}, b[3] = {val.lo_, val.mid_, val.hi_}, - c[7] = {0}; - uint8_t scale = 0; - if (u_scale_ < val.u_scale_) { - for (int i = val.u_scale_ - u_scale_; i > 0; i--) { - decimal_helper_mul10_any(a, 7); - } - } else { - scale = u_scale_ - val.u_scale_; + uint64_t thi = hi_; + uint64_t tmid = mid_; + uint64_t tlo = lo_; + uint8_t rem = decimal_helper_div10(thi, tmid, tlo); + if (rem == 0) { + hi_ = static_cast(thi); + mid_ = static_cast(tmid); + lo_ = static_cast(tlo); + return; } - uint8_t minscale = scale; - if (!IsNotZero()) { - return CFGAS_Decimal(0, 0, 0, false, minscale); + if (u_scale_ < kDecimalScaleLimit) { + u_scale_++; + return; } - while (!a[6]) { - decimal_helper_mul10_any(a, 7); - scale++; + hi_ = static_cast(thi); + mid_ = static_cast(tmid); + lo_ = static_cast(tlo); + if (rem >= 5) { + uint64_t plo = lo_ + 1; + uint64_t pmid = mid_ + Rshift32Bit(plo); + lo_ = static_cast(plo); + uint64_t phi = hi_ + Rshift32Bit(pmid); + mid_ = static_cast(pmid); + hi_ = static_cast(phi); } +} - decimal_helper_div10_any(a, 7); - scale--; - decimal_helper_raw_div(a, 6, b, 3, c, 7); - decimal_helper_shrinkintorange(c, 6, 3, scale); - decimal_helper_truncate(c[2], c[1], c[0], scale, minscale); - return CFGAS_Decimal(static_cast(c[0]), static_cast(c[1]), - static_cast(c[2]), neg, scale); +void CFGAS_Decimal::DivBy100() { + DivBy10(); + DivBy10(); } diff --git a/xfa/fgas/crt/cfgas_decimal.h b/xfa/fgas/crt/cfgas_decimal.h index 31f0796c0d..5c6bab61c0 100644 --- a/xfa/fgas/crt/cfgas_decimal.h +++ b/xfa/fgas/crt/cfgas_decimal.h @@ -23,7 +23,8 @@ class CFGAS_Decimal { double ToDouble() const; CFGAS_Decimal operator*(const CFGAS_Decimal& val) const; - CFGAS_Decimal operator/(const CFGAS_Decimal& val) const; + void DivBy10(); + void DivBy100(); bool IsNotZero() const { return hi_ || mid_ || lo_; } uint8_t GetScale() const { return u_scale_; } diff --git a/xfa/fgas/crt/cfgas_decimal_unittest.cpp b/xfa/fgas/crt/cfgas_decimal_unittest.cpp index cbe4723531..cd47166767 100644 --- a/xfa/fgas/crt/cfgas_decimal_unittest.cpp +++ b/xfa/fgas/crt/cfgas_decimal_unittest.cpp @@ -106,6 +106,9 @@ TEST(CFGASDecimalTest, FromString) { CFGAS_Decimal empty_str(L""); EXPECT_EQ(L"0", empty_str.ToWideString()); + + CFGAS_Decimal blank_str(L" "); + EXPECT_EQ(L"0", blank_str.ToWideString()); } TEST(CFGASDecimalTest, FromString28Digits) { @@ -191,37 +194,44 @@ TEST(CFGASDecimalTest, Multiply) { EXPECT_LE(product.GetScale(), 28); } -TEST(CFGASDecimalTest, Divide) { +TEST(CFGASDecimalTest, DivBy10AndDivBy100) { CFGAS_Decimal zero; - CFGAS_Decimal num(100); - - // Division by zero. - EXPECT_EQ(L"0", (num / zero).ToWideString()); - - // Zero divided by non-zero. - EXPECT_EQ(L"0", (zero / num).ToWideString()); - - // Integer division. - EXPECT_EQ(L"25", (num / CFGAS_Decimal(4)).ToWideString()); - - // Smaller scale divided by larger scale. - CFGAS_Decimal d1(10); - CFGAS_Decimal d2(L"2.5"); - EXPECT_EQ(L"4", (d1 / d2).ToWideString()); - - // Larger scale divided by smaller scale maintains scale difference. - CFGAS_Decimal d3(L"10.0"); - CFGAS_Decimal d4(2); - EXPECT_EQ(L"5.0", (d3 / d4).ToWideString()); - - // Repeating fraction. - CFGAS_Decimal one(1); - CFGAS_Decimal three(3); - CFGAS_Decimal div_result = one / three; - EXPECT_EQ(L"0.3333333333333333333333333333", div_result.ToWideString()); + zero.DivBy10(); + EXPECT_EQ(L"0", zero.ToWideString()); + zero.DivBy100(); + EXPECT_EQ(L"0", zero.ToWideString()); - // Negative operands. - EXPECT_EQ(L"-5", (CFGAS_Decimal(-10) / CFGAS_Decimal(2)).ToWideString()); - EXPECT_EQ(L"-5", (CFGAS_Decimal(10) / CFGAS_Decimal(-2)).ToWideString()); - EXPECT_EQ(L"5", (CFGAS_Decimal(-10) / CFGAS_Decimal(-2)).ToWideString()); + CFGAS_Decimal a(100); + a.DivBy10(); + EXPECT_EQ(L"10", a.ToWideString()); + a.DivBy10(); + EXPECT_EQ(L"1", a.ToWideString()); + a.DivBy10(); + EXPECT_EQ(L"0.1", a.ToWideString()); + + CFGAS_Decimal b(L"10.0"); + b.DivBy10(); + EXPECT_EQ(L"1.0", b.ToWideString()); + + CFGAS_Decimal c(123); + c.DivBy10(); + EXPECT_EQ(L"12.3", c.ToWideString()); + + CFGAS_Decimal neg(-100); + neg.DivBy10(); + EXPECT_EQ(L"-10", neg.ToWideString()); + neg.DivBy10(); + EXPECT_EQ(L"-1", neg.ToWideString()); + + CFGAS_Decimal pct(50); + pct.DivBy100(); + EXPECT_EQ(L"0.5", pct.ToWideString()); + + CFGAS_Decimal pct100(100); + pct100.DivBy100(); + EXPECT_EQ(L"1", pct100.ToWideString()); + + CFGAS_Decimal pct_neg(-75); + pct_neg.DivBy100(); + EXPECT_EQ(L"-0.75", pct_neg.ToWideString()); } diff --git a/xfa/fgas/crt/cfgas_stringformatter.cpp b/xfa/fgas/crt/cfgas_stringformatter.cpp index a28507baeb..a08b5c124a 100644 --- a/xfa/fgas/crt/cfgas_stringformatter.cpp +++ b/xfa/fgas/crt/cfgas_stringformatter.cpp @@ -1684,7 +1684,7 @@ bool CFGAS_StringFormatter::ParseNum(LocaleMgrIface* pLocaleMgr, decimal = decimal * CFGAS_Decimal(powf(10, iExponent), 3); } if (bHavePercentSymbol) { - decimal = decimal / CFGAS_Decimal(100); + decimal.DivBy100(); } *wsValue = decimal.ToWideString(); } @@ -2073,10 +2073,13 @@ bool CFGAS_StringFormatter::FormatNum(LocaleMgrIface* pLocaleMgr, while (decimal.IsNotZero() && fabs(decimal.ToDouble()) > threshold.ValueOrDie()) { - decimal = decimal / CFGAS_Decimal(10); + decimal.DivBy10(); ++exponent; } } + if (fixed_count > 0 && (bAdjusted || exponent != 0)) { + wsSrcNum = decimal.ToWideString(); + } } bool bTrimTailZeros = false; diff --git a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp index 8cb3167868..91d94914cf 100644 --- a/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp +++ b/xfa/fgas/crt/cfgas_stringformatter_unittest.cpp @@ -1019,12 +1019,24 @@ TEST_F(CFGASStringFormatterTest, NumFormatAndParseEdgeCases) { CFGAS_StringFormatter(L"").FormatNum(Mgr(L"en"), L"123", &result)); EXPECT_FALSE( CFGAS_StringFormatter(L"num{999}").FormatNum(Mgr(L"en"), L"", &result)); + EXPECT_FALSE(CFGAS_StringFormatter(L"num{999}") + .FormatNum(Mgr(L"en"), L" ", &result)); + EXPECT_FALSE( + CFGAS_StringFormatter(L"99.999E").FormatNum(Mgr(L"en"), L" ", &result)); // ParseNum empty cases. EXPECT_FALSE( CFGAS_StringFormatter(L"").ParseNum(Mgr(L"en"), L"123", &result)); EXPECT_FALSE( CFGAS_StringFormatter(L"num{999}").ParseNum(Mgr(L"en"), L"", &result)); + EXPECT_FALSE( + CFGAS_StringFormatter(L"num{999}").ParseNum(Mgr(L"en"), L" ", &result)); + EXPECT_FALSE( + CFGAS_StringFormatter(L"99.999E").ParseNum(Mgr(L"en"), L" ", &result)); + + // Exponent threshold overflow error cases. + EXPECT_FALSE(CFGAS_StringFormatter(L"99999999999E") + .FormatNum(Mgr(L"en"), L"1", &result)); // Integer negative formatting without +/- in pattern. result.clear(); From 30b2a20a51ce5049ab460dba42063b6b84573595 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 5 Nov 2026 18:40:08 -0800 Subject: [PATCH 28/34] Fix Lab ICC profile handling in IccTransform CreateTransformSRGB() previously configured srcFormat for Lab profiles with BYTES_SH(0) and no FLOAT_SH(1), which caused cmsCreateTransform() to reject the format. Configure Lab profiles using TYPE_Lab_FLT (FLOAT_SH(1) and BYTES_SH(4)). Update Translate() to feed float inputs to cmsDoTransform(). Update TranslateScanline() to handle Lab profiles by decoding 8-bit PDF Lab samples to float inputs via a new FX_DecodeLab() helper in core/fxge/dib/fx_dib.h. LittleCMS has no native input format for PDF 8-bit Lab encoding (TYPE_LabV2_8 expects legacy ICC v2 integer encoding, which distorts neutral gray and scales L*=100 to 100.392). Deduplicate this decode logic with CPDF_LabCS::TranslateImageLine(). Enforce input buffer sizes and component bounds using CHECK_GE() in both Translate() and TranslateScanline(), and verify transform format flags before delegating to cmsDoTransform(). Update IccTransformTest.LabProfile to verify successful transform creation as well as both float and scanline translation accuracy. Fixed: 567626895 Change-Id: Ie4aee8b595757ca83aa83e02f43a03c3fcd15b63 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163091 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- core/fpdfapi/page/cpdf_colorspace.cpp | 10 ++- core/fxcodec/icc/icc_transform.cpp | 69 ++++++++++++++------- core/fxcodec/icc/icc_transform.h | 12 ++-- core/fxcodec/icc/icc_transform_unittest.cpp | 45 ++++++++++++-- core/fxge/dib/fx_dib.h | 11 ++++ testing/fuzzers/pdf_codec_icc_fuzzer.cc | 4 +- 6 files changed, 110 insertions(+), 41 deletions(-) diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp index f1c83fc9c7..04791bac69 100644 --- a/core/fpdfapi/page/cpdf_colorspace.cpp +++ b/core/fpdfapi/page/cpdf_colorspace.cpp @@ -903,15 +903,13 @@ void CPDF_LabCS::TranslateImageLine(pdfium::span dest_span, fxcrt::reinterpret_span>(src_span).first( static_cast(pixels)); for (auto [lab_ref, bgr_ref] : fxcrt::Zip(lab_span, bgr_span)) { - const float lab[3] = { - static_cast(lab_ref.lightness_star * 100) / 255.0f, - static_cast(lab_ref.a_star - 128), - static_cast(lab_ref.b_star - 128), - }; + const FX_LAB_STRUCT decoded = DecodeLab(lab_ref); + auto float_span = + fxcrt::reinterpret_span(pdfium::span_from_ref(decoded)); // Better code than the equivalent GetRGBOrZerosOnError() since that // is implemented in a base class and can't devirtualize the GetRGB() // call despite this class being marked final. - auto rgb = GetRGB(lab).value_or(FX_RGB_STRUCT{}); + auto rgb = GetRGB(float_span).value_or(FX_RGB_STRUCT{}); bgr_ref.blue = static_cast(rgb.blue * 255); bgr_ref.green = static_cast(rgb.green * 255); bgr_ref.red = static_cast(rgb.red * 255); diff --git a/core/fxcodec/icc/icc_transform.cpp b/core/fxcodec/icc/icc_transform.cpp index 8bdba0e95c..8c9d7ce490 100644 --- a/core/fxcodec/icc/icc_transform.cpp +++ b/core/fxcodec/icc/icc_transform.cpp @@ -9,12 +9,17 @@ #include #include +#include #include -#include "core/fxcrt/data_vector.h" +#include "core/fxcrt/check_op.h" +#include "core/fxcrt/fixed_size_data_vector.h" #include "core/fxcrt/notreached.h" #include "core/fxcrt/numerics/safe_conversions.h" #include "core/fxcrt/ptr_util.h" +#include "core/fxcrt/span_util.h" +#include "core/fxcrt/zip.h" +#include "core/fxge/dib/fx_dib.h" namespace fxcodec { @@ -40,7 +45,7 @@ bool Check3Components(cmsColorSpaceSignature cs) { } // namespace IccTransform::IccTransform(cmsHTRANSFORM hTransform, - int srcComponents, + size_t srcComponents, bool bIsLab, bool bNormal) : transform_(hTransform), @@ -77,8 +82,8 @@ std::unique_ptr IccTransform::CreateTransformSRGB( bool bLab = false; bool bNormal = false; if (srcCS == cmsSigLabData) { - srcFormat = - COLORSPACE_SH(PT_Lab) | CHANNELS_SH(nSrcComponents) | BYTES_SH(0); + srcFormat = FLOAT_SH(1) | COLORSPACE_SH(PT_Lab) | + CHANNELS_SH(nSrcComponents) | BYTES_SH(4); bLab = true; } else { srcFormat = @@ -118,22 +123,16 @@ std::unique_ptr IccTransform::CreateTransformSRGB( void IccTransform::Translate(pdfium::span src_values, pdfium::span dest_values) { - uint8_t output[4]; - // TODO(crbug.com/567626895): Currently the CmsDoTransform method is part of - // LCMS and it will apply some member of transform_ to the input. We need to - // go over all the places which set transform to verify that only - // `src_values.size()` components are used. + CHECK_GE(src_values.size(), src_components_); + constexpr size_t kMaxValidComponents = 4u; // See IsValidIccComponents(). + uint8_t output[kMaxValidComponents]; if (lab_) { - DataVector inputs(std::max(src_values.size(), 16)); - for (uint32_t i = 0; i < src_values.size(); ++i) { - inputs[i] = src_values[i]; - } - cmsDoTransform(transform_, inputs.data(), output, 1); + cmsDoTransform(transform_, src_values.data(), output, 1); } else { - DataVector inputs(std::max(src_values.size(), 16)); - for (size_t i = 0; i < src_values.size(); ++i) { - inputs[i] = - static_cast(std::clamp(src_values[i] * 255.0f, 0.0f, 255.0f)); + std::array inputs; + for (auto [src, dst] : + fxcrt::Zip(src_values.first(src_components_), inputs)) { + dst = static_cast(std::clamp(src * 255.0f, 0.0f, 255.0f)); } cmsDoTransform(transform_, inputs.data(), output, 1); } @@ -142,10 +141,38 @@ void IccTransform::Translate(pdfium::span src_values, dest_values[2] = output[0] / 255.0f; } -void IccTransform::TranslateScanline(pdfium::span pDest, - pdfium::span pSrc, +void IccTransform::TranslateScanline(pdfium::span dest, + pdfium::span src, int32_t pixels) { - cmsDoTransform(transform_, pSrc.data(), pDest.data(), pixels); + CHECK_GT(pixels, 0); + const size_t pixel_count = static_cast(pixels); + CHECK_GE(src.size(), pixel_count * src_components_); + CHECK_GE(dest.size(), pixel_count * 3); + const cmsUInt32Number format = cmsGetTransformInputFormat(transform_); + if (lab_) { + CHECK_EQ(T_FLOAT(format), 1u); + CHECK_EQ(T_BYTES(format), 4u); + CHECK_EQ(T_CHANNELS(format), 3u); + + // LittleCMS lacks an input format for 8-bit PDF Lab encoding. TYPE_LabV2_8 + // expects legacy ICC v2 integer encoding, which distorts neutral gray + // (a*=0, b*=0) and scales L*=100 to 100.392. Promote 8-bit PDF Lab samples + // to float inputs (TYPE_Lab_FLT) to preserve accurate colors. + auto lab_span = + fxcrt::reinterpret_span>(src).first( + pixel_count); + auto inputs = + FixedSizeDataVector>::Uninit(pixel_count); + for (auto [src_lab, dst_lab] : fxcrt::Zip(lab_span, inputs.span())) { + dst_lab = DecodeLab(src_lab); + } + cmsDoTransform(transform_, inputs.span().data(), dest.data(), pixels); + } else { + CHECK_EQ(T_FLOAT(format), 0u); + CHECK_EQ(T_BYTES(format), 1u); + CHECK_EQ(T_CHANNELS(format), static_cast(src_components_)); + cmsDoTransform(transform_, src.data(), dest.data(), pixels); + } } // static diff --git a/core/fxcodec/icc/icc_transform.h b/core/fxcodec/icc/icc_transform.h index c3f4c7b89b..003ec69f8c 100644 --- a/core/fxcodec/icc/icc_transform.h +++ b/core/fxcodec/icc/icc_transform.h @@ -31,23 +31,23 @@ class IccTransform { void Translate(pdfium::span src_values, pdfium::span dest_values); - void TranslateScanline(pdfium::span pDest, - pdfium::span pSrc, - int pixels); + void TranslateScanline(pdfium::span dest, + pdfium::span src, + int32_t pixels); - int components() const { return src_components_; } + size_t components() const { return src_components_; } bool IsNormal() const { return normal_; } static bool IsValidIccComponents(int components); private: IccTransform(cmsHTRANSFORM transform, - int srcComponents, + size_t srcComponents, bool bIsLab, bool bNormal); const cmsHTRANSFORM transform_; - const int src_components_; + const size_t src_components_; const bool lab_; const bool normal_; }; diff --git a/core/fxcodec/icc/icc_transform_unittest.cpp b/core/fxcodec/icc/icc_transform_unittest.cpp index 2dbb52d0d6..30a23bcaa1 100644 --- a/core/fxcodec/icc/icc_transform_unittest.cpp +++ b/core/fxcodec/icc/icc_transform_unittest.cpp @@ -11,6 +11,8 @@ #include "core/fxcrt/data_vector.h" #include "core/fxcrt/span.h" +#include "core/fxcrt/span_util.h" +#include "core/fxge/dib/fx_dib.h" #include "testing/gtest/include/gtest/gtest.h" namespace fxcodec { @@ -81,17 +83,50 @@ TEST(IccTransformTest, SRGBProfile) { } TEST(IccTransformTest, LabProfile) { - // cmsCreateTransform currently fails for Lab profiles because - // IccTransform::CreateTransformSRGB constructs `srcFormat` with - // `BYTES_SH(0)` but omits `FLOAT_SH(1)`, so lcms2 does not recognize it - // as `TYPE_Lab_DBL`. cmsHPROFILE profile = cmsCreateLab4Profile(nullptr); ASSERT_TRUE(profile); DataVector profile_data = SerializeProfile(profile); cmsCloseProfile(profile); ASSERT_FALSE(profile_data.empty()); - EXPECT_FALSE(IccTransform::CreateTransformSRGB(profile_data)); + std::unique_ptr transform = + IccTransform::CreateTransformSRGB(profile_data); + ASSERT_TRUE(transform); + + // L*=100, a*=0, b*=0 is diffuse white. + static constexpr FX_LAB_STRUCT kLabWhite = {100.0f, 0.0f, 0.0f}; + FX_RGB_STRUCT dst_rgb = {}; + transform->Translate( + fxcrt::reinterpret_span(pdfium::span_from_ref(kLabWhite)), + fxcrt::reinterpret_span(pdfium::span_from_ref(dst_rgb)) + .first<3>()); + EXPECT_NEAR(dst_rgb.red, 1.0f, 0.05f); + EXPECT_NEAR(dst_rgb.green, 1.0f, 0.05f); + EXPECT_NEAR(dst_rgb.blue, 1.0f, 0.05f); + + // Translate scanline with 8-bit Lab samples. + // 1 pixel = 3 bytes (L=255, a=128, b=128 is diffuse white). + // Note that destination is BGR_8 format per cmsCreateTransform. + static constexpr FX_LAB_STRUCT kLabWhitePixel = {255, 128, 128}; + FX_BGR_STRUCT dst_pixel = {}; + transform->TranslateScanline(pdfium::byte_span_from_ref(dst_pixel), + pdfium::byte_span_from_ref(kLabWhitePixel), 1); + EXPECT_EQ(dst_pixel.blue, 255); + EXPECT_EQ(dst_pixel.green, 255); + EXPECT_EQ(dst_pixel.red, 255); + + // Translate scanline with multiple 8-bit Lab samples (2 pixels). + static constexpr std::array, 2> kLabTwoPixels = { + {{255, 128, 128}, {255, 128, 128}}}; + std::array, 2> dst_two_pixels = {}; + transform->TranslateScanline(pdfium::byte_span_from_ref(dst_two_pixels), + pdfium::byte_span_from_ref(kLabTwoPixels), 2); + EXPECT_EQ(dst_two_pixels[0].blue, 255); + EXPECT_EQ(dst_two_pixels[0].green, 255); + EXPECT_EQ(dst_two_pixels[0].red, 255); + EXPECT_EQ(dst_two_pixels[1].blue, 255); + EXPECT_EQ(dst_two_pixels[1].green, 255); + EXPECT_EQ(dst_two_pixels[1].red, 255); } TEST(IccTransformTest, GrayProfile) { diff --git a/core/fxge/dib/fx_dib.h b/core/fxge/dib/fx_dib.h index 9914f1031a..3bb49e7bd0 100644 --- a/core/fxge/dib/fx_dib.h +++ b/core/fxge/dib/fx_dib.h @@ -13,6 +13,7 @@ #include #include "core/fxcrt/compiler_specific.h" +#include "core/fxcrt/fx_memory_wrappers.h" #include "core/fxcrt/span.h" #if defined(PDF_USE_SKIA) @@ -113,6 +114,15 @@ struct FX_LAB_STRUCT { T b_star = 0; }; +// Decodes an 8-bit PDF/ICC Lab sample into CIE L*a*b* floats. +inline FX_LAB_STRUCT DecodeLab(FX_LAB_STRUCT lab) { + return { + static_cast(lab.lightness_star * 100) / 255.0f, + static_cast(lab.a_star - 128), + static_cast(lab.b_star - 128), + }; +} + struct FXDIB_ResampleOptions { FXDIB_ResampleOptions(); @@ -292,6 +302,7 @@ using fxge::ArgbToBGRStruct; using fxge::ArgbToColorRef; using fxge::BlendMode; using fxge::CmykEncode; +using fxge::DecodeLab; using fxge::FX_ABGR_STRUCT; using fxge::FX_ARGB; using fxge::FX_BGR_STRUCT; diff --git a/testing/fuzzers/pdf_codec_icc_fuzzer.cc b/testing/fuzzers/pdf_codec_icc_fuzzer.cc index cdfc413c99..755f0b359a 100644 --- a/testing/fuzzers/pdf_codec_icc_fuzzer.cc +++ b/testing/fuzzers/pdf_codec_icc_fuzzer.cc @@ -19,8 +19,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { const float src[4] = {0.5f, 0.5f, 0.5f, 0.5f}; float dst[3]; - transform->Translate( - pdfium::span(src).first(static_cast(transform->components())), - dst); + transform->Translate(pdfium::span(src).first(transform->components()), dst); return 0; } From 3351236ec2a8fba25fa2a17290b9bdaa2ec06eb5 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 6 Nov 2026 10:21:42 -0800 Subject: [PATCH 29/34] Fix NameRecord parsing in CFX_CTTNameTable Read nPlatformID and nNameID directly from the 12-byte NameRecord span rather than incorrectly indexing the base table by header offset twice. This corrects a bug where nNameID was reading the platform ID, causing non-Macintosh (e.g. Windows UTF-16) font name records to be skipped. Also read both fields adjacently before validating nNameID == 1, tighten length and offset bounds checks, and add unit test coverage for Windows UTF-16 name table records. AI-generated patch from coverage data. Change-Id: I448f4a761a454191b2dfbbf9439acc1051d1e6b5 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163111 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- core/fxge/cfx_cttnametable.cpp | 55 ++++++----- core/fxge/cfx_cttnametable_unittest.cpp | 116 +++++++++++++++++++----- 2 files changed, 125 insertions(+), 46 deletions(-) diff --git a/core/fxge/cfx_cttnametable.cpp b/core/fxge/cfx_cttnametable.cpp index fa13f5df21..af2cd5e7dd 100644 --- a/core/fxge/cfx_cttnametable.cpp +++ b/core/fxge/cfx_cttnametable.cpp @@ -4,6 +4,8 @@ #include "core/fxge/cfx_cttnametable.h" +#include + #include "core/fxcrt/byteorder.h" namespace { @@ -16,44 +18,53 @@ uint16_t ReadUInt16FromSpanAtOffset(pdfium::span data, } // namespace CFX_CTTNameTable::CFX_CTTNameTable(pdfium::span name_table) { - if (name_table.empty()) { + if (name_table.size() < 6) { return; } uint16_t nNameCount = ReadUInt16FromSpanAtOffset(name_table, 2); - pdfium::span str = - name_table.subspan(ReadUInt16FromSpanAtOffset(name_table, 4)); + uint16_t nStringOffset = ReadUInt16FromSpanAtOffset(name_table, 4); + if (nStringOffset > name_table.size()) { + return; + } + + pdfium::span str = name_table.subspan(nStringOffset); pdfium::span name_record = name_table.subspan<6u>(); for (uint16_t i = 0; i < nNameCount; ++i) { - uint16_t nNameID = ReadUInt16FromSpanAtOffset(name_table, i * 12 + 6); - if (nNameID != 1) { - continue; + size_t record_offset = static_cast(i) * 12; + if (name_record.size() - record_offset < 12) { + break; } - uint16_t nPlatformID = ReadUInt16FromSpanAtOffset(name_record, i * 12); - uint16_t nNameLength = ReadUInt16FromSpanAtOffset(name_record, i * 12 + 8); - uint16_t nNameOffset = ReadUInt16FromSpanAtOffset(name_record, i * 12 + 10); - if (nPlatformID != 1) { - WideString wsFamily; - for (uint16_t j = 0; j < nNameLength / 2; ++j) { - wchar_t wcTemp = ReadUInt16FromSpanAtOffset(str, nNameOffset + j * 2); - wsFamily += wcTemp; - } - family_names_.push_back(wsFamily); + pdfium::span record = + name_record.subspan(record_offset).first<12u>(); + uint16_t nPlatformID = ReadUInt16FromSpanAtOffset(record, 0); + uint16_t nNameID = ReadUInt16FromSpanAtOffset(record, 6); + if (nNameID != 1) { continue; } + uint16_t nNameLength = ReadUInt16FromSpanAtOffset(record, 8); + uint16_t nNameOffset = ReadUInt16FromSpanAtOffset(record, 10); // Avoid out of bounds crashes if the length and/or offset are wrong. - if (static_cast(nNameLength) + nNameOffset >= str.size()) { + if (static_cast(nNameLength) + nNameOffset > str.size()) { continue; } - WideString wsFamily; - for (uint16_t j = 0; j < nNameLength; ++j) { - wchar_t wcTemp = str[nNameOffset + j]; - wsFamily += wcTemp; + WideString family; + if (nPlatformID != 1) { + family.Reserve(nNameLength / 2); + for (uint16_t j = 0; j < nNameLength / 2; ++j) { + family += static_cast( + ReadUInt16FromSpanAtOffset(str, nNameOffset + j * 2)); + } + } else { + family.Reserve(nNameLength); + for (uint16_t j = 0; j < nNameLength; ++j) { + family += static_cast(str[nNameOffset + j]); + } } - family_names_.push_back(wsFamily); + family_names_.push_back(std::move(family)); } } diff --git a/core/fxge/cfx_cttnametable_unittest.cpp b/core/fxge/cfx_cttnametable_unittest.cpp index 1958d69bb6..2a3e818cfd 100644 --- a/core/fxge/cfx_cttnametable_unittest.cpp +++ b/core/fxge/cfx_cttnametable_unittest.cpp @@ -22,6 +22,22 @@ TEST(CFX_CTTNameTableTest, Empty) { EXPECT_TRUE(empty_table.GetFamilyNames().empty()); } +TEST(CFX_CTTNameTableTest, TruncatedHeader) { + static constexpr uint8_t kTruncatedHeader[] = {0x00, 0x00, 0x00, 0x01}; + CFX_CTTNameTable table(kTruncatedHeader); + EXPECT_TRUE(table.GetFamilyNames().empty()); +} + +TEST(CFX_CTTNameTableTest, InvalidStringOffset) { + static constexpr uint8_t kInvalidStringOffset[] = { + 0x00, 0x00, // format + 0x00, 0x01, // count = 1 + 0xFF, 0xFF, // string_offset = 65535 (out of bounds) + }; + CFX_CTTNameTable table(kInvalidStringOffset); + EXPECT_TRUE(table.GetFamilyNames().empty()); +} + TEST(CFX_CTTNameTableTest, AddFamilyName) { CFX_CTTNameTable table; table.AddFamilyName(L"First"); @@ -30,8 +46,7 @@ TEST(CFX_CTTNameTableTest, AddFamilyName) { } TEST(CFX_CTTNameTableTest, ParseMacintoshRecord) { - // Construct a minimal TT name table with 1 name record for Macintosh - // (Platform 1). + // Minimal TT name table with 1 name record for Macintosh (Platform 1). // Header: // format: 0 (2 bytes) // count: 1 (2 bytes) @@ -44,37 +59,90 @@ TEST(CFX_CTTNameTableTest, ParseMacintoshRecord) { // length: 4 (2 bytes) // offset: 0 (2 bytes) // String storage (starts at offset 18): - // "Test\0" (5 bytes, extra byte to satisfy >= bound check) + // "Test" (4 bytes, testing exact bounds without trailing padding) static constexpr uint8_t kTableData[] = { - 0x00, 0x00, // format - 0x00, 0x01, // count = 1 - 0x00, 0x12, // string_offset = 18 - 0x00, 0x01, // platform_id = 1 - 0x00, 0x00, // encoding_id = 0 - 0x00, 0x00, // language_id = 0 - 0x00, 0x01, // name_id = 1 - 0x00, 0x04, // length = 4 - 0x00, 0x00, // offset = 0 - 'T', 'e', 's', 't', 0 // string data + 0x00, 0x00, // format + 0x00, 0x01, // count = 1 + 0x00, 0x12, // string_offset = 18 + 0x00, 0x01, // platform_id = 1 + 0x00, 0x00, // encoding_id = 0 + 0x00, 0x00, // language_id = 0 + 0x00, 0x01, // name_id = 1 + 0x00, 0x04, // length = 4 + 0x00, 0x00, // offset = 0 + 'T', 'e', 's', 't' // string data }; CFX_CTTNameTable table(kTableData); EXPECT_THAT(table.GetFamilyNames(), ElementsAre(L"Test")); } +TEST(CFX_CTTNameTableTest, ParseWindowsRecord) { + // Minimal TT name table with 1 name record for Windows (Platform 3). + // Uses big-endian UTF-16 encoding. + // Header: + // format: 0 (2 bytes) + // count: 1 (2 bytes) + // string_offset: 18 (2 bytes) + // NameRecord 0 (12 bytes at offset 6): + // platform_id: 3 (2 bytes) + // encoding_id: 1 (2 bytes) + // language_id: 0x0409 (2 bytes) + // name_id: 1 (2 bytes) + // length: 8 (2 bytes) + // offset: 0 (2 bytes) + // String storage (starts at offset 18): + // UTF-16BE "Test" (8 bytes) + static constexpr uint8_t kTableData[] = { + 0x00, 0x00, // format + 0x00, 0x01, // count = 1 + 0x00, 0x12, // string_offset = 18 + 0x00, 0x03, // platform_id = 3 (Windows) + 0x00, 0x01, // encoding_id = 1 + 0x04, 0x09, // language_id = 0x0409 + 0x00, 0x01, // name_id = 1 (Family name) + 0x00, 0x08, // length = 8 bytes + 0x00, 0x00, // offset = 0 + 0x00, 'T', 0x00, 'e', 0x00, 's', 0x00, 't' // string data + }; + + CFX_CTTNameTable table(kTableData); + ASSERT_EQ(1u, table.GetFamilyNames().size()); + EXPECT_EQ(L"Test", table.GetFamilyNames()[0]); +} + +TEST(CFX_CTTNameTableTest, ParseNonFamilyNameRecord) { + // Name record with name_id != 1 (e.g. name_id = 2 for Subfamily/Style). + static constexpr uint8_t kTableData[] = { + 0x00, 0x00, // format + 0x00, 0x01, // count = 1 + 0x00, 0x12, // string_offset = 18 + 0x00, 0x03, // platform_id = 3 + 0x00, 0x01, // encoding_id = 1 + 0x04, 0x09, // language_id = 0x0409 + 0x00, 0x02, // name_id = 2 (not family name) + 0x00, 0x08, // length = 8 + 0x00, 0x00, // offset = 0 + 0x00, 'B', 0x00, 'o', 0x00, 'l', 0x00, 'd' // "Bold" + }; + + CFX_CTTNameTable table(kTableData); + EXPECT_TRUE(table.GetFamilyNames().empty()); +} + TEST(CFX_CTTNameTableTest, ParseOutOfBoundsRecord) { - // Same table, but offset points beyond string storage. + // Offset points beyond string storage. static constexpr uint8_t kTableData[] = { - 0x00, 0x00, // format - 0x00, 0x01, // count = 1 - 0x00, 0x12, // string_offset = 18 - 0x00, 0x01, // platform_id = 1 - 0x00, 0x00, // encoding_id = 0 - 0x00, 0x00, // language_id = 0 - 0x00, 0x01, // name_id = 1 - 0x00, 0x04, // length = 4 - 0x00, 0x20, // offset = 32 (out of bounds) - 'T', 'e', 's', 't', 0 // string data + 0x00, 0x00, // format + 0x00, 0x01, // count = 1 + 0x00, 0x12, // string_offset = 18 + 0x00, 0x01, // platform_id = 1 + 0x00, 0x00, // encoding_id = 0 + 0x00, 0x00, // language_id = 0 + 0x00, 0x01, // name_id = 1 + 0x00, 0x04, // length = 4 + 0x00, 0x20, // offset = 32 (out of bounds) + 'T', 'e', 's', 't' // string data }; CFX_CTTNameTable table(kTableData); From b2887ebe15b05bf79be528c963b7f8f338d83fbd Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 6 Nov 2026 10:22:37 -0800 Subject: [PATCH 30/34] Add unit tests for TIFF progressive decoding and JPX YUV conversion Add unit tests in core/fxcodec: - Add tests in progressive_decoder_unittest.cpp covering TIFF progressive decoding, verifying invalid data error handling and uncompressed 2x2 grayscale TIFF frame info and bitmap decoding. - Add tests in jpx_unittest.cpp covering YUV422 to RGB and YUV444 to RGB conversions in CJPX_Decoder. - Add tests in jpx_unittest.cpp for ColorSyccToRGB covering grayscale fallback on fewer than 3 components and SRGB conversion. Change-Id: I13ce51bb6211447b6513f1567d9c921371714f57 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163130 Commit-Queue: Tom Sepez Reviewed-by: Andy Phan --- core/fxcodec/jpx/cjpx_decoder.cpp | 15 ++ core/fxcodec/jpx/cjpx_decoder.h | 3 + core/fxcodec/jpx/jpx_unittest.cpp | 183 ++++++++++++++++++ core/fxcodec/progressive_decoder_unittest.cpp | 49 +++++ 4 files changed, 250 insertions(+) diff --git a/core/fxcodec/jpx/cjpx_decoder.cpp b/core/fxcodec/jpx/cjpx_decoder.cpp index 7eb63cde04..9dbe98458a 100644 --- a/core/fxcodec/jpx/cjpx_decoder.cpp +++ b/core/fxcodec/jpx/cjpx_decoder.cpp @@ -447,6 +447,21 @@ void CJPX_Decoder::Sycc420ToRgbForTesting(opj_image_t* img) { sycc420_to_rgb(img); } +// static +void CJPX_Decoder::Sycc422ToRgbForTesting(opj_image_t* img) { + sycc422_to_rgb(img); +} + +// static +void CJPX_Decoder::Sycc444ToRgbForTesting(opj_image_t* img) { + sycc444_to_rgb(img); +} + +// static +void CJPX_Decoder::ColorSyccToRgbForTesting(opj_image_t* img) { + color_sycc_to_rgb(img); +} + CJPX_Decoder::CJPX_Decoder(ColorSpaceOption option) : color_space_option_(option) {} diff --git a/core/fxcodec/jpx/cjpx_decoder.h b/core/fxcodec/jpx/cjpx_decoder.h index 5f8c2c058e..ea2bfd8282 100644 --- a/core/fxcodec/jpx/cjpx_decoder.h +++ b/core/fxcodec/jpx/cjpx_decoder.h @@ -50,6 +50,9 @@ class CJPX_Decoder { bool strict_mode); static void Sycc420ToRgbForTesting(opj_image_t* img); + static void Sycc422ToRgbForTesting(opj_image_t* img); + static void Sycc444ToRgbForTesting(opj_image_t* img); + static void ColorSyccToRgbForTesting(opj_image_t* img); ~CJPX_Decoder(); diff --git a/core/fxcodec/jpx/jpx_unittest.cpp b/core/fxcodec/jpx/jpx_unittest.cpp index abb0a2f0d3..2902764d21 100644 --- a/core/fxcodec/jpx/jpx_unittest.cpp +++ b/core/fxcodec/jpx/jpx_unittest.cpp @@ -463,4 +463,187 @@ TEST(fxcodec, YUV420ToRGB) { FX_Free(img.comps); } +TEST(fxcodec, YUV422ToRGB) { + opj_image_comp_t u = { + .dx = 2, + .dy = 1, + .w = 16, + .h = 16, + .prec = 8, + .bpp = 8, + }; + opj_image_comp_t v = { + .dx = 2, + .dy = 1, + .w = 16, + .h = 16, + .prec = 8, + .bpp = 8, + }; + opj_image_comp_t y = { + .dx = 1, + .dy = 1, + .w = 32, + .h = 16, + .prec = 8, + .bpp = 8, + }; + opj_image_t img = { + .x1 = y.w, + .y1 = y.h, + .numcomps = 3, + .color_space = OPJ_CLRSPC_SYCC, + .comps = FX_Alloc(opj_image_comp_t, 3), + }; + y.data = static_cast( + opj_image_data_alloc(y.w * y.h * sizeof(OPJ_INT32))); + v.data = static_cast( + opj_image_data_alloc(v.w * v.h * sizeof(OPJ_INT32))); + u.data = static_cast( + opj_image_data_alloc(u.w * u.h * sizeof(OPJ_INT32))); + + UNSAFE_TODO({ + FXSYS_memset(y.data, 1, y.w * y.h * sizeof(OPJ_INT32)); + FXSYS_memset(u.data, 0, u.w * u.h * sizeof(OPJ_INT32)); + FXSYS_memset(v.data, 0, v.w * v.h * sizeof(OPJ_INT32)); + img.comps[0] = y; + img.comps[1] = u; + img.comps[2] = v; + CJPX_Decoder::Sycc422ToRgbForTesting(&img); + EXPECT_EQ(32u, img.comps[0].w); + EXPECT_EQ(16u, img.comps[0].h); + EXPECT_EQ(32u, img.comps[1].w); + EXPECT_EQ(16u, img.comps[1].h); + EXPECT_EQ(32u, img.comps[2].w); + EXPECT_EQ(16u, img.comps[2].h); + opj_image_data_free(img.comps[0].data); + opj_image_data_free(img.comps[1].data); + opj_image_data_free(img.comps[2].data); + }); + FX_Free(img.comps); +} + +TEST(fxcodec, YUV444ToRGB) { + opj_image_comp_t u = { + .dx = 1, + .dy = 1, + .w = 16, + .h = 16, + .prec = 8, + .bpp = 8, + }; + opj_image_comp_t v = { + .dx = 1, + .dy = 1, + .w = 16, + .h = 16, + .prec = 8, + .bpp = 8, + }; + opj_image_comp_t y = { + .dx = 1, + .dy = 1, + .w = 16, + .h = 16, + .prec = 8, + .bpp = 8, + }; + opj_image_t img = { + .x1 = y.w, + .y1 = y.h, + .numcomps = 3, + .color_space = OPJ_CLRSPC_SYCC, + .comps = FX_Alloc(opj_image_comp_t, 3), + }; + y.data = static_cast( + opj_image_data_alloc(y.w * y.h * sizeof(OPJ_INT32))); + v.data = static_cast( + opj_image_data_alloc(v.w * v.h * sizeof(OPJ_INT32))); + u.data = static_cast( + opj_image_data_alloc(u.w * u.h * sizeof(OPJ_INT32))); + + UNSAFE_TODO({ + FXSYS_memset(y.data, 1, y.w * y.h * sizeof(OPJ_INT32)); + FXSYS_memset(u.data, 0, u.w * u.h * sizeof(OPJ_INT32)); + FXSYS_memset(v.data, 0, v.w * v.h * sizeof(OPJ_INT32)); + img.comps[0] = y; + img.comps[1] = u; + img.comps[2] = v; + CJPX_Decoder::Sycc444ToRgbForTesting(&img); + EXPECT_EQ(16u, img.comps[0].w); + EXPECT_EQ(16u, img.comps[0].h); + EXPECT_EQ(16u, img.comps[1].w); + EXPECT_EQ(16u, img.comps[1].h); + EXPECT_EQ(16u, img.comps[2].w); + EXPECT_EQ(16u, img.comps[2].h); + opj_image_data_free(img.comps[0].data); + opj_image_data_free(img.comps[1].data); + opj_image_data_free(img.comps[2].data); + }); + FX_Free(img.comps); +} + +TEST(fxcodec, ColorSyccToRGB) { + opj_image_t img = { + .numcomps = 2, + .color_space = OPJ_CLRSPC_SYCC, + .comps = FX_Alloc(opj_image_comp_t, 2), + }; + CJPX_Decoder::ColorSyccToRgbForTesting(&img); + EXPECT_EQ(OPJ_CLRSPC_GRAY, img.color_space); + FX_Free(img.comps); + + opj_image_comp_t u = { + .dx = 1, + .dy = 1, + .w = 16, + .h = 16, + .prec = 8, + .bpp = 8, + }; + opj_image_comp_t v = { + .dx = 1, + .dy = 1, + .w = 16, + .h = 16, + .prec = 8, + .bpp = 8, + }; + opj_image_comp_t y = { + .dx = 1, + .dy = 1, + .w = 16, + .h = 16, + .prec = 8, + .bpp = 8, + }; + + img = { + .numcomps = 3, + .color_space = OPJ_CLRSPC_SYCC, + .comps = FX_Alloc(opj_image_comp_t, 3), + }; + y.data = static_cast( + opj_image_data_alloc(y.w * y.h * sizeof(OPJ_INT32))); + v.data = static_cast( + opj_image_data_alloc(v.w * v.h * sizeof(OPJ_INT32))); + u.data = static_cast( + opj_image_data_alloc(u.w * u.h * sizeof(OPJ_INT32))); + + UNSAFE_TODO({ + FXSYS_memset(y.data, 1, y.w * y.h * sizeof(OPJ_INT32)); + FXSYS_memset(u.data, 0, u.w * u.h * sizeof(OPJ_INT32)); + FXSYS_memset(v.data, 0, v.w * v.h * sizeof(OPJ_INT32)); + img.comps[0] = y; + img.comps[1] = u; + img.comps[2] = v; + CJPX_Decoder::ColorSyccToRgbForTesting(&img); + EXPECT_EQ(OPJ_CLRSPC_SRGB, img.color_space); + opj_image_data_free(img.comps[0].data); + opj_image_data_free(img.comps[1].data); + opj_image_data_free(img.comps[2].data); + }); + FX_Free(img.comps); +} + } // namespace fxcodec diff --git a/core/fxcodec/progressive_decoder_unittest.cpp b/core/fxcodec/progressive_decoder_unittest.cpp index ef1f28906e..0d439a4047 100644 --- a/core/fxcodec/progressive_decoder_unittest.cpp +++ b/core/fxcodec/progressive_decoder_unittest.cpp @@ -1085,4 +1085,53 @@ TEST_F(ProgressiveDecoderTest, JpegInvalidComponents) { EXPECT_EQ(FXCODEC_STATUS::kError, status); } +TEST_F(ProgressiveDecoderTest, TiffInvalidData) { + static constexpr uint8_t kBad[] = {0x00, 0x01, 0x02, 0x03}; + + ProgressiveDecoder decoder; + auto source = pdfium::MakeRetain(kBad); + CFX_DIBAttribute attr; + FXCODEC_STATUS status = + decoder.LoadImageInfo(std::move(source), FXCODEC_IMAGE_TIFF, &attr, true); + EXPECT_EQ(FXCODEC_STATUS::kError, status); +} + +TEST_F(ProgressiveDecoderTest, TiffGrayscale2x2) { + static constexpr auto kInput = std::to_array({ + 0x49, 0x49, 0x2a, 0x00, 0x08, 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x01, + 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, 0x01, + 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x02, 0x01, + 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x03, 0x01, + 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x06, 0x01, + 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x11, 0x01, + 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x7a, 0x00, 0x00, 0x00, 0x16, 0x01, + 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x17, 0x01, + 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x1c, 0x01, + 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x80, 0x40, 0xff, + }); + + ProgressiveDecoder decoder; + auto source = pdfium::MakeRetain(kInput); + CFX_DIBAttribute attr; + FXCODEC_STATUS status = + decoder.LoadImageInfo(std::move(source), FXCODEC_IMAGE_TIFF, &attr, true); + ASSERT_EQ(FXCODEC_STATUS::kFrameReady, status); + + EXPECT_EQ(2, decoder.GetWidth()); + EXPECT_EQ(2, decoder.GetHeight()); + EXPECT_EQ(FXDIB_Format::kBgra, decoder.GetBitmapFormat()); + + size_t frames; + std::tie(status, frames) = decoder.GetFrames(); + ASSERT_EQ(FXCODEC_STATUS::kDecodeReady, status); + EXPECT_EQ(1u, frames); + + auto bitmap = pdfium::MakeRetain(); + ASSERT_TRUE(bitmap->Create(decoder.GetWidth(), decoder.GetHeight(), + decoder.GetBitmapFormat())); + status = DecodeToBitmap(decoder, bitmap); + EXPECT_EQ(FXCODEC_STATUS::kDecodeFinished, status); +} + } // namespace fxcodec From 184a2a62d026688c6462f779262bdfb086535ed3 Mon Sep 17 00:00:00 2001 From: Andy Phan Date: Fri, 6 Nov 2026 10:26:34 -0800 Subject: [PATCH 31/34] Add unit test for GetMirrorChar() with supplementary-plane characters Add FXUnicodeTest.GetMirrorCharSupplementary to test pdfium::unicode::GetMirrorChar() with code points above U+FFFF. Bug: 555940408 Change-Id: I94a90de2875b0d1310581082f9b4dbcc38a0afc9 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163471 Reviewed-by: Tom Sepez Commit-Queue: Andy Phan --- core/fxcrt/BUILD.gn | 1 + core/fxcrt/fx_unicode_unittest.cpp | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 core/fxcrt/fx_unicode_unittest.cpp diff --git a/core/fxcrt/BUILD.gn b/core/fxcrt/BUILD.gn index 1623d11ad2..d0df2b57be 100644 --- a/core/fxcrt/BUILD.gn +++ b/core/fxcrt/BUILD.gn @@ -261,6 +261,7 @@ pdfium_unittest_source_set("unittests") { "fx_string_unittest.cpp", "fx_string_wrappers_unittest.cpp", "fx_system_unittest.cpp", + "fx_unicode_unittest.cpp", "mask_unittest.cpp", "maybe_owned_data_vector_unittest.cpp", "maybe_owned_unittest.cpp", diff --git a/core/fxcrt/fx_unicode_unittest.cpp b/core/fxcrt/fx_unicode_unittest.cpp new file mode 100644 index 0000000000..5a5ee24e43 --- /dev/null +++ b/core/fxcrt/fx_unicode_unittest.cpp @@ -0,0 +1,21 @@ +// Copyright 2026 The PDFium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcrt/fx_unicode.h" + +#include "build/build_config.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace pdfium::unicode { + +#if defined(WCHAR_T_IS_32_BIT) +TEST(FXUnicodeTest, GetMirrorCharSupplementary) { + // TODO(crbug.com/555940408): Should be L'\U00010000'. + EXPECT_EQ(L')', GetMirrorChar(L'\U00010000')); + // TODO(crbug.com/555940408): Should be L'🎨'. + EXPECT_EQ(L')', GetMirrorChar(L'🎨')); +} +#endif // defined(WCHAR_T_IS_32_BIT) + +} // namespace pdfium::unicode From 03ad8222f16dd4a107c461d1e147c75cff60040c Mon Sep 17 00:00:00 2001 From: Andy Phan Date: Fri, 6 Nov 2026 10:26:40 -0800 Subject: [PATCH 32/34] Fix GetMirrorChar() for supplementary-plane characters `kTextLayoutCodeProperties` only covers BMP code points (U+0000 to U+FFFF). For supplementary-plane characters (above U+FFFF), GetUnicodeProperties() fell back to returning 0, which GetMirrorChar() treated as mirror index 0 (`')'`) instead of no mirror. Have GetUnicodeProperties() return `std::optional` so GetMirrorChar() returns `wch` unchanged and GetBidiClass() returns `BidiClass::kON` when a code point is out of range. Bug: 555940408 Change-Id: I5e80dd9ddda78ac45dc366f5cf07771affadab41 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163472 Reviewed-by: Tom Sepez Commit-Queue: Andy Phan --- core/fxcrt/fx_unicode.cpp | 19 +++++++++++++------ core/fxcrt/fx_unicode_unittest.cpp | 6 ++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/core/fxcrt/fx_unicode.cpp b/core/fxcrt/fx_unicode.cpp index 847d1b924c..6683934a7c 100644 --- a/core/fxcrt/fx_unicode.cpp +++ b/core/fxcrt/fx_unicode.cpp @@ -9,6 +9,7 @@ #include #include +#include #include "core/fxcrt/check.h" #include "core/fxcrt/check_op.h" @@ -38,14 +39,14 @@ constexpr uint16_t kTextLayoutCodeProperties[] = { static_assert(std::size(kTextLayoutCodeProperties) == 65536, "missing characters"); -uint16_t GetUnicodeProperties(wchar_t wch) { +std::optional GetUnicodeProperties(wchar_t wch) { size_t idx = static_cast(wch); if (idx < std::size(kTextLayoutCodeProperties)) { // SAFETY: `std::size(kTextLayoutCodeProperties)` is the size of the table, // so the condition above verifies `idx` is in range. return UNSAFE_BUFFERS(kTextLayoutCodeProperties[idx]); } - return 0; + return std::nullopt; } #ifdef PDF_ENABLE_XFA @@ -142,8 +143,11 @@ constexpr uint16_t kFXTextLayoutBidiMirror[] = { namespace pdfium::unicode { wchar_t GetMirrorChar(wchar_t wch) { - uint16_t prop = GetUnicodeProperties(wch); - size_t idx = prop >> kMirrorBitPos; + std::optional prop = GetUnicodeProperties(wch); + if (!prop.has_value()) { + return wch; + } + size_t idx = prop.value() >> kMirrorBitPos; if (idx == kMirrorMax) { return wch; } @@ -154,8 +158,11 @@ wchar_t GetMirrorChar(wchar_t wch) { } BidiClass GetBidiClass(wchar_t wch) { - uint16_t prop = GetUnicodeProperties(wch); - uint16_t result = (prop & kBidiClassBitMask) >> kBidiClassBitPos; + std::optional prop = GetUnicodeProperties(wch); + if (!prop.has_value()) { + return BidiClass::kON; + } + uint16_t result = (prop.value() & kBidiClassBitMask) >> kBidiClassBitPos; DCHECK_LE(result, static_cast(BidiClass::kPDF)); return static_cast(result); } diff --git a/core/fxcrt/fx_unicode_unittest.cpp b/core/fxcrt/fx_unicode_unittest.cpp index 5a5ee24e43..f387d1fba8 100644 --- a/core/fxcrt/fx_unicode_unittest.cpp +++ b/core/fxcrt/fx_unicode_unittest.cpp @@ -11,10 +11,8 @@ namespace pdfium::unicode { #if defined(WCHAR_T_IS_32_BIT) TEST(FXUnicodeTest, GetMirrorCharSupplementary) { - // TODO(crbug.com/555940408): Should be L'\U00010000'. - EXPECT_EQ(L')', GetMirrorChar(L'\U00010000')); - // TODO(crbug.com/555940408): Should be L'🎨'. - EXPECT_EQ(L')', GetMirrorChar(L'🎨')); + EXPECT_EQ(L'\U00010000', GetMirrorChar(L'\U00010000')); + EXPECT_EQ(L'🎨', GetMirrorChar(L'🎨')); } #endif // defined(WCHAR_T_IS_32_BIT) From b6ef863c0a7b02b2c454dea6be5bb0faf9545186 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 6 Nov 2026 16:27:10 -0800 Subject: [PATCH 33/34] Add unit tests for core/fxcrt - fx_codepage: Add fx_codepage_unittest.cpp covering charset and codepage lookups, CJK character set checks, charset flags, and conversion functions. - cfx_memorystream: Add tests for IsEOF(), GetPosition(), Flush(), and boundary conditions in ReadBlockAtOffset(). - cfx_datetime: Add tests for FX_IsLeapYear(), FX_DaysInMonth(), GetDayOfWeek() across common and BCE dates, and accessors. - cfx_fileaccess_stream: Add cfx_fileaccess_stream_unittest.cpp Change-Id: Id78224f1d9064aa6e601d7e7ec4b2852402dabfe Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163510 Commit-Queue: Tom Sepez Reviewed-by: Andy Phan --- core/fxcrt/BUILD.gn | 2 + core/fxcrt/cfx_datetime_unittest.cpp | 64 +++++++ core/fxcrt/cfx_fileaccess_stream_unittest.cpp | 120 +++++++++++++ core/fxcrt/cfx_memorystream_unittest.cpp | 43 ++++- core/fxcrt/fx_codepage_unittest.cpp | 161 ++++++++++++++++++ 5 files changed, 387 insertions(+), 3 deletions(-) create mode 100644 core/fxcrt/cfx_fileaccess_stream_unittest.cpp create mode 100644 core/fxcrt/fx_codepage_unittest.cpp diff --git a/core/fxcrt/BUILD.gn b/core/fxcrt/BUILD.gn index d0df2b57be..976b1b5a77 100644 --- a/core/fxcrt/BUILD.gn +++ b/core/fxcrt/BUILD.gn @@ -241,6 +241,7 @@ pdfium_unittest_source_set("unittests") { "cfx_bidi_resolver_unittest.cpp", "cfx_bitstream_unittest.cpp", "cfx_datetime_unittest.cpp", + "cfx_fileaccess_stream_unittest.cpp", "cfx_read_only_container_stream_unittest.cpp", "cfx_seekablestreamproxy_unittest.cpp", "cfx_timer_unittest.cpp", @@ -250,6 +251,7 @@ pdfium_unittest_source_set("unittests") { "fixed_size_data_vector_unittest.cpp", "fx_bidi_unittest.cpp", "fx_ceil_div_unittest.cpp", + "fx_codepage_unittest.cpp", "fx_coordinates_unittest.cpp", "fx_extension_unittest.cpp", "fx_memcpy_wrappers_unittest.cpp", diff --git a/core/fxcrt/cfx_datetime_unittest.cpp b/core/fxcrt/cfx_datetime_unittest.cpp index 5349e2c15c..ab961a35dd 100644 --- a/core/fxcrt/cfx_datetime_unittest.cpp +++ b/core/fxcrt/cfx_datetime_unittest.cpp @@ -16,3 +16,67 @@ TEST_F(FakeTimeTest, Now) { EXPECT_EQ(21, dt.GetSecond()); EXPECT_EQ(0, dt.GetMillisecond()); } + +TEST(CFXDateTimeTest, LeapYear) { + EXPECT_TRUE(FX_IsLeapYear(2000)); + EXPECT_TRUE(FX_IsLeapYear(2024)); + EXPECT_FALSE(FX_IsLeapYear(1900)); + EXPECT_FALSE(FX_IsLeapYear(2023)); +} + +TEST(CFXDateTimeTest, DaysInMonth) { + EXPECT_EQ(31, FX_DaysInMonth(2023, 1)); + EXPECT_EQ(28, FX_DaysInMonth(2023, 2)); + EXPECT_EQ(29, FX_DaysInMonth(2024, 2)); + EXPECT_EQ(31, FX_DaysInMonth(2023, 3)); + EXPECT_EQ(30, FX_DaysInMonth(2023, 4)); + EXPECT_EQ(31, FX_DaysInMonth(2023, 5)); + EXPECT_EQ(30, FX_DaysInMonth(2023, 6)); + EXPECT_EQ(31, FX_DaysInMonth(2023, 7)); + EXPECT_EQ(31, FX_DaysInMonth(2023, 8)); + EXPECT_EQ(30, FX_DaysInMonth(2023, 9)); + EXPECT_EQ(31, FX_DaysInMonth(2023, 10)); + EXPECT_EQ(30, FX_DaysInMonth(2023, 11)); + EXPECT_EQ(31, FX_DaysInMonth(2023, 12)); +} + +TEST(CFXDateTimeTest, DayOfWeek) { + CFX_DateTime dt_now(2020, 4, 23, 0, 0, 0, 0); + EXPECT_EQ(4, dt_now.GetDayOfWeek()); + + // Year 1 AD. + CFX_DateTime dt_ad1(1, 1, 1, 0, 0, 0, 0); + EXPECT_EQ(1, dt_ad1.GetDayOfWeek()); + + // Negative year (BCE). + CFX_DateTime dt_bce(-1, 1, 1, 0, 0, 0, 0); + EXPECT_EQ(0, dt_bce.GetDayOfWeek()); + + // Negative leap year. + CFX_DateTime dt_bce_leap(-4, 2, 29, 0, 0, 0, 0); + EXPECT_EQ(6, dt_bce_leap.GetDayOfWeek()); +} + +TEST(CFXDateTimeTest, AccessorsAndReset) { + CFX_DateTime dt; + EXPECT_FALSE(dt.IsSet()); + EXPECT_EQ(0, dt.GetYear()); + + dt.SetDate(2026, 11, 3); + dt.SetTime(14, 30, 45, 123); + EXPECT_TRUE(dt.IsSet()); + EXPECT_EQ(2026, dt.GetYear()); + EXPECT_EQ(11, dt.GetMonth()); + EXPECT_EQ(3, dt.GetDay()); + EXPECT_EQ(14, dt.GetHour()); + EXPECT_EQ(30, dt.GetMinute()); + EXPECT_EQ(45, dt.GetSecond()); + EXPECT_EQ(123, dt.GetMillisecond()); + + CFX_DateTime dt_copy(2026, 11, 3, 14, 30, 45, 123); + EXPECT_EQ(dt, dt_copy); + + dt.Reset(); + EXPECT_EQ(dt, CFX_DateTime()); + EXPECT_NE(dt, dt_copy); +} diff --git a/core/fxcrt/cfx_fileaccess_stream_unittest.cpp b/core/fxcrt/cfx_fileaccess_stream_unittest.cpp new file mode 100644 index 0000000000..19760e4fc8 --- /dev/null +++ b/core/fxcrt/cfx_fileaccess_stream_unittest.cpp @@ -0,0 +1,120 @@ +// Copyright 2026 The PDFium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcrt/cfx_fileaccess_stream.h" + +#include + +#include +#include +#include +#include +#include + +#include "core/fxcrt/fileaccess_iface.h" +#include "core/fxcrt/fx_string.h" +#include "core/fxcrt/retain_ptr.h" +#include "core/fxcrt/span.h" +#include "core/fxcrt/span_util.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::ElementsAre; + +namespace { + +class FakeFileAccess final : public FileAccessIface { + public: + FakeFileAccess() = default; + ~FakeFileAccess() override = default; + + bool Open(const ByteString& file_name) override { return true; } + void Close() override {} + + FX_FILESIZE GetSize() const override { + return static_cast(buffer_.size()); + } + + FX_FILESIZE GetPosition() const override { return pos_; } + + FX_FILESIZE SetPosition(FX_FILESIZE pos) override { + if (pos < 0 || static_cast(pos) > buffer_.size()) { + return -1; + } + pos_ = pos; + return pos_; + } + + size_t Read(pdfium::span buffer) override { + if (pos_ < 0 || static_cast(pos_) >= buffer_.size()) { + return 0; + } + size_t count = + std::min(buffer.size(), buffer_.size() - static_cast(pos_)); + fxcrt::spancpy(buffer, pdfium::span(buffer_).subspan( + static_cast(pos_), count)); + pos_ += count; + return count; + } + + bool Write(pdfium::span buffer) override { + if (pos_ < 0) { + return false; + } + size_t offset = static_cast(pos_); + if (offset + buffer.size() > buffer_.size()) { + buffer_.resize(offset + buffer.size()); + } + fxcrt::spancpy(pdfium::span(buffer_).subspan(offset), buffer); + pos_ += buffer.size(); + return true; + } + + size_t ReadPos(pdfium::span buffer, FX_FILESIZE pos) override { + if (SetPosition(pos) == -1) { + return 0; + } + return Read(buffer); + } + + bool Flush() override { return true; } + bool Truncate(FX_FILESIZE szFile) override { return true; } + + private: + std::vector buffer_; + FX_FILESIZE pos_ = 0; +}; + +} // namespace + +TEST(CFXFileAccessStreamTest, CreateFromNonExistentFilename) { + RetainPtr stream = + CFX_FileAccessStream::CreateFromFilename("non_existent_file_path.dat"); + EXPECT_FALSE(stream); +} + +TEST(CFXFileAccessStreamTest, Operations) { + auto fake = std::make_unique(); + auto stream = pdfium::MakeRetain(std::move(fake)); + ASSERT_TRUE(stream); + + EXPECT_EQ(0, stream->GetSize()); + EXPECT_EQ(0, stream->GetPosition()); + EXPECT_TRUE(stream->IsEOF()); + EXPECT_TRUE(stream->Flush()); + + static constexpr uint8_t kData[] = {'a', 'b', 'c', 'd'}; + EXPECT_TRUE(stream->WriteBlock(kData)); + EXPECT_EQ(4, stream->GetSize()); + EXPECT_EQ(4, stream->GetPosition()); + EXPECT_TRUE(stream->IsEOF()); + + std::array read_buf; + EXPECT_TRUE(stream->ReadBlockAtOffset(read_buf, 1)); + EXPECT_THAT(read_buf, ElementsAre('b', 'c')); + EXPECT_EQ(3, stream->GetPosition()); + EXPECT_FALSE(stream->IsEOF()); + + EXPECT_FALSE(stream->ReadBlockAtOffset(read_buf, 10)); +} diff --git a/core/fxcrt/cfx_memorystream_unittest.cpp b/core/fxcrt/cfx_memorystream_unittest.cpp index 5be3323ee2..b64bb1a17e 100644 --- a/core/fxcrt/cfx_memorystream_unittest.cpp +++ b/core/fxcrt/cfx_memorystream_unittest.cpp @@ -24,9 +24,6 @@ TEST(CFXMemoryStreamTest, ReadBlockAtOffset) { ASSERT_TRUE(stream->ReadBlockAtOffset(buffer, 3)); ASSERT_THAT(buffer, testing::ElementsAre('d', 'e', 'f')); - - ASSERT_FALSE(stream->ReadBlockAtOffset(buffer, 4)); - ASSERT_THAT(buffer, testing::ElementsAre('d', 'e', 'f')); } TEST(CFXMemoryStreamTest, WriteBlock) { @@ -56,3 +53,43 @@ TEST(CFXMemoryStreamTest, WriteZeroBytes) { ASSERT_TRUE(stream->WriteBlock({})); ASSERT_THAT(stream->GetSpan(), testing::ElementsAre('a', 'b', 'c')); } + +TEST(CFXMemoryStreamTest, PositionAndEOF) { + auto stream = pdfium::MakeRetain(); + EXPECT_EQ(0, stream->GetSize()); + EXPECT_EQ(0, stream->GetPosition()); + EXPECT_TRUE(stream->IsEOF()); + EXPECT_TRUE(stream->Flush()); + + const uint8_t kData[] = {1, 2, 3, 4}; + EXPECT_TRUE(stream->WriteBlock(kData)); + EXPECT_EQ(4, stream->GetSize()); + EXPECT_EQ(4, stream->GetPosition()); + EXPECT_TRUE(stream->IsEOF()); + + uint8_t buf[2]; + EXPECT_TRUE(stream->ReadBlockAtOffset(buf, 0)); + EXPECT_EQ(2, stream->GetPosition()); + EXPECT_FALSE(stream->IsEOF()); + + EXPECT_TRUE(stream->ReadBlockAtOffset(buf, 2)); + EXPECT_EQ(4, stream->GetPosition()); + EXPECT_TRUE(stream->IsEOF()); +} + +TEST(CFXMemoryStreamTest, ReadBlockAtOffsetInvalid) { + auto stream = pdfium::MakeRetain(); + const uint8_t kData[] = {1, 2, 3}; + EXPECT_TRUE(stream->WriteBlock(kData)); + + // Empty buffer returns false. + EXPECT_FALSE(stream->ReadBlockAtOffset({}, 0)); + + // Negative offset returns false. + uint8_t buf[1]; + EXPECT_FALSE(stream->ReadBlockAtOffset(buf, -1)); + + // Read beyond size returns false. + EXPECT_FALSE(stream->ReadBlockAtOffset(buf, 3)); + EXPECT_FALSE(stream->ReadBlockAtOffset(buf, 10)); +} diff --git a/core/fxcrt/fx_codepage_unittest.cpp b/core/fxcrt/fx_codepage_unittest.cpp new file mode 100644 index 0000000000..5b9fb433f1 --- /dev/null +++ b/core/fxcrt/fx_codepage_unittest.cpp @@ -0,0 +1,161 @@ +// Copyright 2026 The PDFium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fxcrt/fx_codepage.h" + +#include + +#include "build/build_config.h" +#include "core/fxcrt/bytestring.h" +#include "core/fxcrt/widestring.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::ElementsAre; + +TEST(FXCodePageTest, GetACP) { +#if BUILDFLAG(IS_WIN) + EXPECT_NE(FX_CodePage::kFailure, FX_GetACP()); +#else + EXPECT_EQ(FX_CodePage::kDefANSI, FX_GetACP()); +#endif +} + +TEST(FXCodePageTest, GetCodePageFromCharset) { + EXPECT_EQ(FX_CodePage::kMSWin_WesternEuropean, + FX_GetCodePageFromCharset(FX_Charset::kANSI)); + EXPECT_EQ(FX_CodePage::kShiftJIS, + FX_GetCodePageFromCharset(FX_Charset::kShiftJIS)); + EXPECT_EQ(FX_CodePage::kHangul, + FX_GetCodePageFromCharset(FX_Charset::kHangul)); + EXPECT_EQ(FX_CodePage::kChineseSimplified, + FX_GetCodePageFromCharset(FX_Charset::kChineseSimplified)); + EXPECT_EQ(FX_CodePage::kChineseTraditional, + FX_GetCodePageFromCharset(FX_Charset::kChineseTraditional)); + EXPECT_EQ(FX_CodePage::kMSWin_Cyrillic, + FX_GetCodePageFromCharset(FX_Charset::kMSWin_Cyrillic)); + EXPECT_EQ(FX_CodePage::kMSDOS_Thai, + FX_GetCodePageFromCharset(FX_Charset::kThai)); + EXPECT_EQ(FX_CodePage::kMSWin_EasternEuropean, + FX_GetCodePageFromCharset(FX_Charset::kMSWin_EasternEuropean)); + + // Invalid charset returns kFailure. + EXPECT_EQ(FX_CodePage::kFailure, + FX_GetCodePageFromCharset(static_cast(253))); +} + +TEST(FXCodePageTest, GetCharsetFromCodePage) { + EXPECT_EQ(FX_Charset::kANSI, + FX_GetCharsetFromCodePage(FX_CodePage::kMSWin_WesternEuropean)); + EXPECT_EQ(FX_Charset::kShiftJIS, + FX_GetCharsetFromCodePage(FX_CodePage::kShiftJIS)); + EXPECT_EQ(FX_Charset::kHangul, + FX_GetCharsetFromCodePage(FX_CodePage::kHangul)); + EXPECT_EQ(FX_Charset::kChineseSimplified, + FX_GetCharsetFromCodePage(FX_CodePage::kChineseSimplified)); + EXPECT_EQ(FX_Charset::kChineseTraditional, + FX_GetCharsetFromCodePage(FX_CodePage::kChineseTraditional)); + EXPECT_EQ(FX_Charset::kMSWin_Cyrillic, + FX_GetCharsetFromCodePage(FX_CodePage::kMSWin_Cyrillic)); + EXPECT_EQ(FX_Charset::kThai, + FX_GetCharsetFromCodePage(FX_CodePage::kMSDOS_Thai)); + EXPECT_EQ(FX_Charset::kMSWin_EasternEuropean, + FX_GetCharsetFromCodePage(FX_CodePage::kMSWin_EasternEuropean)); + + // Unknown codepage defaults to kANSI. + EXPECT_EQ(FX_Charset::kANSI, + FX_GetCharsetFromCodePage(FX_CodePage::kFailure)); +} + +TEST(FXCodePageTest, GetCharsetFromInt) { + EXPECT_EQ(FX_Charset::kANSI, FX_GetCharsetFromInt(0)); + EXPECT_EQ(FX_Charset::kDefault, FX_GetCharsetFromInt(1)); + EXPECT_EQ(FX_Charset::kSymbol, FX_GetCharsetFromInt(2)); + EXPECT_EQ(FX_Charset::kMAC_Roman, FX_GetCharsetFromInt(77)); + EXPECT_EQ(FX_Charset::kMAC_ShiftJIS, FX_GetCharsetFromInt(78)); + EXPECT_EQ(FX_Charset::kShiftJIS, FX_GetCharsetFromInt(128)); + EXPECT_EQ(FX_Charset::kHangul, FX_GetCharsetFromInt(129)); + EXPECT_EQ(FX_Charset::kJohab, FX_GetCharsetFromInt(130)); + EXPECT_EQ(FX_Charset::kChineseSimplified, FX_GetCharsetFromInt(134)); + EXPECT_EQ(FX_Charset::kChineseTraditional, FX_GetCharsetFromInt(136)); + EXPECT_EQ(FX_Charset::kMSWin_Greek, FX_GetCharsetFromInt(161)); + EXPECT_EQ(FX_Charset::kMSWin_Turkish, FX_GetCharsetFromInt(162)); + EXPECT_EQ(FX_Charset::kMSWin_Vietnamese, FX_GetCharsetFromInt(163)); + EXPECT_EQ(FX_Charset::kMSWin_Hebrew, FX_GetCharsetFromInt(177)); + EXPECT_EQ(FX_Charset::kMSWin_Arabic, FX_GetCharsetFromInt(178)); + EXPECT_EQ(FX_Charset::kMSWin_Baltic, FX_GetCharsetFromInt(186)); + EXPECT_EQ(FX_Charset::kMSWin_Cyrillic, FX_GetCharsetFromInt(204)); + EXPECT_EQ(FX_Charset::kThai, FX_GetCharsetFromInt(222)); + EXPECT_EQ(FX_Charset::kMSWin_EasternEuropean, FX_GetCharsetFromInt(238)); + EXPECT_EQ(FX_Charset::kOEM, FX_GetCharsetFromInt(255)); + + // Invalid values return kANSI. + EXPECT_EQ(FX_Charset::kANSI, FX_GetCharsetFromInt(-1)); + EXPECT_EQ(FX_Charset::kANSI, FX_GetCharsetFromInt(999)); +} + +TEST(FXCodePageTest, CharsetFlagForCharset) { + EXPECT_EQ(FX_CharsetFlag::kANSI, FX_CharsetFlagForCharset(FX_Charset::kANSI)); + EXPECT_EQ(FX_CharsetFlag::kSymbol, + FX_CharsetFlagForCharset(FX_Charset::kSymbol)); + EXPECT_EQ(FX_CharsetFlag::kShiftJIS, + FX_CharsetFlagForCharset(FX_Charset::kShiftJIS)); + EXPECT_EQ(FX_CharsetFlag::kHangul, + FX_CharsetFlagForCharset(FX_Charset::kHangul)); + EXPECT_EQ(FX_CharsetFlag::kJohab, + FX_CharsetFlagForCharset(FX_Charset::kJohab)); + EXPECT_EQ(FX_CharsetFlag::kChineseSimplified, + FX_CharsetFlagForCharset(FX_Charset::kChineseSimplified)); + EXPECT_EQ(FX_CharsetFlag::kChineseTraditional, + FX_CharsetFlagForCharset(FX_Charset::kChineseTraditional)); + EXPECT_EQ(FX_CharsetFlag::kMSWin_Greek, + FX_CharsetFlagForCharset(FX_Charset::kMSWin_Greek)); + EXPECT_EQ(FX_CharsetFlag::kMSWin_Turkish, + FX_CharsetFlagForCharset(FX_Charset::kMSWin_Turkish)); + EXPECT_EQ(FX_CharsetFlag::kMSWin_Vietnamese, + FX_CharsetFlagForCharset(FX_Charset::kMSWin_Vietnamese)); + EXPECT_EQ(FX_CharsetFlag::kMSWin_Hebrew, + FX_CharsetFlagForCharset(FX_Charset::kMSWin_Hebrew)); + EXPECT_EQ(FX_CharsetFlag::kMSWin_Arabic, + FX_CharsetFlagForCharset(FX_Charset::kMSWin_Arabic)); + EXPECT_EQ(FX_CharsetFlag::kMSWin_Baltic, + FX_CharsetFlagForCharset(FX_Charset::kMSWin_Baltic)); + EXPECT_EQ(FX_CharsetFlag::kMSWin_Cyrillic, + FX_CharsetFlagForCharset(FX_Charset::kMSWin_Cyrillic)); + EXPECT_EQ(FX_CharsetFlag::kThai, FX_CharsetFlagForCharset(FX_Charset::kThai)); + EXPECT_EQ(FX_CharsetFlag::kMSWin_EasternEuropean, + FX_CharsetFlagForCharset(FX_Charset::kMSWin_EasternEuropean)); + EXPECT_EQ(FX_CharsetFlag::kOEM, FX_CharsetFlagForCharset(FX_Charset::kOEM)); + + // Charsets without an explicit flag return kNone. + EXPECT_EQ(FX_CharsetFlag::kNone, + FX_CharsetFlagForCharset(FX_Charset::kDefault)); + EXPECT_EQ(FX_CharsetFlag::kNone, FX_CharsetFlagForCharset(FX_Charset::kUS)); + EXPECT_EQ(FX_CharsetFlag::kNone, + FX_CharsetFlagForCharset(static_cast(253))); +} + +TEST(FXCodePageTest, CharSetIsCJK) { + EXPECT_TRUE(FX_CharSetIsCJK(FX_Charset::kShiftJIS)); + EXPECT_TRUE(FX_CharSetIsCJK(FX_Charset::kHangul)); + EXPECT_TRUE(FX_CharSetIsCJK(FX_Charset::kChineseSimplified)); + EXPECT_TRUE(FX_CharSetIsCJK(FX_Charset::kChineseTraditional)); + + EXPECT_FALSE(FX_CharSetIsCJK(FX_Charset::kANSI)); + EXPECT_FALSE(FX_CharSetIsCJK(FX_Charset::kSymbol)); + EXPECT_FALSE(FX_CharSetIsCJK(FX_Charset::kMSWin_Cyrillic)); + EXPECT_FALSE(FX_CharSetIsCJK(FX_Charset::kThai)); + EXPECT_FALSE(FX_CharSetIsCJK(FX_Charset::kMSWin_EasternEuropean)); +} + +TEST(FXCodePageTest, CharacterConversion) { + std::array mb_buf; + EXPECT_EQ(4u, FX_WideCharToMultiByte(FX_CodePage::kDefANSI, L"Test", mb_buf)); + EXPECT_THAT(mb_buf, ElementsAre('T', 'e', 's', 't')); + + std::array wide_buf; + EXPECT_EQ(4u, + FX_MultiByteToWideChar(FX_CodePage::kDefANSI, "Test", wide_buf)); + EXPECT_THAT(wide_buf, ElementsAre(L'T', L'e', L's', L't')); +} From 6688c576fd851f4159c6fae39396da50939de110 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 6 Nov 2026 16:27:55 -0800 Subject: [PATCH 34/34] Add embedder test for FPDF_AddInstalledFont Test FPDF_AddInstalledFont in FPDFVersion1Versus2ComparisonTest by registering custom fonts (localized multi-byte names), during EnumFonts execution. Verify that PDFium queries the localized font name via GetFont and that page rendering succeeds. Change-Id: I92460806f3090837f73aced9bd0007f6e65d2c8b Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/163451 Reviewed-by: Andy Phan Commit-Queue: Tom Sepez --- fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp | 31 ++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp b/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp index 8d79455c2c..43103ac0c2 100644 --- a/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp +++ b/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp @@ -5,6 +5,7 @@ #include "public/fpdf_sysfontinfo.h" #include +#include #include #include "build/build_config.h" @@ -21,6 +22,8 @@ struct TrackingFontInfo { FPDF_SYSFONTINFO* wrapped_font_info; int* enum_fonts_call_count; int* map_font_call_count; + std::vector* get_font_faces; + std::vector>* fonts_to_add; }; extern "C" { @@ -28,6 +31,10 @@ extern "C" { void TrackingEnumFonts(FPDF_SYSFONTINFO* sys_font_info, void* mapper) { auto* tracking_info = reinterpret_cast(sys_font_info); (*tracking_info->enum_fonts_call_count)++; + ASSERT_NE(tracking_info->fonts_to_add, nullptr); + for (const auto& [face, charset] : *tracking_info->fonts_to_add) { + FPDF_AddInstalledFont(mapper, face.c_str(), charset); + } if (tracking_info->wrapped_font_info && tracking_info->wrapped_font_info->EnumFonts) { tracking_info->wrapped_font_info->EnumFonts( @@ -55,6 +62,7 @@ void* TrackingMapFont(FPDF_SYSFONTINFO* sys_font_info, void* TrackingGetFont(FPDF_SYSFONTINFO* sys_font_info, const char* face) { auto* tracking_info = reinterpret_cast(sys_font_info); + tracking_info->get_font_faces->push_back(face); if (tracking_info->wrapped_font_info && tracking_info->wrapped_font_info->GetFont) { return tracking_info->wrapped_font_info->GetFont( @@ -239,6 +247,8 @@ class FPDFVersion1Versus2ComparisonTest : public EmbedderTest { tracking_font_info_.wrapped_font_info = wrapped_font_info_; tracking_font_info_.enum_fonts_call_count = &enum_fonts_call_count_; tracking_font_info_.map_font_call_count = &map_font_call_count_; + tracking_font_info_.get_font_faces = &get_font_faces_; + tracking_font_info_.fonts_to_add = &fonts_to_add_; FPDF_SetSystemFontInfo(&tracking_font_info_.base); } @@ -262,6 +272,8 @@ class FPDFVersion1Versus2ComparisonTest : public EmbedderTest { int enum_fonts_call_count_ = 0; int map_font_call_count_ = 0; + std::vector get_font_faces_; + std::vector> fonts_to_add_; FPDF_SYSFONTINFO* wrapped_font_info_ = nullptr; TrackingFontInfo tracking_font_info_ = {}; }; @@ -282,15 +294,28 @@ TEST_F(FPDFVersion1Versus2ComparisonTest, Version1CallsEnumFonts) { EXPECT_GT(enum_fonts_call_count_, 0); } +TEST_F(FPDFVersion1Versus2ComparisonTest, AddInstalledFont) { + fonts_to_add_ = { + {"\x81\x82\x83", FXFONT_DEFAULT_CHARSET}, + }; + SetupVersion(1); + ASSERT_TRUE(OpenDocument("hello_world.pdf")); + ScopedPage page = LoadScopedPage(0); + ASSERT_TRUE(page); + ScopedFPDFBitmap bitmap = RenderPage(page.get()); + ASSERT_TRUE(bitmap); + EXPECT_GT(enum_fonts_call_count_, 0); + EXPECT_THAT(get_font_faces_, testing::Contains("\x81\x82\x83")); +} + TEST_F(FPDFVersion1Versus2ComparisonTest, Version2SkipsEnumFonts) { SetupVersion(2); ASSERT_TRUE(OpenDocument("hello_world.pdf")); - FPDF_PAGE page = LoadPage(0); + ScopedPage page = LoadScopedPage(0); ASSERT_TRUE(page); - ScopedFPDFBitmap bitmap = RenderPage(page); + ScopedFPDFBitmap bitmap = RenderPage(page.get()); ASSERT_TRUE(bitmap); - UnloadPage(page); // Version 2 should NOT call EnumFonts but should call MapFont. EXPECT_EQ(enum_fonts_call_count_, 0);