U++ forum: Welcome to the forum

Status & Roadmap

Authors & License

Funding Ultimate++

Search on this site

Search in forums

Home » U++ Library support » U++ Core » Bugfix: XmlParser in endless loop

Show: Today's Messages :: Show Polls :: Message Navigator
E-mail to friend

Switch to threaded view of this topic

Create a new topic

Submit Reply

Bugfix: XmlParser in endless loop [message #60191]

Mon, 02 October 2023 18:07

zsolt is currently offline

zsolt
Messages: 697
Registered: December 2005
Location: Budapest, Hungary

Contributor

Try this simple code:

ParseXML("</b>");

This function goes into an endless loop, because at the end of sReadXmlNode() function, the line

p.ReadText(); // skip empty text

doesn't do anything.

My first idea was to change it to

p.Skip();// skip empty text

It seems to be better, but the error message will not be too useful.

My proposed change:

@@ -1005,16 +1005,15 @@ static XmlNode sReadXmlNode(XmlParser& p, ParseXmlFilter *filter, dword style)
 		return m;
 	}
 	if(p.IsText()) {
 		m.CreateText(p.ReadText());
 		m.Shrink();
 		return m;
 	}
-	p.ReadText(); // skip empty text
-	return m;
+	throw XmlError("Unexpected text");
 }
 
 void ParseXmlFilter::EndTag() {}
 
 XmlNode ParseXML(XmlParser& p, dword style, ParseXmlFilter *filter)
 {
 	XmlNode r;

Report message to a moderator

Send a private message to this user

Re: Bugfix: XmlParser in endless loop [message #60195 is a reply to message #60191]

Tue, 03 October 2023 00:55

mirek is currently offline

mirek
Messages: 13975
Registered: November 2005

Ultimate Member

zsolt wrote on Mon, 02 October 2023 18:07

Try this simple code:
ParseXML("</b>");
This function goes into an endless loop, because at the end of sReadXmlNode() function, the line
p.ReadText(); // skip empty text
doesn't do anything.

My first idea was to change it to
p.Skip();// skip empty text
It seems to be better, but the error message will not be too useful.

My proposed change:
@@ -1005,16 +1005,15 @@ static XmlNode sReadXmlNode(XmlParser& p, ParseXmlFilter *filter, dword style)
 		return m;
 	}
 	if(p.IsText()) {
 		m.CreateText(p.ReadText());
 		m.Shrink();
 		return m;
 	}
-	p.ReadText(); // skip empty text
-	return m;
+	throw XmlError("Unexpected text");
 }
 
 void ParseXmlFilter::EndTag() {}
 
 XmlNode ParseXML(XmlParser& p, dword style, ParseXmlFilter *filter)
 {
 	XmlNode r;

I am not 100% sure about removing ReadText to skip empty text, I think there are cornercases that require that (I bet it is actually a fix).

But this definitely should work:

	if(p.ReadText().GetCount() == 0) // skip empty text
		throw XmlError("Unexpected text");

(in master now)

Report message to a moderator

Send a private message to this user

Re: Bugfix: XmlParser in endless loop [message #60196 is a reply to message #60195]

Wed, 04 October 2023 03:10

zsolt is currently offline

zsolt
Messages: 697
Registered: December 2005
Location: Budapest, Hungary

Contributor

Yes, it is a good idea. And seems to be working with my user provided test "XML" as well Smile

Smile

Thank you.

Report message to a moderator

Send a private message to this user

Re: Bugfix: XmlParser in endless loop [message #60204 is a reply to message #60196]

Mon, 09 October 2023 10:32

mirek is currently offline

mirek
Messages: 13975
Registered: November 2005

Ultimate Member

So that fix proved wrong, breaking autotests, so I have reverted and provided proper fix by moving the check / throw one level up:

static XmlNode sReadXmlNode(XmlParser& p, ParseXmlFilter *filter, dword style)
{
	XmlNode m;
	if(p.IsTag()) {
		String tag = p.ReadTag();
		if(!filter || filter->DoTag(tag)) {
			m.CreateTag(tag);
			m.SetAttrs(p.PickAttrs());
			while(!p.End())
				if(!Ignore(p, style)) {
					XmlNode n = sReadXmlNode(p, filter, style);
					if(n.GetType() != XML_DOC) // tag was ignored
						m.Add() = pick(n);
				}
			if(filter)
				filter->EndTag();
		}
		else
			p.SkipEnd();
		return m;
	}
	if(p.IsPI()) {
		m.CreatePI(p.ReadPI());
		return m;
	}
	if(p.IsDecl()) {
		m.CreateDecl(p.ReadDecl());
		return m;
	}
	if(p.IsComment()) {
		m.CreateComment(p.ReadComment());
		return m;
	}
	if(p.IsText()) {
		m.CreateText(p.ReadText());
		m.Shrink();
		return m;
	}
	p.ReadText(); // skip empty text
	return m;
}

void ParseXmlFilter::EndTag() {}

XmlNode ParseXML(XmlParser& p, dword style, ParseXmlFilter *filter)
{
	XmlNode r;
	while(!p.IsEof())
		if(!Ignore(p, style)) {
			XmlNode n = sReadXmlNode(p, filter, style);
			if(n.GetType() != XML_DOC) // tag was ignored
				r.Add() = pick(n);
			else {
				if(p.IsRelaxed())
					p.Skip();
				else
					throw XmlError("Unexpected text");
			}
		}
	return r;
}

Report message to a moderator

Send a private message to this user

Re: Bugfix: XmlParser in endless loop [message #60207 is a reply to message #60204]

Thu, 12 October 2023 05:15

zsolt is currently offline

zsolt
Messages: 697
Registered: December 2005
Location: Budapest, Hungary

Contributor

Thank you.
Working well.

Report message to a moderator

Send a private message to this user

Switch to threaded view of this topic

Create a new topic

Submit Reply

Previous Topic:	XmlParser encoding detection bugfix.
Next Topic:	pitfall with storing integers in a stream

Goto Forum:

-=] Back to Top [=-

[ Syndicate this forum (XML) ] [

] [

PDF

]

Current Time: Sun Apr 28 21:08:44 CEST 2024

Total time taken to generate the page: 0.02677 seconds