Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code embeddable in argument to -i switch #12128

Open
p5pRT opened this issue May 23, 2012 · 12 comments
Open

Code embeddable in argument to -i switch #12128

p5pRT opened this issue May 23, 2012 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented May 23, 2012

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

Searchable as RT113046$

@p5pRT
Copy link
Author

p5pRT commented May 23, 2012

From jdmadea@gmail.com

Created by jeremy@cpan.org

This is a bug report for perl from jeremy@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.16.0.

-----------------------------------------------------------------
Example​:
$ perl -i'.bak -e warn 1;' -e'warn 2'
1 at -e line 1.
2 at -e line 2.

Showing that code can be embedded in the argument to the -i
command line switch. Other switches can be embedded as well.
This is only possible when the switch is preceded by exactly
one space. The extension can be empty and the switch does not
require the dash.

$ perl -i' e warn 1'
1 at -e line 1.

And this shows that, with two spaces, the bug is not exercised​:

$ perl -MO=Deparse -i' e warn 1' -e'warn 2'
BEGIN { $^I = ""; }
warn 2;
-e syntax OK

The bug was first reported by EvanCarroll on perlmonks.org

http​://www.perlmonks.org/?node_id=971866

Here is a patch.

Inline Patch
--- orig-perl.c 2012-05-22 15:43:39.000000000 -0400
+++ perl.c      2012-05-22 15:45:45.000000000 -0400
@@ -3187,11 +3187,8 @@

            PL_inplace = savepvn(start, s - start);
        }
-       if (*s) {
-           ++s;
-           if (*s == '-')      /* Additional switches on #! line. */
-               s++;
-       }
+       if (*s == '-')  /* Additional switches on #! line. */
+           s++;
        return s;
     case 'I':  /* -I handled both here and in parse_body() */
        forbid_setid('I', FALSE);
Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.16.0:

Configured by jmadea at Tue May 22 13:17:52 EDT 2012.

Summary of my perl5 (revision 5 version 16 subversion 0) configuration:

  Platform:
    osname=linux, osvers=2.6.32-5-686, archname=i686-linux-thread-multi
    uname='linux kermit 2.6.32-5-686 #1 smp mon jan 16 16:04:25 utc
2012 i686 gnulinux '
    config_args=''
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE
-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe
-fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.4.5', 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='gcc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib/../lib /usr/lib/../lib /lib /usr/lib
    libs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.11.3.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.11.3'
  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.16.0:
    /home/jmadea/lib/perl5/site_perl/5.16.0/i686-linux-thread-multi
    /home/jmadea/lib/perl5/site_perl/5.16.0
    /home/jmadea/lib/perl5/5.16.0/i686-linux-thread-multi
    /home/jmadea/lib/perl5/5.16.0
    .


Environment for perl 5.16.0:
    HOME=/home/jmadea
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/jmadea/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Jun 7, 2012

From @ap

* Jeremy Madea <perlbug-followup@​perl.org> [2012-05-25 09​:50]​:

Example​:
$ perl -i'.bak -e warn 1;' -e'warn 2'
1 at -e line 1.
2 at -e line 2.

Showing that code can be embedded in the argument to the -i command
line switch.

Red herring.

Other switches can be embedded as well.

That is what you can in fact do. If the switch you embed is a -e then
of course that will let you embed code, but ultimately this is about
switches, not code.

The bug was first reported by EvanCarroll on perlmonks.org

This is not actually a bug. Consider
http​://www.nntp.perl.org/group/perl.perl5.porters/;msgid=CALJW-qH4L3kNp-exwgNsTgz-QDrpymbmUhQEbkuS2=uhBQJfcg%40mail.gmail.com

And I believe that comment in the code you propose to remove​:

- if (*s) {
- ++s;
- if (*s == '-') /* Additional switches on #! line. */
- s++;
- }

… is a honking big clue that Eric’s conjecture is right.

In which case your proposed replacement​:

+ if (*s == '-') /* Additional switches on #! line. */
+ s++;

… by preserving the comment while removing the functionality, makes it
a lie.

Now, I don’t know if we want to *keep* the feature. It is a workaround,
and in and of itself the behaviour is crazy.

But it is not there on accident so it shouldn’t be removed the same way
as something that were.

@p5pRT
Copy link
Author

p5pRT commented Jun 7, 2012

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

@p5pRT
Copy link
Author

p5pRT commented Jun 7, 2012

From jeremy@cpan.org

I'm sorry, but I believe you misunderstand the nature of this bug and my
title has probably confused the issue more than necessary.

The switch parsing in general may or may not be considered buggy. It
will take additional review. If it is "fixed" it will probably need
different code for different platforms.

This fix is for a separate issue.

The bug this patch fixes is that -i behaves differently from other
similar switches. I'll use -F as an example. Please review the following
with close attention to what I have called "Group #4"​:

Group #1​: With dash, no space.

Unpatched.

$ perl -i'-e1'
^C
$ perl -F'-e1'
^C

Patched.

