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

Already on GitHub? Sign in to your account

On error, YAML::Tiny only cries 'Exporter' #30

Closed
hillu opened this Issue Jun 11, 2014 · 2 comments

Comments

Projects
None yet
2 participants
Contributor

hillu commented Jun 11, 2014

I get nonsensical error messages when trying to load malformed strings:

$ perl -Ilib -MYAML::Tiny -e 'Load("asdfsa")'
Exporter at -e line 1.

instead of the expected

YAML::Tiny failed to classify line 'asdfsa' at -e line 1.

This happens with perl 5.18.2 (Debian/unstable) and perl 5.14.2 (Debian/wheezy). git bisect points me at commit aaebed1. I have narrowed this down to the removal of use Data::Dumper: Re-adding the simple ´use` statement fixes the problem.

Contributor

hillu commented Jun 11, 2014

Further investigation shows that the implementation of the _error method is at fault:

sub _error {
    require Carp;
    $errstr = $_[1];
    $errstr =~ s/ at \S+ line \d+.*//;
    Carp::croak( $errstr );
}

If Carp hasn't been loaded, the require clobbers $@. An easy fix is therefore:

--- a/lib/YAML/Tiny.pm
+++ b/lib/YAML/Tiny.pm
@@ -776,8 +776,8 @@ our $errstr    = '';

 # Set error
 sub _error {
-    require Carp;
     $errstr = $_[1];
     $errstr =~ s/ at \S+ line \d+.*//;
+    require Carp;
     Carp::croak( $errstr );
 }

Had I used

$ perl -MCarp -MYAML::Tiny -e 'Load("asdfsa")'

I would not have noticed the bug. Data::Dumper uses Carp, so the bug was simply hidden before aaebed1.

Member

miyagawa commented Jun 11, 2014

The real bug is inside sub _load_string where it doesn't properly assign $@ to another variable (i.e. if (my $err = $@) ...), because as you say $@ is global and can be clobbered by loading a module etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment