Skip to content

Fix XML parser in std.xml throws TagException if the attr value is put in apostrophes #4064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cosmo-kramer wants to merge 1 commit into from
Closed

Fix XML parser in std.xml throws TagException if the attr value is put in apostrophes #4064

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions std/xml.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1059,9 +1059,10 @@ class Tag
munch(s,whitespace);
reqc(s,'=');
munch(s,whitespace);
reqc(s,'"');
string val = decode(munch(s,"^\""), DecodeMode.LOOSE);
reqc(s,'"');
char quote = requireOneOf(s,"'\"");
char[2] notQuote = ['^', quote];
string val = decode(munch(s,notQuote[]), DecodeMode.LOOSE);
reqc(s,quote);
munch(s,whitespace);
attr[key] = val;
}
Expand Down Expand Up @@ -2712,6 +2713,16 @@ EOS";
}
}

unittest
{
string test_xml = `<?xml version="1.0" encoding='UTF-8'?><stream:stream
xmlns:stream="http://etherx.'jabber'.org/streams"
xmlns="jabber:'client'" from='jid.pl' id="587a5767"
xml:lang="en" version="1.0"></stream:stream>`;

DocumentParser parser = new DocumentParser(test_xml);
Copy link
Member

@quickfur quickfur Mar 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also test that the parsed structure matches the expected structure? I.e., that the value of encoding is correctly parsed as UTF-8?

Copy link
Member

@quickfur quickfur Mar 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(And the xmlns in the stream:stream tag contains the entire value?)

Also, what about a tag like <p attr='a"b"c'/>? Does the parser handle that correctly?

Copy link
Contributor Author

@cosmo-kramer cosmo-kramer Mar 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @quickfur it handles cases with " inside properly.I tested it.

Copy link
Contributor

@aG0aep6G aG0aep6G Mar 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also test that the parsed structure matches the expected structure? I.e., that the value of encoding is correctly parsed as UTF-8?

Yeah, that should be tested. Unfortunately, std.xml doesn't seem to generate events for the root element, making this a bit more cumbersome than it should be.

@CoderAbhishek, if your motivation is still up for it, please add asserts for the values of the attributes. You can see how to do that in the unittest block that's below the one you added. Like there, you'll have to wrap things in a new root element. Also include a case like attr='a"b"c'.

Copy link
Member

@quickfur quickfur Mar 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, once the asserts are added, we can merge this. Thanks!

}

unittest
{
string s = q"EOS
Expand Down Expand Up @@ -2858,6 +2869,15 @@ private
s = s[1..$];
}

char requireOneOf(ref string s, string chars)
{
if (s.length == 0 || indexOf(chars,s[0]) == -1)
throw new TagException("");
char ch = s[0];
s = s[1..$];
return ch;
}

size_t hash(string s,size_t h=0) @trusted nothrow
{
return typeid(s).getHash(&s) + h;
Expand Down Expand Up @@ -2970,3 +2990,5 @@ private
throw new XMLException(s);
}
}


Copy link
Contributor

@aG0aep6G aG0aep6G Mar 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now there are three newlines at the end. One would be enough :)