in reply to Re^4: Renaming an image file
in thread Renaming an image file

This line is confusing to me, my $md5 = md5_hex( do{ local( @ARGV, $/) = $file; <> } );

This bit: do{ local( @ARGV, $/) = $file; <> } reads the contents of the file whose name is $file.

By all means spread that out across multiple lines if you prefer; Or perhaps use File::Slurp.

I get "use of uninitialized value $_ in substitution (s///)

It is very hard to see how that could happen, given that $_ isn't referenced anywhere in the line that has the substitution operator?

why replacing the number with the md5 of that number is a poor idea?

Let's say you have file joe_1.jpg and you rename it to joe_c4ca4238a0b923820dcc509a6f75849b,jpg, then next week joe uploads a new photo and calls it joe_1.jpg. Different photo, but same name. Overwrite.

People tend to be very lazy and will likely stick to numbers if the low thousands, the potential for clashes is huge.

But even if all the images are tiny, say just 10k bytes, that means there are 2.3836626600959079364564032010142e+24660 possibilities.

Even with MD5 reducing that to 3.4028236692093846346337460743177e+38 the chances of collision are minuscule.

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.
RIP an inspiration; A true Folk's Guy

Replies are listed 'Best First'.
Re^6: Renaming an image file
by Anonymous Monk on Nov 28, 2010 at 18:02 UTC
    At this point I already read the file in, is this looks ok based in what you have here? 
      for(my $i=0;$i<4;$i++) {
  
     my $file = $q->param('pic_'.$i);
     my @full = split(/(\\)|(\/)/,$file);
     my $filename = pop(@full);
        $filename =~ s/\n//;
        $filename =~ s/\r//;
        $filename = $username_pic."_".$filename;
        #$filename=~s/^([^_]+_)(.*)(\.\w{3})\z/$1.md5_hex($2).$3/eg;
        my $md5 = md5_hex( $filename );
        $filename =s[_(\d+).jpg$][_$md5.jpg];
}

    [download]
[reply]
[d/l]

      Sorry, I don't know much about cgi, but it still looks to me that you are md5'ing the filename and not the contents of the file.

[reply]
        OK I will add another string of numbers right after the MD5 if this file already exists: 
                if ( -e "$path/$filename" ) { 
           my $unique = unique();
           $filename=~s/^([^_]+_)(.*)(\.\w{3})\z/$1.$2.$unique.$3/eg;
           rename( "$path/$filename", "$path/$filename");
           unlink($filename);
         }
}
sub unique {
my ($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $isdst) = loc
+altime(time);
$mon++;
$year = $year + 1900;
my $timestamp = sprintf "%4d%02d%02d%02d%02d%02d", $year,$mon,$mday,$h
+our,$min,$sec;
my $rand = (rand 10000);
$rand = int($rand);
my $unique = "$timestamp$rand";
}

        [download]
[reply]
[d/l]
      This is wrong: $filename =s[_(\d+).jpg$][_$md5.jpg];

      You mean =~, not =. That fixes the error mentioning $_ (s/// works on $_ by default).

      Also, the Perlish way to loop over an interval like that is for my $i (0..3) {.

[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom