I consider this bug critical, because it results in File::MMagic returning silently the wrong answer "at random" (ie non-deterministic)
The basic usage of File::MMagic->new() with no parameters is broken, as soon as the program instantiates a second (or third) File::MMagic object. This scenario is likely if more than one module uses File::MMagic, and caches the object for re-use.
Specifically, the code to lazily read from DATA is not robust. It basically can't be. The implementation shares the same DATA file handle between all created File::MMagic objects, but the data handle only has one seek position. Hence if one object is used to make a call to (say) checktype_filename(), and that call advances the data handle (but does not hit EOF) then a call to checktype_filename() made on a second object will not see the entire Magic database, and hence give wrong results.
Test case:
#!/usr/bin/perl -w
use strict;
use Test::More;
use File::MMagic;
my $imgfile = 'cpan.png';
my $textfile = $0;
my $fm1 = File::MMagic->new();
my $fm2 = File::MMagic->new();
# Third test fails:
is($fm1->checktype_filename($imgfile), 'image/png');
is($fm1->checktype_filename($textfile), 'text/plain');
is($fm2->checktype_filename($imgfile), 'image/png');
# but comment out the two uses of $fm1, and the remaing test starts passing.
__END__
output is
ok 1
ok 2
not ok 3
# Failed test at mmagic_caching_fail.t line 17.
# got: 'application/octet-stream'
# expected: 'image/png'
1..3
# Looks like you failed 1 test of 3.
I'm not sure what the best fix is. I'd be tempted to eliminate the lazy-reading from the case of ->new(), read <DATA> eagerly, cache the generated array in a file lexical, and then share it between all objects generated with default arguments to new().
Subject: | cpan.png |