Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.85k stars 524 forks source link

File::Copy module #7559

Closed p5pRT closed 17 years ago

p5pRT commented 19 years ago

Migrated from rt.perl.org#32135 (status was 'resolved')

Searchable as RT32135$

p5pRT commented 19 years ago

From bhirt@mobygames.com

Created by root@loopy.tr.berkhirt.com

I found a problem in the File​::Copy module which is part of the core
perl distribution. I contacted the owner of the module and was advised to file a bug report with 'perlbug'.

If you copy from an IO​::Scalar to a filename the module croaks. It's because of this check​:

File/Copy.pm​:76 (version 2.07)

  if ($from eq $to) { # works for references\, too   croak("'$from' and '$to' are identical (not copied)");   }

The problem is triggered because IO​::Scalar is a tied string. You can do a little test and see how it crashes.

perl -e 'use IO​::Scalar; $ios = IO​::Scalar->new(\"hello"); print ($ios
eq "/tmp/somefilename" ? "yes" : "no")'

there are probably a few ways to fix this\, if you want my suggestion\, i would say that the check should only be done if from and to are both filenames. It's not really protecting you that much otherwise. For example this snipit won't croak that the files are identical\, even
though they are.

$fh = new IO​::File "/tmp/file"; File​::Copy​::copy($fh\,'/tmp/file');

Even the file check is basic at that;   File​::Copy​::copy('/tmp/../tmp/file'\,'/tmp/file') wont trigger the error either.

I'm happy to supply a patch if you want.

