r23616 by jghali - Fix potential crash when converting a page to masterpage in case some of the page items are using patterns

scribus-commit scribus-commit at lists.scribus.net
Thu Apr 16 12:22:52 UTC 2020


Author: jghali
Date: Thu Apr 16 12:22:52 2020
New Revision: 23616

URL: http://scribus.net/websvn/listing.php?repname=Scribus&sc=1&rev=23616
Log:
Fix potential crash when converting a page to masterpage in case some of the page items are using patterns

Modified:
    trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.cpp
    trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.h
    trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format_save.cpp

Modified: trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.cpp
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23616&path=/trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.cpp
==============================================================================
--- trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.cpp	(original)
+++ trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.cpp	Thu Apr 16 12:22:52 2020
@@ -215,7 +215,6 @@
 	QStack<int> groupStackMI2;
 	QStack<int> groupStackPI2;
 
-
 	itemRemap.clear();
 	itemNext.clear();
 	itemCount = 0;
@@ -229,6 +228,11 @@
 	bool firstElement = true;
 	bool success = true;
 
+	ReadObjectParams readObjectParams;
+	readObjectParams.baseDir = fileDir;
+	readObjectParams.itemKind = PageItem::StandardItem;
+	readObjectParams.loadingPage = true;
+
 	ScXmlStreamReader reader(data);
 	ScXmlStreamAttributes attrs;
 	while (!reader.atEnd() && !reader.hasError())
