Bug #51086 for XML-LibXML: [patch] XML::LibXML::InputCallback object not reset to usable state after parsing

Tue Nov 03 14:55:54 2009 daniel.frett [...] ccci.org - Ticket created

Subject:

XML::LibXML::InputCallback object not reset to usable state after parsing

the state of the XML::LibXML::InputCallback object doesn't appear to be reset after finishing parsing. This means subsequent requests to parse xml data will not correctly trigger the input callbacks. Attached is a patch for the 28new_callbacks_multiple.t test that attempts to parse the input xml a second time which ends up failing because of the problem with XML::LibXML::InputCallback.

Subject:

icb.patch

Index: t/28new_callbacks_multiple.t =================================================================== --- t/28new_callbacks_multiple.t (revision 819) +++ t/28new_callbacks_multiple.t (working copy) @@ -60,9 +60,12 @@ $parser->expand_xinclude(1); $parser->input_callbacks($icb); my $doc = $parser->parse_string($string); + my $doc2 = $parser->parse_string($string); ok($doc); ok($doc->string_value(), "\ntest\nbar..\nbar..\n"); + ok($doc2); + ok($doc2->string_value(), "\ntest\nbar..\nbar..\n"); print $doc->serialize(); $icb->unregister_callbacks( [ \&match_hash2, \&open_hash,

Tue Nov 03 21:58:41 2009 daniel.frett [...] ccci.org - Correspondence added

From:

daniel.frett [...] ccci.org

On Tue Nov 03 14:55:54 2009, dfrett wrote: Show quoted text

> the state of the XML::LibXML::InputCallback object doesn't appear to be > reset after finishing parsing. This means subsequent requests to parse > xml data will not correctly trigger the input callbacks. > > Attached is a patch for the 28new_callbacks_multiple.t test that > attempts to parse the input xml a second time which ends up failing > because of the problem with XML::LibXML::InputCallback.

I tracked down the problem to the _cleanup_callbacks calling unregister_callbacks to unregister the callbacks set by the legacy callback methods. attached is a patch that uses the same test as in _init_callbacks to determine if unregister_callbacks actually needs to be called. And it also corrects the call to unregister_callbacks to send all 4 callbacks which is what unregister_callbacks is actually expecting. -Daniel Frett

Index: LibXML.pm =================================================================== --- LibXML.pm (revision 819) +++ LibXML.pm (working copy) @@ -738,8 +738,15 @@ sub _cleanup_callbacks { my $self = shift; $self->{XML_LIBXML_CALLBACK_STACK}->cleanup_callbacks(); + my $mcb = $self->match_callback(); - $self->{XML_LIBXML_CALLBACK_STACK}->unregister_callbacks( [$mcb] ); + my $ocb = $self->open_callback(); + my $rcb = $self->read_callback(); + my $ccb = $self->close_callback(); + + if ( defined $mcb and defined $ocb and defined $rcb and defined $ccb ) { + $self->{XML_LIBXML_CALLBACK_STACK}->unregister_callbacks( [$mcb, $ocb, $rcb, $ccb] ); + } } sub __read { Index: t/28new_callbacks_multiple.t =================================================================== --- t/28new_callbacks_multiple.t (revision 819) +++ t/28new_callbacks_multiple.t (working copy) @@ -1,6 +1,6 @@ # $Id$ use Test; -BEGIN { plan tests => 50 } +BEGIN { plan tests => 70 } END { ok(0) unless $loaded } use XML::LibXML; use IO::File; @@ -60,9 +60,12 @@ $parser->expand_xinclude(1); $parser->input_callbacks($icb); my $doc = $parser->parse_string($string); + my $doc2 = $parser->parse_string($string); ok($doc); ok($doc->string_value(), "\ntest\nbar..\nbar..\n"); + ok($doc2); + ok($doc2->string_value(), "\ntest\nbar..\nbar..\n"); print $doc->serialize(); $icb->unregister_callbacks( [ \&match_hash2, \&open_hash,

Wed Dec 16 13:42:56 2009 daniel.frett [...] ccci.org - Subject changed from 'XML::LibXML::InputCallback object not reset to usable state after parsing' to '[patch] XML::LibXML::InputCallback object not reset to usable state after parsing'

Mon Jun 27 06:12:34 2011 SHLOMIF [...] cpan.org - Correspondence added

This has already been fixed in the repository. I added the regression test just in case.

Mon Jun 27 06:12:36 2011 The RT System itself - Status changed from 'new' to 'open'

Mon Jun 27 06:12:36 2011 SHLOMIF [...] cpan.org - Status changed from 'open' to 'resolved'