[perl #109146] 3-arg open of undef for read

[Yitzchak Scott-Thoennes]

# New Ticket Created by Yitzchak Scott-Thoennes
# Please include the string: [perl #109146]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt.perl.org:443/rt3/Ticket/Display.html?id=109146 >


This is a bug report for perl from stho...@gmail.com,
generated with the help of perlbug 1.39 running under perl 5.14.2.


-----------------------------------------------------------------
[Please describe your issue here]

open(FH, "<", undef) triggers special behavior to open a temporary file
(at least with perlio) that is documented as only applying to +> and +<.

To keep open(FH, "<", $hash{typoedkey}) from quietly succeeding, this
should at least trigger a warning (and the doc be updated to not be mode
specific).

Crude patch from avar in irc:

diff --git a/perlio.c b/perlio.c
index a985dcc..09e415d 100644
--- a/perlio.c
+++ b/perlio.c
@@ -1601,6 +1601,8 @@ PerlIO_openn(pTHX_ const char *layers, const
char *mode, int fd,
{
dVAR;
if (!f && narg == 1 && *args == &PL_sv_undef) {
+ if (mode && strEQ(mode, "r"))
+ Perl_croak(aTHX_ "You have an undef filename with a < mode");
if ((f = PerlIO_tmpfile())) {
if (!layers || !*layers)
layers = Perl_PerlIO_context_layers(aTHX_ mode);

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

Configured by ysth at Mon Dec 26 13:23:55 PST 2011.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration:

Platform:
osname=linux, osvers=2.6.32-37-generic, archname=i686-linux
uname='linux lark.servola.org 2.6.32-37-generic #81-ubuntu smp fri
dec 2 20:35:14 utc 2011 i686 gnulinux '
config_args='-de -Dprefix=/home/ysth/perl5/perlbrew/perls/perl-5.14.2'
hint=recommended, useposix=true, d_sigaction=define
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 -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2',
cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
ccversion='', gccversion='4.4.3', 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 =' -fstack-protector -L/usr/local/lib'
libpth=/usr/local/lib /lib/../lib /usr/lib/../lib /lib /usr/lib
/usr/lib/i486-linux-gnu /usr/lib64
libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc
perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
libc=/lib/libc-2.11.1.so, so=so, useshrplib=false, libperl=libperl.a
gnulibc_version='2.11.1'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib
-fstack-protector'

Locally applied patches:


---
@INC for perl 5.14.2:
/usr/lib/shiftboard
/home/ysth/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/i686-linux
/home/ysth/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2
/home/ysth/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2/i686-linux
/home/ysth/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2
.

---
Environment for perl 5.14.2:
HOME=/home/ysth
LANG=en_US.utf8
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home/ysth/perl5/perlbrew/bin:/home/ysth/perl5/perlbrew/perls/current/bin:/home/ysth/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
PERL5LIB=/usr/lib/shiftboard
PERLBREW_PATH=/home/ysth/perl5/perlbrew/bin:/home/ysth/perl5/perlbrew/perls/current/bin
PERLBREW_PERL=perl-5.14.2
PERLBREW_ROOT=/home/ysth/perl5/perlbrew
PERLBREW_VERSION=0.18
PERL_BADLANG (unset)
SHELL=/bin/bash

[Ævar Arnfjörð Bjarmason]

On Fri, Jan 27, 2012 at 02:25, Yitzchak Scott-Thoennes
<perlbug-...@perl.org> wrote:

> open(FH, "<", undef) triggers special behavior to open a temporary file
> (at least with perlio) that is documented as only applying to +> and +<.
>
> To keep open(FH, "<", $hash{typoedkey}) from quietly succeeding, this
> should at least trigger a warning (and the doc be updated to not be mode
> specific).
>
> Crude patch from avar in irc:
>
> diff --git a/perlio.c b/perlio.c
> index a985dcc..09e415d 100644
> --- a/perlio.c
> +++ b/perlio.c
> @@ -1601,6 +1601,8 @@ PerlIO_openn(pTHX_ const char *layers, const
> char *mode, int fd,
>  {
>     dVAR;
>     if (!f && narg == 1 && *args == &PL_sv_undef) {
> +       if (mode && strEQ(mode, "r"))
> +           Perl_croak(aTHX_ "You have an undef filename with a < mode");
>        if ((f = PerlIO_tmpfile())) {
>            if (!layers || !*layers)
>                layers = Perl_PerlIO_context_layers(aTHX_ mode);
>

Two points:

* That patch is probably in completely the wrong place, it should
likely be in Perl_pp_open or Perl_do_openn.

* With that patch all of perl's tests pass, which means that we have
no tests for this feature.

And also:

* Did this just slip between the cracks when in-memory file support
was added (i.e. open my $fh, ">", \(my $file))? Shouldn't we always
be dying when passed undef with modes that aren't +> or <+ ? From
perldoc -f open:

As a special case the three-argument form with a
read/write mode and the third argument being "undef":

open(my $tmp, "+>", undef) or die ...

opens a filehandle to an anonymous temporary file.
Also using "+<" works for symmetry, but you really
should consider writing something to the temporary file
first. You will need to seek() to do the reading.

Is this used for anything else?

* What do we actually get on `open(FH, "<", undef)`? It seems to me
that we create some skeleton filehandle that we can't actually do
anything with.

[Nicholas Clark]

On Thu, Jan 26, 2012 at 05:25:56PM -0800, Yitzchak Scott-Thoennes wrote:

> open(FH, "<", undef) triggers special behavior to open a temporary file
> (at least with perlio) that is documented as only applying to +> and +<.
>
> To keep open(FH, "<", $hash{typoedkey}) from quietly succeeding, this
> should at least trigger a warning (and the doc be updated to not be mode
> specific).

I think that a comprehensive fix should be possible. Note that XS can tell
the difference:

$ ./perl -Ilib -MDevel::Peek -e 'Dump undef'
SV = NULL(0x0) at 0x1003aa990
REFCNT = 2147483621
FLAGS = (READONLY)
$ ./perl -Ilib -MDevel::Peek -e 'Dump $a'
SV = NULL(0x0) at 0x100812b90
REFCNT = 1
FLAGS = ()
$ ./perl -Ilib -MDevel::Peek -e 'Dump $hash{typoedkey}'
SV = PVLV(0x100818a00) at 0x100802ed0
REFCNT = 1
FLAGS = (TEMP,GMG,SMG)
IV = 0
NV = 0
PV = 0
MAGIC = 0x100509040
MG_VIRTUAL = &PL_vtbl_defelem
MG_TYPE = PERL_MAGIC_defelem(y)
MG_FLAGS = 0x02
REFCOUNTED
MG_OBJ = 0x100803110
SV = PV(0x100801070) at 0x100803110
REFCNT = 1
FLAGS = (POK,pPOK)
PV = 0x100501b50 "typoedkey"\0
CUR = 9
LEN = 16
TYPE = y
TARGOFF = 0
TARGLEN = 1
TARG = 0x100812ae8
FLAGS = 0
SV = PVHV(0x100807ea0) at 0x100812ae8
REFCNT = 2
FLAGS = (SHAREKEYS)
ARRAY = 0x0
KEYS = 0
FILL = 0
MAX = 7
RITER = -1
EITER = 0x0

Quite a big difference.

And that the case of a simple undefined variable is already spotted:

$ ./perl -e 'open $fh, "<", $a or die'
Died at -e line 1.
$ ./perl -e 'open $fh, "<", $hash{typoedkey} or die'


So I think that there's something more subtly amis here, probably due to
how and when the magic is executed for hash keys passed to subroutines.

Nicholas Clark

[Zefram]

Nicholas Clark wrote:
>I think that a comprehensive fix should be possible. Note that XS can tell
>the difference:

Better to distinguish at compile time. Literal "undef" is a different
opcode from "$undefined" or "$hash{typoed_key}".

-zefram

[Nicholas Clark]

On Fri, Jan 27, 2012 at 11:02:38AM +0000, Nicholas Clark wrote:
> On Thu, Jan 26, 2012 at 05:25:56PM -0800, Yitzchak Scott-Thoennes wrote:
>
> > open(FH, "<", undef) triggers special behavior to open a temporary file
> > (at least with perlio) that is documented as only applying to +> and +<.
> >
> > To keep open(FH, "<", $hash{typoedkey}) from quietly succeeding, this
> > should at least trigger a warning (and the doc be updated to not be mode
> > specific).
>
> I think that a comprehensive fix should be possible. Note that XS can tell
> the difference:

Bother. I suspect I'm not correct here. XS can tell the difference because
there is a whole bunch of games when making the subroutine call. OPs don't
do this.

Nicholas Clark

[Father Chrysostomos via RT]

Please don’t add more special-cased parsing to open(). It’s complex
enough already. Also, your proposal is not backward-compatible.

--

Father Chrysostomos


---
via perlbug: queue: perl5 status: open
https://rt.perl.org:443/rt3/Ticket/Display.html?id=109146

[Yitzchak Scott-Thoennes]

On Fri, Jan 27, 2012 10:58:00 +0100, Ævar Arnfjörð Bjarmason wrote:
> What do we actually get on `open(FH, "<", undef)`? It seems to me
> that we create some skeleton filehandle that we can't actually do
> anything with.

It does open a temporary file, the same as the read-write modes would
have done. The file is opened read/write, but I believe the perl handle
is marked as read only.

[Aristotle Pagaltzis]

* Zefram <zef...@fysh.org> [2012-01-27 12:35]:

Does that mean that a wrapper for `open` would have to do this in order
to provide an undef op?

defined $name ? $name : undef

Ugh.

Or would even that not work because that’s an expression more complex
than a plain `undef`? Ugh.

Please, no.

--
*AUTOLOAD=*_;sub _{s/$/$"/;s/(.*):://;wantarray//substr$_,-1,1,",$/";print;$1}
&Just->another->Perl->hack;
#Aristotle Pagaltzis // <http://plasmasturm.org/>

[David Nicol]

opening $hash{wrong} makes sense when you don't want to have to
compulsively errorcheck every little thing multiple times, maybe the
resulting useless filehandle could fail when used, instead of silently
loading a buffer there's no way to read back from?