@@ -323,7 +327,7 @@
 		if ((tagName == "ITEM") || (tagName == "PAGEOBJECT") || (tagName == "FRAMEOBJECT"))
 		{
 			ItemInfo itemInfo;
-			success = readObject(m_Doc, reader, itemInfo, fileDir, true);
+			success = readObject(m_Doc, reader, readObjectParams, itemInfo);
 			if (!success)
 				break;
 			itemInfo.item->m_layerID = LayerToPaste;
@@ -785,6 +789,11 @@
 	bool success = true;
 	int  progress = 0;
 
+	ReadObjectParams readObjectParams;
+	readObjectParams.baseDir = fileDir;
+	readObjectParams.itemKind = PageItem::StandardItem;
+	readObjectParams.loadingPage = false;
+
 	ScXmlStreamReader reader(f);
 	ScXmlStreamAttributes attrs;
 	while (!reader.atEnd() && !reader.hasError())
@@ -849,7 +858,7 @@
 		if (tagName == "PAGEOBJECT" || tagName == "MASTEROBJECT" || tagName == "FRAMEOBJECT")
 		{
 			ItemInfo itemInfo;
-			success = readObject(m_Doc, reader, itemInfo, fileDir, false);
+			success = readObject(m_Doc, reader, readObjectParams, itemInfo);
 			if (!success) break;
 
 			if (isNewFormat)
@@ -1204,7 +1213,6 @@
 		if (!converted)
 			gItem->groupItemList = gpL;
 	}
-
 
 	while (groupStackF.count() > 0)
 	{
@@ -1364,6 +1372,11 @@
 	m_Doc->PageColors.clear();
 	m_Doc->Layers.clear();
 
+	ReadObjectParams readObjectParams;
+	readObjectParams.baseDir = fileDir;
+	readObjectParams.itemKind = PageItem::StandardItem;
+	readObjectParams.loadingPage = false;
+
 	bool firstElement = true;
 	bool success = true;
 	bool hasPageSets = false;
@@ -1549,7 +1562,7 @@
 		if (tagName == "PAGEOBJECT" || tagName == "MASTEROBJECT" || tagName == "FRAMEOBJECT")
 		{
 			ItemInfo itemInfo;
-			success = readObject(m_Doc, reader, itemInfo, fileDir, false);
+			success = readObject(m_Doc, reader, readObjectParams, itemInfo);
 			if (!success)
 				break;
 
@@ -3607,7 +3620,7 @@
 		{
 			ScXmlStreamAttributes attrs = reader.scAttributes();
 
-			QString label = "";
+			QString label;
 			if (attrs.hasAttribute("label"))
 				label = attrs.valueAsString("label");
 
@@ -3615,7 +3628,7 @@
 			if (attrs.hasAttribute("type"))
 				type = (MarkType) attrs.valueAsInt("type");
 
-			if (label != "" && type != MARKNoType)
+			if (!label.isEmpty() && type != MARKNoType)
 			{
 				Mark* mark = doc->newMark();
 				mark->label=attrs.valueAsString("label");
@@ -3743,6 +3756,8 @@
 		qDebug() << "scribus150format: corrupted masterpage with empty name detected";
 		return true;
 	}
+
+	bool savedMasterPageMode = m_Doc->masterPageMode();
 	m_Doc->setMasterPageMode(!pageName.isEmpty());
 	ScPage* newPage = pageName.isEmpty() ? doc->addPage(pageNum) : doc->addMasterPage(pageNum, pageName);
 
@@ -3772,8 +3787,6 @@
 			newPage->setSize(pageSize);
 	}
 
-
-
 	newPage->setInitialHeight(newPage->height());
 	newPage->setInitialWidth(newPage->width());
 	newPage->initialMargins.setTop(qMax(0.0, attrs.valueAsDouble("BORDERTOP")));
@@ -3783,8 +3796,9 @@
 	newPage->marginPreset = attrs.valueAsInt("PRESET", 0);
 	newPage->Margins.setTop(newPage->initialMargins.top());
 	newPage->Margins.setBottom(newPage->initialMargins.bottom());
-	m_Doc->setMasterPageMode(false);
-	//m_Doc->Pages=&m_Doc->DocPages;
+
+	m_Doc->setMasterPageMode(savedMasterPageMode);
+
 	// guides reading
 	newPage->guides.setHorizontalAutoGap(attrs.valueAsDouble("AGhorizontalAutoGap", 0.0));
 	newPage->guides.setVerticalAutoGap(attrs.valueAsDouble("AGverticalAutoGap", 0.0));
@@ -3815,12 +3829,13 @@
 	return true;
 }
 
-bool Scribus150Format::readObject(ScribusDoc* doc, ScXmlStreamReader& reader, ItemInfo& info, const QString& baseDir, bool loadPage, const QString& renamedPageName)
+bool Scribus150Format::readObject(ScribusDoc* doc, ScXmlStreamReader& reader, const ReadObjectParams& readObjectParams, ItemInfo& info)
 {
 	QStringRef tagName = reader.name();
 	ScXmlStreamAttributes attrs = reader.scAttributes();
 
-	if (!loadPage)
+	bool savedMasterPageMode = doc->masterPageMode();
+	if (!readObjectParams.loadingPage)
 	{
 		if (tagName == "PAGEOBJECT" || tagName == "FRAMEOBJECT" || tagName == "PatternItem" || tagName == "ITEM")
 			doc->setMasterPageMode(false);
@@ -3834,18 +3849,23 @@
 	else if (tagName =="PatternItem")
 		itemKind = PageItem::PatternItem;
 
-	int pagenr = -1;
+	// We are loading patterns, force itemKind to PatternItem
+	if (readObjectParams.itemKind == PageItem::PatternItem)
+		itemKind = PageItem::PatternItem;
+
+	int pageNr = -1;
 	QString masterPageName = attrs.valueAsString("OnMasterPage");
 	if ((!masterPageName.isEmpty()) && (tagName == "MASTEROBJECT"))
 	{
-		if (!renamedPageName.isEmpty())
-			masterPageName = renamedPageName;
+		if (!readObjectParams.renamedMasterPage.isEmpty())
+			masterPageName = readObjectParams.renamedMasterPage;
 		doc->setCurrentPage(doc->MasterPages.at(doc->MasterNames[masterPageName]));
-		pagenr = -2;
+		pageNr = -2;
 	}
 	layerFound = false;
-	clipPath = "";
-	PageItem* newItem = pasteItem(doc, attrs, baseDir, itemKind, pagenr);
+	clipPath.clear();
+
+	PageItem* newItem = pasteItem(doc, attrs, readObjectParams.baseDir, itemKind, pageNr);
 	newItem->setRedrawBounding();
 	if (tagName == "MASTEROBJECT")
 		newItem->setOwnerPage(doc->OnPage(newItem));
@@ -3880,7 +3900,7 @@
 		newItem->setLayer(doc->firstLayerID());
 	}
 
-	info.item     = newItem;
+	info.item   = newItem;
 	isNewFormat = attrs.hasAttribute("ItemID");
 	if (isNewFormat)
 	{
@@ -4174,25 +4194,28 @@
 			if (newItem->asGroupFrame())
 			{
 			//	bool success = true;
-				QList<PageItem*>* DItems = doc->Items;
-				QList<PageItem*> GroupItems;
-				doc->Items = &GroupItems;
+				QList<PageItem*>* docItems = doc->Items;
+				QList<PageItem*> groupItems;
+				doc->Items = &groupItems;
 				ItemInfo itemInfo;
 				// #12313: set the 'loadPage' parameter to true in order to
 				// avoid the change of page mode and the doc item lists switch
 				// when loading groups in masterpages
-			//	success =
-				readObject(doc, reader, itemInfo, baseDir, true);
-				for (int as = 0; as < GroupItems.count(); ++as)
-				{
-					PageItem* currItem = GroupItems.at(as);
+				ReadObjectParams readObjectParams2;
+				readObjectParams2.baseDir = readObjectParams.baseDir;
+				readObjectParams2.itemKind = (itemKind == PageItem::PatternItem) ? PageItem::PatternItem : PageItem::StandardItem;
+				readObjectParams2.loadingPage = true;
+				readObject(doc, reader, readObjectParams2, itemInfo);
+				for (int as = 0; as < groupItems.count(); ++as)
+				{
+					PageItem* currItem = groupItems.at(as);
 					newItem->groupItemList.append(currItem);
 					currItem->Parent = newItem;
 					currItem->m_layerID = newItem->m_layerID;
 					currItem->OwnPage = newItem->OwnPage;
 					currItem->OnMasterPage = newItem->OnMasterPage;
 				}
-				doc->Items = DItems;
+				doc->Items = docItems;
 			}
 		}
 		if (tName == "MARK")
@@ -4333,8 +4356,8 @@
 			}
 		}
 	}
-	if (!loadPage)
-		doc->setMasterPageMode(false);
+	if (!readObjectParams.loadingPage)
+		doc->setMasterPageMode(savedMasterPageMode);
 	return !reader.hasError();
 }
 
@@ -4367,6 +4390,11 @@
 	pat.scaleY  = attrs.valueAsDouble("scaleY", 0.0);
 	pat.xoffset = attrs.valueAsDouble("xoffset", 0.0);
 	pat.yoffset = attrs.valueAsDouble("yoffset", 0.0);
+	
+	ReadObjectParams readObjectParams;
+	readObjectParams.baseDir = baseDir;
+	readObjectParams.itemKind = PageItem::PatternItem;
+	readObjectParams.loadingPage = false;
 
 	bool savedAlignGrid = m_Doc->SnapGrid;
 	bool savedAlignGuides = m_Doc->SnapGuides;
@@ -4391,13 +4419,12 @@
 		ScXmlStreamAttributes tAtt = reader.attributes();
 			
 		ItemInfo itemInfo;
-		
-		//int ownPage = tAtt.valueAsInt("OwnPage");
-		success = readObject(doc, reader, itemInfo, baseDir, false);
+
+		success = readObject(doc, reader, readObjectParams, itemInfo);
 		if (!success) break;
 
 		itemInfo.item->OwnPage = -1 /*ownPage*/;
-		itemInfo.item->OnMasterPage = "";
+		itemInfo.item->OnMasterPage.clear();
 		if (isNewFormat)
 		{
 			if (itemInfo.item->isTableItem)
@@ -4906,7 +4933,7 @@
 	return !reader.hasError();
 }
 
-PageItem* Scribus150Format::pasteItem(ScribusDoc *doc, ScXmlStreamAttributes& attrs, const QString& baseDir, PageItem::ItemKind itemKind, int pagenr)
+PageItem* Scribus150Format::pasteItem(ScribusDoc *doc, ScXmlStreamAttributes& attrs, const QString& baseDir, PageItem::ItemKind itemKind, int pageNr)
 {
 	int z = 0;
 	struct ImageLoadRequest loadingInfo;
@@ -4944,8 +4971,8 @@
 	case PageItem::ItemType1:
 		z = doc->itemAdd(PageItem::Polygon, PageItem::Ellipse, x, y, w, h, pw, Pcolor, Pcolor2, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	//
 	case PageItem::ImageFrame:
@@ -4953,8 +4980,8 @@
 	case PageItem::LatexFrame: /*Everything that is valid for image frames is also valid for latex frames*/
 		z = doc->itemAdd(pt, PageItem::Unspecified, x, y, w, h, 1, doc->itemToolPrefs().imageFillColor, doc->itemToolPrefs().imageStrokeColor, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		UndoManager::instance()->setUndoEnabled(false);
 		currItem->ScaleType   = attrs.valueAsInt("SCALETYPE", 1);
 		currItem->AspectRatio = attrs.valueAsInt("RATIO", 0);
@@ -5033,53 +5060,53 @@
 	case PageItem::ItemType3:
 		z = doc->itemAdd(PageItem::Polygon, PageItem::Rectangle, x, y, w, h, pw, Pcolor, Pcolor2, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	//
 	case PageItem::PathText:
 		z = doc->itemAdd(PageItem::PathText, PageItem::Unspecified, x, y, w, h, pw, CommonStrings::None, Pcolor, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::NoteFrame:
 	case PageItem::TextFrame:
 		z = doc->itemAdd(pt, PageItem::Unspecified, x, y, w, h, pw, CommonStrings::None, Pcolor, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::Line:
 		z = doc->itemAdd(PageItem::Line, PageItem::Unspecified, x, y, w, h, pw, CommonStrings::None, Pcolor2, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::Polygon:
 		z = doc->itemAdd(PageItem::Polygon, PageItem::Unspecified, x, y, w, h, pw, Pcolor, Pcolor2, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::PolyLine:
 		z = doc->itemAdd(PageItem::PolyLine, PageItem::Unspecified, x, y, w, h, pw, Pcolor, Pcolor2, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::Symbol:
 		z = doc->itemAdd(PageItem::Symbol, PageItem::Unspecified, x, y, w, h, 0, CommonStrings::None, CommonStrings::None, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		currItem->setPattern( attrs.valueAsString("pattern", "") );
 		break;
 	case PageItem::Group:
 		z = doc->itemAdd(PageItem::Group, PageItem::Unspecified, x, y, w, h, 0, CommonStrings::None, CommonStrings::None, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		currItem->groupWidth = attrs.valueAsDouble("groupWidth", w);
 		currItem->groupHeight = attrs.valueAsDouble("groupHeight", h);
 		doc->GroupCounter++;
@@ -5087,26 +5114,26 @@
 	case PageItem::RegularPolygon:
 		z = doc->itemAdd(PageItem::RegularPolygon, PageItem::Unspecified, x, y, w, h, pw, Pcolor, Pcolor2, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::Arc:
 		z = doc->itemAdd(PageItem::Arc, PageItem::Unspecified, x, y, w, h, pw, Pcolor, Pcolor2, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::Spiral:
 		z = doc->itemAdd(PageItem::Spiral, PageItem::Unspecified, x, y, w, h, pw, Pcolor, Pcolor2, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2) 
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2) 
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::Table:
 		z = doc->itemAdd(PageItem::Table, PageItem::Unspecified, x, y, w, h, 0.0, CommonStrings::None, CommonStrings::None, itemKind);
 		currItem = doc->Items->at(z);
-		if (pagenr > -2)
-			currItem->setOwnerPage(pagenr);
+		if (pageNr > -2)
+			currItem->setOwnerPage(pageNr);
 		break;
 	case PageItem::Multiple:
 		Q_ASSERT(false);
@@ -6164,6 +6191,12 @@
 	}
 
 	QString fileDir = QFileInfo(fileName).absolutePath();
+	
+	ReadObjectParams readObjectParams;
+	readObjectParams.baseDir = fileDir;
+	readObjectParams.itemKind = PageItem::StandardItem;
+	readObjectParams.loadingPage = true;
+	readObjectParams.renamedMasterPage = Mpage ? renamedPageName : QString();
 
 	bool firstElement = true;
 	bool success = true;
@@ -6394,8 +6427,7 @@
 			else
 			{
 				ItemInfo itemInfo;
-				QString masterPageName = Mpage ? renamedPageName : QString();
-				success = readObject(m_Doc, reader, itemInfo, fileDir, true, masterPageName);
+				success = readObject(m_Doc, reader, readObjectParams, itemInfo);
 				if (!success) break;
 
 				PageItem* newItem = itemInfo.item;

Modified: trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.h
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23616&path=/trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.h
==============================================================================
--- trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.h	(original)
+++ trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format.h	Thu Apr 16 12:22:52 2020
@@ -88,6 +88,17 @@
 			int itemID;
 			bool isGroupFlag;
 			bool isWeldFlag;
+		};
+
+		class ReadObjectParams
+		{
+		public:
+			ReadObjectParams() {}
+
+			PageItem::ItemKind itemKind { PageItem::StandardItem };
+			bool    loadingPage { false };
+			QString baseDir;
+			QString renamedMasterPage;
 		};
 
 		void registerFormats();
@@ -119,7 +130,7 @@
 		bool readLatexInfo(PageItem_LatexFrame* item, ScXmlStreamReader& reader);
 		void readLayers(ScLayer& layer, ScXmlStreamAttributes& attrs);
 		bool readMultiline(multiLine& ml, ScXmlStreamReader& reader);
-		bool readObject(ScribusDoc* doc, ScXmlStreamReader& reader, ItemInfo& info, const QString& baseDir, bool loadPage, const QString& renamedPageName = QString());
+		bool readObject(ScribusDoc* doc, ScXmlStreamReader& reader, const ReadObjectParams& loadParams, ItemInfo& info);
 		bool readPage(ScribusDoc* doc, ScXmlStreamReader& reader);
 		bool readPageItemAttributes(PageItem* item, ScXmlStreamReader& reader);
 		bool readPageSets(ScribusDoc* doc, ScXmlStreamReader& reader);
@@ -154,7 +165,7 @@
 		
 		void updateNames2Ptr(); //after document load items pointers should be updated in markeredItemList
 
-		PageItem* pasteItem(ScribusDoc *doc, ScXmlStreamAttributes& attrs, const QString& baseDir, PageItem::ItemKind itemKind, int pagenr = -2 /* currentPage*/);
+		PageItem* pasteItem(ScribusDoc *doc, ScXmlStreamAttributes& attrs, const QString& baseDir, PageItem::ItemKind itemKind, int pageNr = -2 /* currentPage*/);
 
 		void writeCheckerProfiles(ScXmlStreamWriter& docu);
 		void writeLineStyles(ScXmlStreamWriter& docu);

Modified: trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format_save.cpp
URL: http://scribus.net/websvn/diff.php?repname=Scribus&rev=23616&path=/trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format_save.cpp
==============================================================================
--- trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format_save.cpp	(original)
+++ trunk/Scribus/scribus/plugins/fileloader/scribus150format/scribus150format_save.cpp	Thu Apr 16 12:22:52 2020
@@ -1792,16 +1792,13 @@
 		switch (master)
 		{
 			case ItemSelectionMaster:
-//				item = doc->MasterItems.at(j);
 				docu.writeStartElement("MASTEROBJECT");
 				break;
 			case ItemSelectionGroup:
 			case ItemSelectionPage:
-//				item = doc->DocItems.at(j);
 				docu.writeStartElement("PAGEOBJECT");
 				break;
 			case ItemSelectionFrame:
-//				item = doc->FrameItems.at(j);
 				docu.writeStartElement("FRAMEOBJECT");
 				break;
 			case ItemSelectionPattern:




More information about the scribus-commit mailing list