$ ./perl -i'-e1'
^C
$ ./perl -F'-e1'
^C

Group #2​: Without dash, no space.

Unpatched.

$ perl -i'e1'
^C
$ perl -F'e1'
^C

Patched.

$ ./perl -i'e1'
^C
$ ./perl -F'e1'
^C

Group #3​: With dash, one space.

Unpatched.

$ perl -i' -e1'
$ perl -F' -e1'

Patched.

$ ./perl -i' -e1'
$ ./perl -F' -e1'

Group #4​: Without dash, one space.

Unpatched. NOTE DIFFERENT BEHAVIOR.

$ perl -i' e1'
$ perl -F' e1'
^C

Patched. (Different behavior fixed.)

$ ./perl -i' e1'
^C
$ ./perl -F' e1'
^C

Group #5​: With dash, two spaces.

Unpatched.

$ perl -i' -e1'
$ perl -F' -e1'

Patched.

$ ./perl -i' -e1'
$ ./perl -F' -e1'

Group #6​: Without dash, two spaces.

Unpatched.

$ perl -i' e1'
^C
$ perl -F' e1'
^C

Patched.

$ ./perl -i' e1'
^C
$ ./perl -F' e1'
^C

Group #7​: With dash, three spaces.

Unpatched.

$ perl -i' -e1'
$ perl -F' -e1'

Patched.

$ ./perl -i' -e1'
$ ./perl -F' -e1'

Group #8​: Without dash, three spaces.

Patched.

$ ./perl -i' e1'
^C
$ ./perl -F' e1'
^C

$ perl -i' e1'
^C
$ perl -F' e1'
^C

And so on.

I believe that discrepancy in behavior probably was an accident and
should be removed as if it were. ;-)

I do not believe this patch will break if

#!/usr/bin/perl -w -i

is executed as

/usr/bin/perl '-i -w' foo.pl

I don't have such a system though. If there is no test for that case,
one should be added, of course.

On Wed Jun 06 21​:04​:24 2012, aristotle wrote​:

* Jeremy Madea <perlbug-followup@​perl.org> [2012-05-25 09​:50]​:

Example​:
$ perl -i'.bak -e warn 1;' -e'warn 2'
1 at -e line 1.
2 at -e line 2.

Showing that code can be embedded in the argument to the -i command
line switch.

Red herring.

Other switches can be embedded as well.

That is what you can in fact do. If the switch you embed is a -e then
of course that will let you embed code, but ultimately this is about
switches, not code.

The bug was first reported by EvanCarroll on perlmonks.org

This is not actually a bug. Consider
http​://www.nntp.perl.org/group/perl.perl5.porters/;msgid=CALJW-
qH4L3kNp-exwgNsTgz-QDrpymbmUhQEbkuS2=uhBQJfcg%40mail.gmail.com

And I believe that comment in the code you propose to remove​:

- if (*s) {
- ++s;
- if (*s == '-') /* Additional switches on #! line.
*/
- s++;
- }

… is a honking big clue that Eric’s conjecture is right.

In which case your proposed replacement​:

+ if (*s == '-') /* Additional switches on #! line. */
+ s++;

… by preserving the comment while removing the functionality, makes it
a lie.

Now, I don’t know if we want to *keep* the feature. It is a
workaround,
and in and of itself the behaviour is crazy.

But it is not there on accident so it shouldn’t be removed the same
way
as something that were.

@p5pRT
Copy link
Author

p5pRT commented Jun 7, 2012

From @ap

* Jeremy Madea via RT <perlbug-followup@​perl.org> [2012-06-07 13​:25]​:

I'm sorry, but I believe you misunderstand the nature of this bug and
my title has probably confused the issue more than necessary.

So I did.

The bug this patch fixes is that -i behaves differently from other
similar switches. I'll use -F as an example. Please review the
following with close attention to what I have called "Group #4"​:

Thanks! That was a helpful illustration.

I believe that discrepancy in behavior probably was an accident and
should be removed as if it were. ;-)

That seems more likely to me now as well.

@p5pRT
Copy link
Author

p5pRT commented Jun 9, 2012

From @cpansprout

On Thu Jun 07 03​:38​:16 2012, jmadea wrote​:

I don't have such a system though.

Does anyone know which system that is? Is it possible to probe for it
at configure time?

If there is no test for that case,
one should be added, of course.

Could you write tests for the cases that your patch changes?

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jun 9, 2012

From jeremy@cpan.org

On Fri Jun 08 18​:03​:22 2012, sprout wrote​:

On Thu Jun 07 03​:38​:16 2012, jmadea wrote​:

I don't have such a system though.

Does anyone know which system that is? Is it possible to probe for it
at configure time?

Eric Brine <ikegami@​adaelis.com> suggested this was a possibility in a
thread on p5p, though he himself posed it as a question.

If there is no test for that case,
one should be added, of course.

Could you write tests for the cases that your patch changes?

My initial report was poorly worded as I was still partially focused on
a wider (unresolved) issue. Let me clearly state what my patch does
before answering your question.