Perl Info ``` Flags: category=core severity=low Site configuration information for perl v5.8.3: Configured by root at Wed May 5 18:40:43 MDT 2004. Summary of my perl5 (revision 5.0 version 8 subversion 3) configuration: Platform: osname=linux, osvers=2.4.22-1.2188.nptl, archname=i686-linux uname='linux loopy.tr.berkhirt.com 2.4.22-1.2188.nptl #1 wed apr 21 20:36:05 edt 2004 i686 i686 i386 gnulinux ' config_args='-des -Dprefix=/usr/local' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm', optimize='-O3', cppflags='-fno-strict-aliasing -I/usr/local/include -I/usr/include/gdbm' ccversion='', gccversion='3.3.2 20031022 (Red Hat Linux 3.3.2-1)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=/lib/libc-2.3.2.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.3.2' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic' cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.3: /usr/local/lib/perl5/5.8.3/i686-linux /usr/local/lib/perl5/5.8.3 /usr/local/lib/perl5/site_perl/5.8.3/i686-linux /usr/local/lib/perl5/site_perl/5.8.3 /usr/local/lib/perl5/site_perl . Environment for perl v5.8.3: HOME=/root LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/kerberos/sbin:/usr/kerberos/bin:/usr/local/sbin:/usr/local/ bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/pg-7.4/bin:/usr/X11R6/bin:/root/ bin PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 17 years ago

From a.r.ferreira@gmail.com

On Mon Oct 25 11​:50​:07 2004\, bhirt@​mobygames.com wrote​:

This is a bug report for perl from root@​loopy.tr.berkhirt.com\, generated with the help of perlbug 1.34 running under perl v5.8.3.

----------------------------------------------------------------- [Please enter your report here]

I found a problem in the File​::Copy module which is part of the core perl distribution. I contacted the owner of the module and was advised to file a bug report with 'perlbug'.

If you copy from an IO​::Scalar to a filename the module croaks. It's because of this check​:

File/Copy.pm​:76 (version 2.07)

 if \($from eq $to\) \{ \# works for references\, too
    croak\("'$from' and '$to' are identical \(not copied\)"\);
 \}

The problem is triggered because IO​::Scalar is a tied string. You can do a little test and see how it crashes.

perl -e 'use IO​::Scalar; $ios = IO​::Scalar->new(\"hello"); print ($ios eq "/tmp/somefilename" ? "yes" : "no")'

there are probably a few ways to fix this\, if you want my suggestion\, i would say that the check should only be done if from and to are both filenames. It's not really protecting you that much otherwise. For example this snipit won't croak that the files are identical\, even though they are.

$fh = new IO​::File "/tmp/file"; File​::Copy​::copy($fh\,'/tmp/file');

Even the file check is basic at that; File​::Copy​::copy('/tmp/../tmp/file'\,'/tmp/file') wont trigger the error either.

I'm happy to supply a patch if you want.

Unlike the requestor thought\, the problem is not triggered because IO​::Scalar is a tied string\, but because IO​::Scalar is a package which provides an overloading for stringification. (IO​::Scalar does some tricky things with overloading and ties at the same time\, but the issue here is due to stringification.)

The problem can be demonstrated with​:

$ perl -e 'package foo; use overload q{""} => sub { "x" };   package main;   $foo = bless {}\, "foo";   print ($foo eq "a")' Operation "eq"​: no method found\,   left argument in overloaded package foo\,   right argument has no overloaded magic at -e line 2.

The problem with the offending code

File/Copy.pm​:76 (version 2.07)

 if \($from eq $to\) \{ \# works for references\, too
    croak\("'$from' and '$to' are identical \(not copied\)"\);
 \}

is that it tries to be too clever with a short expression to test equality both for filenames as well as references. And it works as long as the blessed references doesn't overload stringification\, when Perl tries to determine an overload for 'eq'.

The solution I found was to make the code dumber and clearer​:

Inline Patch ```diff diff -u -r perl-current/lib/File/Copy.pm perl-exp/lib/File/Copy.pm --- perl-current/lib/File/Copy.pm 2006-12-11 11:21:54.000000000 -0200 +++ perl-exp/lib/File/Copy.pm 2007-01-26 11:59:52.000000000 -0200 @@ -64,6 +64,14 @@ return File::Spec->catfile($to, basename($from)); } +# _eq($from, $to) tells whether $from and $to are identical +# works for strings and references +sub _eq { + return $_[0] == $_[1] if ref $_[0] && ref $_[1]; + return $_[0] eq $_[1] if !ref $_[0] && !ref $_[1]; + return ""; +} + sub copy { croak("Usage: copy(FROM, TO [, BUFFERSIZE]) ") unless(@_ == 2 || @_ == 3); @@ -82,7 +90,7 @@ || UNIVERSAL::isa($to, 'IO::Handle')) : (ref(\$to) eq 'GLOB')); - if ($from eq $to) { # works for references, too + if (_eq($from, $to)) { # works for references, too carp("'$from' and '$to' are identical (not copied)"); # The "copy" was a success as the source and destination contain # the same data. ```

[Please do not change anything below this line] ----------------------------------------------------------------- --- Flags​: category=core severity=low --- Site configuration information for perl v5.8.3​:

Configured by root at Wed May 5 18​:40​:43 MDT 2004.

Summary of my perl5 (revision 5.0 version 8 subversion 3) configuration​: Platform​: osname=linux\, osvers=2.4.22-1.2188.nptl\, archname=i686-linux uname='linux loopy.tr.berkhirt.com 2.4.22-1.2188.nptl #1 wed apr 21 20​:36​:05 edt 2004 i686 i686 i386 gnulinux ' config_args='-des -Dprefix=/usr/local' hint=recommended\, useposix=true\, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n\, bincompat5005=undef Compiler​: cc='cc'\, ccflags ='-fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm'\, optimize='-O3'\, cppflags='-fno-strict-aliasing -I/usr/local/include -I/usr/include/gdbm' ccversion=''\, gccversion='3.3.2 20031022 (Red Hat Linux 3.3.2-1)'\, gccosandvers='' intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234 d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=12 ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8 alignbytes=4\, prototype=define Linker and Libraries​: ld='cc'\, ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=/lib/libc-2.3.2.so\, so=so\, useshrplib=false\, libperl=libperl.a gnulibc_version='2.3.2' Dynamic Linking​: dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-rdynamic' cccdlflags='-fpic'\, lddlflags='-shared -L/usr/local/lib'

