commit 0a30c29ba84639c7cc631656121a78264ae78bb0
author Tom Sepez <tsepez@chromium.org> Mon Jul 22 18:16:09 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Mon Jul 22 18:16:09 2019 +0000
tree a1f14aaed76ae45253f9acf97508104015d56857
parent 9105249648ba25ecb1257ce0978e1d48f95a3658

Return copy of bind nodes vector from CJX_Object::GetBindNodes().

Otherwise, range-based for loops aren't safe in face of additional
manipulations of the set of nodes. Rename to GetBindNodesCopy() to
indicate the copy is intentional. Add HasBindNodes() for the one case
where we don't actually want a copy.

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

fxjs/xfa/cjx_object.cpp

diff --git a/fxjs/xfa/cjx_object.cpp b/fxjs/xfa/cjx_object.cpp
index 6b42802..73a730d 100644
--- a/fxjs/xfa/cjx_object.cpp
+++ b/fxjs/xfa/cjx_object.cpp
@@ -592,7 +592,7 @@
                 wsSaveTextArray[i], wsSaveTextArray[i], false, false);
             i++;
           }
-          for (auto* pArrayNode : *(pBind->GetBindItems())) {
+          for (auto* pArrayNode : pBind->GetBindItemsCopy()) {
             if (pArrayNode != ToNode(GetXFAObject())) {
               pArrayNode->JSObject()->SetContent(wsContent, wsContent, bNotify,
                                                  bScriptModify, false);
@@ -618,7 +618,7 @@
       if (pBindNode && bSyncData) {
         pBindNode->JSObject()->SetContent(wsContent, wsXMLValue, bNotify,
                                           bScriptModify, false);
-        for (auto* pArrayNode : *(pBindNode->GetBindItems())) {
+        for (auto* pArrayNode : pBindNode->GetBindItemsCopy()) {
           if (pArrayNode != ToNode(GetXFAObject())) {
             pArrayNode->JSObject()->SetContent(wsContent, wsContent, bNotify,
                                                true, false);
@@ -692,7 +692,7 @@
 
   SetAttributeValue(wsContent, wsXMLValue, bNotify, bScriptModify);
   if (pBindNode && bSyncData) {
-    for (auto* pArrayNode : *(pBindNode->GetBindItems())) {
+    for (auto* pArrayNode : pBindNode->GetBindItemsCopy()) {
       pArrayNode->JSObject()->SetContent(wsContent, wsContent, bNotify,
                                          bScriptModify, false);
     }
@@ -1418,7 +1418,7 @@
     CXFA_Node* pContainerNode = nullptr;
     if (ToNode(GetXFAObject())->GetPacketType() == XFA_PacketType::Datasets) {
       WideString wsPicture;
-      for (auto* pFormNode : *(ToNode(GetXFAObject())->GetBindItems())) {
+      for (auto* pFormNode : ToNode(GetXFAObject())->GetBindItemsCopy()) {
         if (!pFormNode || pFormNode->HasRemovedChildren())
           continue;
 

xfa/fxfa/parser/cxfa_node.cpp

diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 8204849..465f786 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -1285,7 +1285,7 @@
       return nullptr;
 
     CXFA_Node* pFieldNode = nullptr;
-    for (auto* pFormNode : *(pDataNode->GetBindItems())) {
+    for (auto* pFormNode : pDataNode->GetBindItemsCopy()) {
       if (!pFormNode || pFormNode->HasRemovedChildren())
         continue;
       pFieldNode = pFormNode->IsWidgetReady() ? pFormNode : nullptr;
@@ -5024,7 +5024,7 @@
       if (GetPacketType() == XFA_PacketType::Datasets) {
         for (CXFA_Node* pChildDataNode = GetFirstChild(); pChildDataNode;
              pChildDataNode = pChildDataNode->GetNextSibling()) {
-          if (!pChildDataNode->GetBindItems()->empty()) {
+          if (pChildDataNode->HasBindItems()) {
             bDeleteChildren = false;
             break;
           }

xfa/fxfa/parser/cxfa_node.h

diff --git a/xfa/fxfa/parser/cxfa_node.h b/xfa/fxfa/parser/cxfa_node.h
index 087d0cc..22063c3 100644
--- a/xfa/fxfa/parser/cxfa_node.h
+++ b/xfa/fxfa/parser/cxfa_node.h
@@ -182,7 +182,8 @@
   CXFA_Node* GetDataDescriptionNode();
   void SetDataDescriptionNode(CXFA_Node* pDataDescriptionNode);
   CXFA_Node* GetBindData();
-  std::vector<CXFA_Node*>* GetBindItems() { return &binding_nodes_; }
+  bool HasBindItems() const { return !binding_nodes_.empty(); }
+  std::vector<CXFA_Node*> GetBindItemsCopy() { return binding_nodes_; }
   int32_t AddBindItem(CXFA_Node* pFormNode);
   int32_t RemoveBindItem(CXFA_Node* pFormNode);
   bool HasBindItem() const;