While looking at the behavior reported by EvanCarroll, I found -i
working differently than other switches. Specifically, after exactly one
space in its argument, a following letter (w/o a dash) would be
interpreted as a switch.

The following emit warnings​:
perl -i'foo e warn 1'
perl -i' e warn 1'

With two spaces after the argument, these do not emit warnings​:
perl -i'foo e warn 1'
perl -i' e warn 1'

I quickly found that there were other switches that behaved similarly to
-i wrt to putting further switches in their arguments; -F and -C are
examples. But I did not find another that would interpret a letter
without a dash as a switch.

For instance, while these do emit warnings​:
perl -F'foo -e warn 1'
perl -F' -e warn 1'
perl -C'1 -e warn 1'
perl -C' -e warn 1'

The following do not​:
perl -F'foo e warn 1'
perl -F' warn 1'
perl -C'1 e warn 1'
perl -C' e warn 1'

My patch changes -i to work as other switches (like -F and -C) do.

In other words, after the patch, the following no longer emit warnings​:
perl -i' e warn 1'
perl -i'foo e warn 1'

You asked if I can add a test case for it. I suppose I could. But do you
really want to test that perl is *not* doing something that it
*shouldn't*? If so, it seems like there might be an awful lot of test
cases to add. :-) I don't imagine it's very likely someone will add this
behavior to -i again by accident. So long as this patch doesn't cause
any regressions, I would think that should be sufficient.

-j

--
Jeremy Madea

@p5pRT
Copy link
Author

p5pRT commented Jun 11, 2012

From @cpansprout

On Sat Jun 09 08​:06​:27 2012, jmadea wrote​:

I don't imagine it's very likely someone will add this
behavior to -i again by accident.

Stranger things have happened.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jun 11, 2012

From [Unknown Contact. See original ticket]

On Sat Jun 09 08​:06​:27 2012, jmadea wrote​:

I don't imagine it's very likely someone will add this
behavior to -i again by accident.

Stranger things have happened.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jun 11, 2012

From @cpansprout

On Sun Jun 10 22​:41​:54 2012, sprout wrote​:

On Sat Jun 09 08​:06​:27 2012, jmadea wrote​:

I don't imagine it's very likely someone will add this
behavior to -i again by accident.

Stranger things have happened.

Oops. I forgot the smiley​: :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jun 11, 2012

From [Unknown Contact. See original ticket]

On Sun Jun 10 22​:41​:54 2012, sprout wrote​:

On Sat Jun 09 08​:06​:27 2012, jmadea wrote​:

I don't imagine it's very likely someone will add this
behavior to -i again by accident.

Stranger things have happened.

Oops. I forgot the smiley​: :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jun 11, 2012

From jeremy@cpan.org

Apologies for any duplicates. I think I hit the link to toggle the CC
of p5p on the RT web interface. (Aside​: Is that link really all that
helpful?)

On Fri Jun 08 18​:03​:22 2012, sprout wrote​:

On Thu Jun 07 03​:38​:16 2012, jmadea wrote​:

I don't have such a system though.

Does anyone know which system that is?  Is it possible to probe for it
at configure time?

Eric Brine <ikegami@​adaelis.com> suggested this was a possibility in a
thread on p5p, though he himself posed it as a question.

If there is no test for that case,
one should be added, of course.

Could you write tests for the cases that your patch changes?

My initial report was poorly worded as I was still partially focused on
a wider (unresolved) issue. Let me clearly state what my patch does
before answering your question.

While looking at the behavior reported by EvanCarroll, I found -i
working differently than other switches. Specifically, after exactly one
space in its argument, a following letter (w/o a dash) would be
interpreted as a switch.

The following emit warnings​:
perl -i'foo e warn 1'
perl -i' e warn 1'

With two spaces after the argument, these do not emit warnings​:
perl -i'foo  e warn 1'
perl -i'  e warn 1'

I quickly found that there were other switches that behaved similarly to
-i wrt to putting further switches in their arguments; -F and -C are
examples. But I did not find another that would interpret a letter
without a dash as a switch.

For instance, while these do emit warnings​:
perl -F'foo -e warn 1'
perl -F' -e warn 1'
perl -C'1 -e warn 1'
perl -C' -e warn 1'

The following do not​:
perl -F'foo e warn 1'
perl -F' warn 1'
perl -C'1 e warn 1'
perl -C' e warn 1'

My patch changes -i to work as other switches (like -F and -C) do.

In other words, after the patch, the following no longer emit warnings​:
perl -i' e warn 1'
perl -i'foo e warn 1'

You asked if I can add a test case for it. I suppose I could. But do you
really want to test that perl is *not* doing something that it
*shouldn't*? If so, it seems like there might be an awful lot of test
cases to add. :-) I don't imagine it's very likely someone will add this
behavior to -i again by accident. So long as this patch doesn't cause
any regressions, I would think that should be sufficient.

-j

--
Jeremy Madea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants