[ expat-Bugs-896188 ] Not well-formed internal parameter entities

Bugs item #896188, was opened at 2004-02-12 19:11
Message generated for change (Comment added) made by fdrake
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110127&aid=896188&group_id=10127

Category: None
Group: Test Required
>Status: Closed
Resolution: Fixed
Priority: 5
Submitted By: Jeremy Kloth (jkloth)
Assigned to: Fred L. Drake, Jr. (fdrake)
Summary: Not well-formed internal parameter entities

Initial Comment:
In testing against the XML Conformance Test Suites, we've 
discovered that Expat doesn't verify that internal parameter 
entities are well-formed (as per XML 1.0 3rd 4.3.2). 
 
Attached is a patch that fixes this and 2 example documents that 
demonstrate both valid and invalid behavior. 
 

----------------------------------------------------------------------

>Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2004-07-15 20:45

Message:
Logged In: YES 
user_id=3066

Testing with the OASIS test suite is sufficient.  Closing
this report.

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2004-02-13 13:21

Message:
Logged In: YES 
user_id=290026

I applied a simplified version of Jeremy's second patch.
Tested with the OASIS test-suite as well as
with a read buffer size of 1, to stress test the
buffer boundary handling, since we changed some
code involving XML_TOK_NONE.

This means that we now pass the two test cases
of the test suite, xmltest/not-wf/not-sa/010.xml and
xmltest/not-wf/not-sa/011.xml, that we previously failed.

Check xmlparse.c rev. 1.123 and xmlrole.c rev. 1.16.

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2004-02-13 01:14

Message:
Logged In: YES 
user_id=290026

This new patch seems to work with the test-suite.

Will have a closer look tomorrow.
(There was a warning about integral size mismatch).


----------------------------------------------------------------------

Comment By: Jeremy Kloth (jkloth)
Date: 2004-02-13 00:34

Message:
Logged In: YES 
user_id=38980

OK.  After going through the spec again, we came to the same 
conclusion (WFC: PE Between Declarations).  Adding a new patch which 
uses this for its check as opposed to just checking if it is a parameter 
entity. 

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2004-02-13 00:01

Message:
Logged In: YES 
user_id=290026

One more comment:

It seems that Well-formedness constraint: PE Between 
Declarations applies here rather than section 4.3.2.

I guess the solution might involve modifying the
processInternalEntity() function in such a way, that
the state before and after calling doProlog() is
checked for two things:
1) is this a PE reference between declarations?
2) if yes, how does it return - any incomplete declarations?

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2004-02-12 22:39

Message:
Logged In: YES 
user_id=290026

Well, the patch seems to fix the two test cases mentioned
below, but when I ran Expat through the whole test suite 
again, it turned out that it now fails on 32 test cases more
than before. 

One example: test case xmlconf/sun/valid/optional.xml
uses the dtdtest.dtd in the same directory. 
It is well-formed and valid, but Expat now reports a syntax 
error on line 19, column 26 of the dtd.

Seems this patch should be examined more closely.
But I wouldn't give up yet.

----------------------------------------------------------------------

Comment By: Jeremy Kloth (jkloth)
Date: 2004-02-12 21:28

Message:
Logged In: YES 
user_id=38980

Attached is the "Fix Explained" (hopefully) document. 

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2004-02-12 21:02

Message:
Logged In: YES 
user_id=290026

OK, now I remember. If you look at bug #569461 there
was a discussion regarding these two test cases,
(a while back) and there I thought it would require some
time to understand the logic in Expat well enough for a fix.

This is cool - having a fix for that! Thanks!

I would like you to explain the logic behind the fix,
because I really don't want to "fix" Expat so short before
a new release unless I understand the fix and can reasonably
be sure it doesn't mess up anything else.

You might want to create a "Fix Explained" attachment
rather than a note, as this web interface really isn't
a good text editor.

----------------------------------------------------------------------

Comment By: Jeremy Kloth (jkloth)
Date: 2004-02-12 20:52

Message:
Logged In: YES 
user_id=38980

Actually, the error should be on line 4 (the %e; expansion).  I'm using a 
CVS (updated today) build. 
 
I've attached another invalid example to further demonstrate the 
problem. 
 
The XML Test Suite tests for this case are 
xmltest/not-wf/not-sa/010.xml and 011.xml 

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2004-02-12 20:13

Message:
Logged In: YES 
user_id=290026

I tried the current Expat (CVS), and it reports a syntax
error on line 5, column 0 for bad.xml. Is that not what it 
should do? Are you saying that your copy of Expat
doesn't do that? Which version are you using?

Which test cases of the XML conformance suite
does Expat not pass - related to this issue?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110127&aid=896188&group_id=10127