Locally applied patches​:

--- @​INC for perl v5.8.3​: /usr/local/lib/perl5/5.8.3/i686-linux /usr/local/lib/perl5/5.8.3 /usr/local/lib/perl5/site_perl/5.8.3/i686-linux /usr/local/lib/perl5/site_perl/5.8.3 /usr/local/lib/perl5/site_perl .

--- Environment for perl v5.8.3​: HOME=/root LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset)

PATH=/usr/kerberos/sbin​:/usr/kerberos/bin​:/usr/local/sbin​:/usr/local/ bin​:/sbin​:/bin​:/usr/sbin​:/usr/bin​:/usr/pg-7.4/bin​:/usr/X11R6/bin​:/root/ bin PERL_BADLANG (unset) SHELL=/bin/bash

p5pRT commented 17 years ago

From a.r.ferreira@gmail.com

file-copy.diff

p5pRT commented 17 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 17 years ago

@rgs - Status changed from 'open' to 'resolved'

p5pRT commented 17 years ago

From @rgarcia

On 26/01/07\, Adriano Ferreira \a\.r\.ferreira@​gmail\.com wrote​:

The problem with the offending code

File/Copy.pm​:76 (version 2.07)

 if \($from eq $to\) \{ \# works for references\, too
    croak\("'$from' and '$to' are identical \(not copied\)"\);
 \}

is that it tries to be too clever with a short expression to test equality both for filenames as well as references. And it works as long as the blessed references doesn't overload stringification\, when Perl tries to determine an overload for 'eq'.

The solution I found was to make the code dumber and clearer​:

diff -u -r perl-current/lib/File/Copy.pm perl-exp/lib/File/Copy.pm --- perl-current/lib/File/Copy.pm 2006-12-11 11​:21​:54.000000000 -0200 +++ perl-exp/lib/File/Copy.pm 2007-01-26 11​:59​:52.000000000 -0200

Thanks\, applied as #30013.

p5pRT commented 15 years ago

From @nwc10

On Fri\, Jan 26\, 2007 at 12​:56​:18PM -0200\, Adriano Ferreira wrote​:

Unlike the requestor thought\, the problem is not triggered because IO​::Scalar is a tied string\, but because IO​::Scalar is a package which provides an overloading for stringification. (IO​::Scalar does some tricky things with overloading and ties at the same time\, but the issue here is due to stringification.)

The problem can be demonstrated with​:

$ perl -e 'package foo; use overload q{""} => sub { "x" }; package main; $foo = bless {}\, "foo"; print ($foo eq "a")' Operation "eq"​: no method found\, left argument in overloaded package foo\, right argument has no overloaded magic at -e line 2.

The problem with the offending code

File/Copy.pm​:76 (version 2.07)

if \($from eq $to\) \{ \# works for references\, too
   croak\("'$from' and '$to' are identical \(not copied\)"\);
\}

is that it tries to be too clever with a short expression to test

No\, it's not being too clever. Anything here should be able to compare with eq without croaking.

equality both for filenames as well as references. And it works as long as the blessed references doesn't overload stringification\, when Perl tries to determine an overload for 'eq'.

That means that the reference is semantically broken. If it wants to play at being a string\, by providing stringification overloading\, then it needs to play fully at being a string\, by providing all the string overloading operators.

File​::Copy is documented as taking a file handle\, or a file name. The latter is a string. If it's being passed something that is a half-baked string\, then it's the fault of the half-baking\, not the string.

The more I look at this\, the more I think that we're piling hack on hack to work round a bug in IO​::Scalar\, and that the bug should be fixed *there*.

(Although arguably the IO​::Scalar bug would also be side-stepped by comparing files after feeding them to File​::Spec->canonpath())

Nicholas Clark