[Fwd: [Mono-devel-list] Re: System.Xml patch]
Hello Eno.
Can you please look and give your opinion on this patch?
Andrew.
Original Message
Subject: [Mono-devel-list] Re: System.Xml patch
Date: Wed, 22 Jun 2005 12:48:06 +0300
From: Andrew Skiba [EMAIL PROTECTED]
To: Atsushi Eno [EMAIL PROTECTED]
CC: mono-devel mailing list mono-devel-list@lists.ximian.com
References: [EMAIL PROTECTED] [EMAIL PROTECTED]
Atsushi Eno wrote:
Hi Andrew,
Good catch :-) However the fix does not look correct. Your patch
also avoids entity content evaluation, that means it sometimes
results in incorrect well-formedness.
Good catch, too ;-)
How about this patch? To save iterations, I made the same check in
ResolveExternalEntityReplacementText, please tell me, if it's unnecessary.
Cheers,
Andrew Skiba.
Index: DTDReader.cs
===
--- DTDReader.cs(revision 46238)
+++ DTDReader.cs(working copy)
@@ -641,8 +641,9 @@
private void ResolveExternalEntityReplacementText
(DTDEntityBase decl)
{
if (decl.LiteralEntityValue.StartsWith (?xml)) {
+ // FIXME: not always it should be read in
Element context - AS
XmlTextReader xtr = new XmlTextReader
(decl.LiteralEntityValue, XmlNodeType.Element, null);
- if (decl is DTDEntityDeclaration) {
+ if (decl is DTDEntityDeclaration
DTD.EntityDecls [decl.Name] == null) {
// GE - also checked as valid contents
StringBuilder sb = new StringBuilder ();
xtr.Normalization = this.Normalization;
@@ -710,8 +711,9 @@
}
decl.ReplacementText = CreateValueString ();
- if (decl is DTDEntityDeclaration) {
+ if (decl is DTDEntityDeclaration DTD.EntityDecls
[decl.Name] == null) {
// GE - also checked as valid contents
+ // FIXME: not always it should be read in
Element context - AS
XmlTextReader xtr = new XmlTextReader
(decl.ReplacementText, XmlNodeType.Element, null);
StringBuilder sb = new StringBuilder ();
xtr.Normalization = this.Normalization;
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Fwd: [Mono-devel-list] Re: System.Xml patch]
Hi Andrew, Please go ahead as I wrote before ;-) http://www.mail-archive.com/mono-devel-list@lists.ximian.com/msg02075.html Andrew Skiba wrote: Hello Eno. Can you please look and give your opinion on this patch? Andrew. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
[Mono-devel-list] Re: System.Xml patch
Atsushi Eno wrote:
Hi Andrew,
Good catch :-) However the fix does not look correct. Your patch
also avoids entity content evaluation, that means it sometimes
results in incorrect well-formedness.
Good catch, too ;-)
How about this patch? To save iterations, I made the same check in
ResolveExternalEntityReplacementText, please tell me, if it's unnecessary.
Cheers,
Andrew Skiba.
Index: DTDReader.cs
===
--- DTDReader.cs(revision 46238)
+++ DTDReader.cs(working copy)
@@ -641,8 +641,9 @@
private void ResolveExternalEntityReplacementText
(DTDEntityBase decl)
{
if (decl.LiteralEntityValue.StartsWith (?xml)) {
+ // FIXME: not always it should be read in
Element context - AS
XmlTextReader xtr = new XmlTextReader
(decl.LiteralEntityValue, XmlNodeType.Element, null);
- if (decl is DTDEntityDeclaration) {
+ if (decl is DTDEntityDeclaration
DTD.EntityDecls [decl.Name] == null) {
// GE - also checked as valid contents
StringBuilder sb = new StringBuilder ();
xtr.Normalization = this.Normalization;
@@ -710,8 +711,9 @@
}
decl.ReplacementText = CreateValueString ();
- if (decl is DTDEntityDeclaration) {
+ if (decl is DTDEntityDeclaration DTD.EntityDecls
[decl.Name] == null) {
// GE - also checked as valid contents
+ // FIXME: not always it should be read in
Element context - AS
XmlTextReader xtr = new XmlTextReader
(decl.ReplacementText, XmlNodeType.Element, null);
StringBuilder sb = new StringBuilder ();
xtr.Normalization = this.Normalization;
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
[Mono-devel-list] Re: System.Xml patch
Hi Andrew,
How about this patch? To save iterations, I made the same check in
ResolveExternalEntityReplacementText, please tell me, if it's unnecessary.
This patch looks fine. Let's check it in svn.
I'll apply the patch attached after your commit.
Index: DTDReader.cs
===
--- DTDReader.cs(revision 46238)
+++ DTDReader.cs(working copy)
@@ -641,8 +641,9 @@
private void ResolveExternalEntityReplacementText
(DTDEntityBase decl)
{
if (decl.LiteralEntityValue.StartsWith (?xml)) {
+ // FIXME: not always it should be read in
Element context - AS
What does this mean? I guess you have in mind that there are cases that
entities could be used inside attributes, but attribute content check
is less prohibiting and '' characters are checked anyways.
(BTW we won't understand what -AS means there ;-)
Besides the comment itself.
In fact I also don't like that part of code. Actually, though
replacing entities with a simple string is good for performance
as compared to such code that loads entity possibly from external
files every time, it causes incorrect BaseURI resolution (that
results in incorrect not-wf error for XHTML 1.1 DTD; bug #51495).
So, basically rewriting that entity expansion part is the best
solution. But I haven't tried that since it will not be done as a
quick hack and it will first result in several breakage in
standalone tests.
Thanks,
Atsushi Eno
Index: Test/System.Xml/XmlTextReaderTests.cs
===
--- Test/System.Xml/XmlTextReaderTests.cs (revision 46370)
+++ Test/System.Xml/XmlTextReaderTests.cs (working copy)
@@ -1100,5 +1100,20 @@
AssertEquals (#1, 0xf090, (int) r.Value [0]);
AssertEquals (#1, 0x8080, (int) r.Value [1]);
}
+
+ [Test]
+ [ExpectedException (typeof (XmlException))]
+ public void EntityDeclarationNotWF ()
+ {
+ string xml = @!DOCTYPE doc [
+ !ELEMENT doc (#PCDATA)
+ !ENTITY e ''
+ !ENTITY e 'foo'
+ ]
+ doce;/doc ;
+ XmlTextReader xtr = new XmlTextReader (xml,
+ XmlNodeType.Document, null);
+ xtr.Read ();
+ }
}
}
___
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
