[Expat-bugs] [ expat-Bugs-620343 ] segfault: bad API/callback interaction

[SourceForge.net]

Bugs item #620343, was opened at 2002-10-08 12:50
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110127&aid=620343&group_id=10127

Category: None
Group: None
Status: Open
Resolution: Fixed
Priority: 5
Submitted By: Fred L. Drake, Jr. (fdrake)
Assigned to: Fred L. Drake, Jr. (fdrake)
Summary: segfault: bad API/callback interaction

Initial Comment:
There is a bad interaction between the call to the
start element handler and the Expat API:  After the
presence of the callback is checked, the addBinding()
function can call XML_SetStartElementHandler(parser,
NULL), after which doContent() will still attempt to
call the start element handler.  This results in a call
to the NULL address, resulting in a memory fault.

This is unlikely to be an issue for applications
written entirely in C, but can reasonably happen when a
wrapper library clears callbacks when an error
condition or exception has been detected.

I've attached a patch and a regression test for this case.


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

>Comment By: Karl Waclawek (kwaclaw)
Date: 2003-01-21 11:59

Message:
Logged In: YES 
user_id=290026

I have attached a patch (ElmHandlers.diff) that calls
storeAtts whenever any of the attribute related handlers
is called, i.e. :
- startElmHandler, 
- startNamespaceDeclHandler, and
- endNamespaceDeclHandler.

The default handler calls are adjusted appropriately.

This patch should make it possible to have the 
namespace declaration handlers called even when 
no startElementHandlers is set.

Please review. Not tested yet.

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

Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2003-01-21 11:14

Message:
Logged In: YES 
user_id=3066

Added notes about the limitation in the documentation:
doc/reference.html 1.41

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

Comment By: Karl Waclawek (kwaclaw)
Date: 2003-01-21 10:55

Message:
Logged In: YES 
user_id=290026

I had another look and this looks trickier than I thought.
I suggest we add documentation that states that
the start/endNamespaceDeclHandlers will not be called
when the startElementHandler is not set.

I'll post more details in a follow-up.

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

Comment By: Karl Waclawek (kwaclaw)
Date: 2002-10-08 13:40

Message:
Logged In: YES 
user_id=290026

I am not *that* eager <g>.
You take care of it.

Thanks!

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

Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2002-10-08 13:36

Message:
Logged In: YES 
user_id=3066

Original patch checked in as lib/xmlparse.c 1.92 and
tests/runtests.c 1.36, before I saw Karl's comments.

Karl:  Yes, we should probably fix things so the namespace
decl handlers can be set independently, as the API suggests.
 I can take care of this, unless you beat me to it.  ;-)

Don't know that it warrants a separate issue report, so I'll
leave this one as fixed but open for now.

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

Comment By: Karl Waclawek (kwaclaw)
Date: 2002-10-08 13:21

Message:
Logged In: YES 
user_id=290026

Yes, verified. To be precise, addBinding() can call back
to startNamespaceDeclHandler() which then allows
the application to clear the startElementHandler.

One comment: storeAtts() is called like this

  if (startElementHandler) {
    result = storeAtts(parser, ...);
    ...
  }

presumably because the attributes should only be
stored if needed for the startElementHandler.
However, storeAtts does double duty by also
processing namespace declarations and calling
startNamespaceDeclHandler().

So, if that handler is set, should then not storeAtts()
be called like this:

  if (startElementHandler || startNamespaceDeclHandler)
  {
    result = storeAtts(parser, ...);
    ...
  }

And, if true, one should then apply this logic to the
other occasions when storeAtts() is called.

Or does it not make sense to set 
startNamespaceDeclHandler when
startElementHandler is cleared?



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

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