3 ms·

At first glance, I didn't see anything wrong with the code shown (aside from using << instead of >> for append). I must be one of those ugly old Perl hackers;

by drv 16y ago

At first glance, I didn't see anything wrong with the code shown (aside from using << instead of >> for append). I must be one of those ugly old Perl hackers; I probably have an old edition of the camel book somewhere. Constructively, though, what is wrong with the code snippet? The improvements he lists don't improve this case at all. Three-argument open vs two here is debatable (I doubt $runpath is insecure input), but fancy filehandle-in-a-variable doesn't gain anything when it's just in use over three lines.

jerf 16y ago
The real danger is in the habit, not this specific code snippet. Two-parameter open includes the ability to open to and from arbitrary shell commands. I can and have used this to escalate to root out of "nobody" with the combination of a bad two-parameter open and an excessively permissive sudoers file. It's better to just say never use two-parameter open; there's no reason to do it.
lysium 16y ago
Don't know why you're downvoted. I also missed his point why his alternative solution is in any way "better".
- phaylon 16y ago
  Code like this: open my $fh, ">$file" or die "..."; will append to target if $file contains ">target". Code like this: open my $fh, '>', $file or die "..."; explicitly tells Perl what the mode and what the filename is, so there can't be any confusion. Using lexicals instead of typeglobs means you store the filehandle in a lexical variable instead of making it globally accessible in your package.
  - Xurinos 16y ago
    And ultimately, wouldn't this be even more "modern", even clearer? -- my $file_path = Path::Class::dir("some_path")->file("file_name"); my $fh = $file_path->openw(); $fh->print("my line\n"); $fh->close(); Or for reading the whole file quickly, instead of the @var = <FILE> idiom -- my @var = $file_path->slurp();
    - phaylon 16y ago
      It can be, of course. There is more than one way to do it, because there's more than one situation in which one wants to use it. Most of my larger projects use Path::Class, but none of my scripts in ~phaylon/bin do, since I want to use some of them even if I don't have a perl setup yet. Another reason might be that you're writing a CPAN module and don't do much with files. If you just use it to write a debug log when an environment variable is set, you might not want to include another dependency. What your code demonstrates very good in my opinion is the fact that lexical filehandles are just more interoperable. You can pass them around transparently like any other reference, use modules that pass them around, and so on.
phaylon 16y ago
$runpath might be safe now, but it doesn't have to be forever. It could come from a config file one day. I'm not sure why using a lexical variable to store a filehandle you only need lexically is more fancy than storing it in a globally accessable package typeglob for anyone to use and modify. But it does give you the advantage that your garbage collector will close your file handles instead of you having to do it manually. If I see close($fh) in my own code, I know I had a reason other than "I didn't need it anymore."

by drv 16y ago

jerf 16y ago

The real danger is in the habit, not this specific code snippet. Two-parameter open includes the ability to open to and from arbitrary shell commands. I can and have used this to escalate to root out of "nobody" with the combination of a bad two-parameter open and an excessively permissive sudoers file. It's better to just say never use two-parameter open; there's no reason to do it.

lysium 16y ago

Don't know why you're downvoted. I also missed his point why his alternative solution is in any way "better".

phaylon 16y ago
Code like this: open my $fh, ">$file" or die "..."; will append to target if $file contains ">target". Code like this: open my $fh, '>', $file or die "..."; explicitly tells Perl what the mode and what the filename is, so there can't be any confusion. Using lexicals instead of typeglobs means you store the filehandle in a lexical variable instead of making it globally accessible in your package.
- Xurinos 16y ago
  And ultimately, wouldn't this be even more "modern", even clearer? -- my $file_path = Path::Class::dir("some_path")->file("file_name"); my $fh = $file_path->openw(); $fh->print("my line\n"); $fh->close(); Or for reading the whole file quickly, instead of the @var = <FILE> idiom -- my @var = $file_path->slurp();
  - phaylon 16y ago
    It can be, of course. There is more than one way to do it, because there's more than one situation in which one wants to use it. Most of my larger projects use Path::Class, but none of my scripts in ~phaylon/bin do, since I want to use some of them even if I don't have a perl setup yet. Another reason might be that you're writing a CPAN module and don't do much with files. If you just use it to write a debug log when an environment variable is set, you might not want to include another dependency. What your code demonstrates very good in my opinion is the fact that lexical filehandles are just more interoperable. You can pass them around transparently like any other reference, use modules that pass them around, and so on.

phaylon 16y ago

$runpath might be safe now, but it doesn't have to be forever. It could come from a config file one day. I'm not sure why using a lexical variable to store a filehandle you only need lexically is more fancy than storing it in a globally accessable package typeglob for anyone to use and modify. But it does give you the advantage that your garbage collector will close your file handles instead of you having to do it manually. If I see close($fh) in my own code, I know I had a reason other than "I didn't need it anymore."