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

Small bug in include directory slash interpretation #12670

Closed
p5pRT opened this issue Dec 24, 2012 · 16 comments
Closed

Small bug in include directory slash interpretation #12670

p5pRT opened this issue Dec 24, 2012 · 16 comments

Comments

@p5pRT
Copy link

p5pRT commented Dec 24, 2012

Migrated from rt.perl.org#116192 (status was 'resolved')

Searchable as RT116192$

@p5pRT
Copy link
Author

p5pRT commented Dec 24, 2012

From victor@vsespb.ru

Created by victor@vsespb.ru

If perl ran as "perl -Isrc/ src/myscript.pl" I am getting the
following warning message for my script​:
Odd number of elements in hash assignment at src//ChildWorker.pm line 35.
(i.e. two '/' characters)

but when run as perl -Isrc/ src/myscript.pl I am getting just on slash
Odd number of elements in hash assignment at src/ChildWorker.pm line 35.

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.10.1:

Configured by Debian Project at Tue Nov 27 00:14:30 UTC 2012.

Summary of my perl5 (revision 5 version 10 subversion 1) configuration:

  Platform:
    osname=linux, osvers=2.6.42-23-generic,
archname=x86_64-linux-gnu-thread-multi
    uname='linux komainu 2.6.42-23-generic #36-ubuntu smp tue apr 10
20:39:51 utc 2012 x86_64 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN
-Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr
-Dprivlib=/usr/share/perl/5.10 -Darchlib=/usr/lib/perl/5.10
-Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5
-Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local
-Dsitelib=/usr/local/share/perl/5.10.1
-Dsitearch=/usr/local/lib/perl/5.10.1 -Dman1dir=/usr/share/man/man1
-Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1
-Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl
-Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio
-Uusenm -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib
-Dlibperl=libperl.so.5.10.1 -Dd_dosuid -des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN
-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing
-pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.4.3', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=/lib/libc-2.11.1.so, so=so, useshrplib=true, libperl=libperl.so.5.10.1
    gnulibc_version='2.11.1'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib
-fstack-protector'

Locally applied patches:



@INC for perl 5.10.1:
    /etc/perl
    /usr/local/lib/perl/5.10.1
    /usr/local/share/perl/5.10.1
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.10
    /usr/share/perl/5.10
    /usr/local/lib/site_perl
    .


Environment for perl 5.10.1:
    HOME=/home/vse
    LANG=ru_RU.utf8
    LANGUAGE=en_US:en
    LC_MESSAGES=en_US.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/vse/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/home/vse/.rvm/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Dec 25, 2012

From @jkeenan

On Mon Dec 24 08​:42​:38 2012, vsespb wrote​:

This is a bug report for perl from victor@​vsespb.ru,
generated with the help of perlbug 1.39 running under perl 5.10.1.

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

If perl ran as "perl -Isrc/ src/myscript.pl" I am getting the
following warning message for my script​:
Odd number of elements in hash assignment at src//ChildWorker.pm line
35.
(i.e. two '/' characters)

but when run as perl -Isrc/ src/myscript.pl I am getting just on slash
Odd number of elements in hash assignment at src/ChildWorker.pm line
35.

Can you supply more details as to the content of 'src/myscript.pl' and
'src/ChildWorker.pm'?

It may be that your code has a problem which truly ought to generate a
warning. But it will be difficult to tell for sure until we see some
more code.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Dec 25, 2012

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

@p5pRT
Copy link
Author

p5pRT commented Dec 25, 2012

From victor@vsespb.ru

Ok, I made proof-of-concept code​:

$ cat src/myscript.pl
use MyModule;
1;
$ cat src/MyModule.pm
package MyModule;
use warnings;
use strict;
$x=1;
1;
$ perl -Isrc src/myscript.pl
Global symbol "$x" requires explicit package name at src/MyModule.pm line 4.
Compilation failed in require at src/myscript.pl line 1.
BEGIN failed--compilation aborted at src/myscript.pl line 1.
$ perl -Isrc/ src/myscript.pl
Global symbol "$x" requires explicit package name at src//MyModule.pm
line 4.
Compilation failed in require at src/myscript.pl line 1.
BEGIN failed--compilation aborted at src/myscript.pl line 1.

bug in message "Global symbol "$x" requires explicit package name at
src//MyModule.pm"

Пнд. Дек. 24 20​:11​:13 2012, jkeenan писал​:

On Mon Dec 24 08​:42​:38 2012, vsespb wrote​:

This is a bug report for perl from victor@​vsespb.ru,
generated with the help of perlbug 1.39 running under perl 5.10.1.

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

If perl ran as "perl -Isrc/ src/myscript.pl" I am getting the
following warning message for my script​:
Odd number of elements in hash assignment at src//ChildWorker.pm line
35.
(i.e. two '/' characters)

but when run as perl -Isrc/ src/myscript.pl I am getting just on slash
Odd number of elements in hash assignment at src/ChildWorker.pm line
35.

Can you supply more details as to the content of 'src/myscript.pl' and
'src/ChildWorker.pm'?

