3-arg open of undef for read #11910

p5pRT · 2012-01-27T01:25:56Z

Migrated from rt.perl.org#109146 (status was 'open')

Searchable as RT109146$

p5pRT · 2012-01-27T01:25:56Z

From @ysth

Created by @ysth

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:

Inline Patch

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$;

Perl Info


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

p5pRT · 2012-01-27T09:58:50Z

From @avar

On Fri, Jan 27, 2012 at 02:25, Yitzchak Scott-Thoennes
<perlbug-followup@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.

p5pRT · 2012-01-27T09:58:51Z

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

p5pRT · 2012-01-27T11:03:12Z

From @nwc10

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

p5pRT · 2012-01-27T11:31:29Z

From zefram@fysh.org

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

p5pRT · 2012-01-27T11:53:01Z

From @nwc10

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

p5pRT · 2012-01-27T20:46:42Z

From @cpansprout

On Fri Jan 27 03:31:29 2012, zefram@fysh.org wrote:

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}".

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

--

Father Chrysostomos

p5pRT · 2012-01-27T21:04:41Z

From @ysth

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.

p5pRT · 2012-01-28T05:52:05Z

From @ap

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

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}".

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/>

p5pRT · 2013-07-10T02:06:04Z

From @rjbs

* Ævar Arnfjörð Bjarmason <avarab@gmail.com> [2012-01-27T04:58:00]

* 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:

That's what I would think.

--
rjbs

p5pRT · 2013-07-11T20:23:57Z

From @davidnicol

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?

p5pRT added Severity Low distro-Linux type-core labels Oct 19, 2019

xenu removed the affects-5.14 label Nov 19, 2021

xenu removed the Severity Low label Dec 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3-arg open of undef for read #11910

3-arg open of undef for read #11910

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

p5pRT commented Jan 28, 2012

p5pRT commented Jul 10, 2013

p5pRT commented Jul 11, 2013

3-arg open of undef for read #11910

3-arg open of undef for read #11910

Comments

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

From @ysth

Created by @ysth

p5pRT commented Jan 27, 2012

From @avar

p5pRT commented Jan 27, 2012

p5pRT commented Jan 27, 2012

From @nwc10

p5pRT commented Jan 27, 2012

From zefram@fysh.org

p5pRT commented Jan 27, 2012

From @nwc10

p5pRT commented Jan 27, 2012

From @cpansprout

p5pRT commented Jan 27, 2012

From @ysth

p5pRT commented Jan 28, 2012

From @ap

p5pRT commented Jul 10, 2013

From @rjbs

p5pRT commented Jul 11, 2013

From @davidnicol