It may be that your code has a problem which truly ought to generate a
warning. But it will be difficult to tell for sure until we see some
more code.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Dec 25, 2012

From @hvds

"James E Keenan via RT" <perlbug-followup@​perl.org> wrote​:
:On Mon Dec 24 08​:42​:38 2012, vsespb wrote​:
:> This is a bug report for perl from victor@​vsespb.ru,
:> generated with the help of perlbug 1.39 running under perl 5.10.1.
[...]
:Can you supply more details as to the content of 'src/myscript.pl' and
:'src/ChildWorker.pm'?
:
:It may be that your code has a problem which truly ought to generate a
:warning. But it will be difficult to tell for sure until we see some
:more code.

I don't think the script content has any particular relevance. Here's an
example​:

% mkdir mylib ; echo 'print "hello world" . $undef' >mylib/A.pm
% perl -Imylib/ -wle 'use A'
Use of uninitialized value $undef in concatenation (.) or string at mylib//A.pm line 1.
hello world
%

Note the doubled slash in the warning message "mylib//A.pm"? That still
happens with a recent blead.

Hugo

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2012

From @wolfsage

I've attached a patch that I believe solves this, though I'm not sure of
its portability on windows. Someone with more experience about how Perl
behaves in that regard would be useful.

Also, I've tested it through t/run/switchM.t, though the bug is really
in "require", so that might not be appropriate.

Hope this helps regardless.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2012

From @wolfsage

0001-RT-116192-If-a-directory-in-INC-already-has-a-traili.patch
From ae66195d52cc37b0e6c70d2aa2ce01317171f38d Mon Sep 17 00:00:00 2001
From: Matthew Horsfall (alh) <wolfsage@gmail.com>
Date: Thu, 27 Dec 2012 10:38:08 -0500
Subject: [PATCH] RT-116192 - If a directory in @INC already has a trailing '/', don't add another.

---
 MANIFEST             |    2 ++
 pp_ctl.c             |    7 ++++++-
 t/run/flib/broken.pm |    8 ++++++++
 t/run/switchM.t      |   19 +++++++++++++++++++
 4 files changed, 35 insertions(+), 1 deletions(-)
 create mode 100644 t/run/flib/broken.pm
 create mode 100644 t/run/switchM.t

diff --git a/MANIFEST b/MANIFEST
index f6ba34c..28ee69a 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -5505,6 +5505,7 @@ t/run/cloexec.t			Test close-on-exec.
 t/run/dtrace.pl			For dtrace.t
 t/run/dtrace.t			Test for DTrace probes
 t/run/exit.t			Test perl's exit status.
+t/run/flib/broken.pm		Bad .pm file for switchM.t
 t/run/fresh_perl.t		Tests that require a fresh perl.
 t/run/locale.t		Tests related to locale handling
 t/run/mad.t			Test vs MAD environment
@@ -5520,6 +5521,7 @@ t/run/switches.t		Tests for the other switches (-0, -l, -c, -s, -M, -m, -V, -v,
 t/run/switchF1.t		Pathological tests for the -F switch
 t/run/switchF.t			Test the -F switch
 t/run/switchI.t			Test the -I switch
+t/run/switchM.t			Test the -M switch
 t/run/switchn.t			Test the -n switch
 t/run/switchp.t			Test the -p switch
 t/run/switcht.t			Test the -t switch
diff --git a/pp_ctl.c b/pp_ctl.c
index 199df1f..900bde1 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -3889,7 +3889,12 @@ PP(pp_require)
 
 			memcpy(tmp, dir, dirlen);
 			tmp +=dirlen;
-			*tmp++ = '/';
+
+			/* Avoid '<dir>//<file>' */
+			if (*(tmp-1) != '/') {
+			    *tmp++ = '/';
+			}
+
 			/* name came from an SV, so it will have a '\0' at the
 			   end that we can copy as part of this memcpy().  */
 			memcpy(tmp, name, len + 1);
diff --git a/t/run/flib/broken.pm b/t/run/flib/broken.pm
new file mode 100644
index 0000000..18f4d45
--- /dev/null
+++ b/t/run/flib/broken.pm
@@ -0,0 +1,8 @@
+package broken;
+
+use strict;
+use warnings;
+
+$x = 1;
+
+1;
diff --git a/t/run/switchM.t b/t/run/switchM.t
new file mode 100644
index 0000000..72e8908
--- /dev/null
+++ b/t/run/switchM.t
@@ -0,0 +1,19 @@
+#!./perl
+
+BEGIN {
+    chdir 't' if -d 't';
+    @INC = '../lib';
+}
+use strict;
+
+require './test.pl';
+
+plan(2);
+
+like(runperl(switches => ['-Irun/flib', '-Mbroken'], stderr => 1),
+     qr/^Global symbol "\$x" requires explicit package name at run\/flib\/broken.pm line 6\./,
+     "Ensure -Irun/flib produces correct filename in warnings");
+
+like(runperl(switches => ['-Irun/flib/', '-Mbroken'], stderr => 1),
+     qr/^Global symbol "\$x" requires explicit package name at run\/flib\/broken.pm line 6\./,
+     "Ensure -Irun/flib/ produces correct filename in warnings");
-- 
1.7.0.4

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2012

From victor@vsespb.ru

Will it work if dirlen==0 ?

On Thu Dec 27 07​:45​:05 2012, alh wrote​:

I've attached a patch that I believe solves this, though I'm not sure of
its portability on windows. Someone with more experience about how Perl
behaves in that regard would be useful.

Also, I've tested it through t/run/switchM.t, though the bug is really
in "require", so that might not be appropriate.

Hope this helps regardless.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2012

From [Unknown Contact. See original ticket]

Will it work if dirlen==0 ?

On Thu Dec 27 07​:45​:05 2012, alh wrote​:

I've attached a patch that I believe solves this, though I'm not sure of
its portability on windows. Someone with more experience about how Perl
behaves in that regard would be useful.

Also, I've tested it through t/run/switchM.t, though the bug is really
in "require", so that might not be appropriate.

Hope this helps regardless.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2012

From @wolfsage

On Thu, Dec 27, 2012 at 10​:45 AM, Matthew Horsfall via RT <
perlbug-followup@​perl.org> wrote​:

I've attached a patch that I believe solves this, though I'm not sure of
its portability on windows. Someone with more experience about how Perl
behaves in that regard would be useful.

Also, I've tested it through t/run/switchM.t, though the bug is really
in "require", so that might not be appropriate.

Hope this helps regardless.

-- Matthew Horsfall (alh)

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=116192

Now that I look at this, is it possible for *(tmp-1) to access bad memory?
Would "if (!dirlen || *(tmp-1) != '/') { ... } be better?

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Dec 27, 2012

From victor@vsespb.ru

.. or even better if (dirlen && *(tmp-1) != '/') { ... }

On Thu Dec 27 12​:49​:17 2012, alh wrote​:

On Thu, Dec 27, 2012 at 10​:45 AM, Matthew Horsfall via RT <
perlbug-followup@​perl.org> wrote​:

I've attached a patch that I believe solves this, though I'm not sure of
its portability on windows. Someone with more experience about how Perl
behaves in that regard would be useful.

Also, I've tested it through t/run/switchM.t, though the bug is really
in "require", so that might not be appropriate.

Hope this helps regardless.

-- Matthew Horsfall (alh)

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=116192

Now that I look at this, is it possible for *(tmp-1) to access bad memory?
Would "if (!dirlen || *(tmp-1) != '/') { ... } be better?

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Dec 28, 2012

From @wolfsage

On Thu, Dec 27, 2012 at 3​:54 PM, Victor Efimov via RT <
perlbug-followup@​perl.org> wrote​:

.. or even better if (dirlen && *(tmp-1) != '/') { ... }

Right now '/' is added whether or not dirlen is > 0, so (if !dirlen ||
*(tmp-1) != '/') would continue to behave the same.

if (dirlen && ...) would change that behavior.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Dec 28, 2012

From victor@vsespb.ru

Ok, get it. Agree.

On Fri Dec 28 09​:18​:51 2012, alh wrote​:

On Thu, Dec 27, 2012 at 3​:54 PM, Victor Efimov via RT <
perlbug-followup@​perl.org> wrote​:

.. or even better if (dirlen && *(tmp-1) != '/') { ... }

Right now '/' is added whether or not dirlen is > 0, so (if !dirlen ||
*(tmp-1) != '/') would continue to behave the same.

if (dirlen && ...) would change that behavior.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Dec 28, 2012

From [Unknown Contact. See original ticket]

Ok, get it. Agree.

On Fri Dec 28 09​:18​:51 2012, alh wrote​:

On Thu, Dec 27, 2012 at 3​:54 PM, Victor Efimov via RT <
perlbug-followup@​perl.org> wrote​:

.. or even better if (dirlen && *(tmp-1) != '/') { ... }

Right now '/' is added whether or not dirlen is > 0, so (if !dirlen ||
*(tmp-1) != '/') would continue to behave the same.

if (dirlen && ...) would change that behavior.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2013

From @cpansprout

On Thu Dec 27 12​:49​:17 2012, alh wrote​:

On Thu, Dec 27, 2012 at 10​:45 AM, Matthew Horsfall via RT <
perlbug-followup@​perl.org> wrote​:

I've attached a patch that I believe solves this, though I'm not sure of
its portability on windows. Someone with more experience about how Perl
behaves in that regard would be useful.

Also, I've tested it through t/run/switchM.t, though the bug is really
in "require", so that might not be appropriate.

Hope this helps regardless.

-- Matthew Horsfall (alh)

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=116192

Now that I look at this, is it possible for *(tmp-1) to access bad memory?
Would "if (!dirlen || *(tmp-1) != '/') { ... } be better?

Yes. Thank you. Applied as 6b0bdd7.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2013

@cpansprout - Status changed from 'open' to 'resolved'

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

1 participant