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

whole-name-quoted filename can't be read [cpan #57586] #14003

Closed
p5pRT opened this issue Jul 27, 2014 · 10 comments
Closed

whole-name-quoted filename can't be read [cpan #57586] #14003

p5pRT opened this issue Jul 27, 2014 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 27, 2014

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

Searchable as RT122415$

@p5pRT
Copy link
Author

p5pRT commented Jul 27, 2014

From @jkeenan

[Since ExtUtils-Manifest is now primarily maintained by Perl 5 Porters, I am moving this ticket here from https://rt.cpan.org/Ticket/Display.html?id=57586. -- jkeenan]

While adding support for quoted filenames (names with spaces) to Dist​::Zilla, I found this bug
in ExtUtils​::Manifest. If you have a filename that begins and ends with quotes, and you
enquote it as suggested, it is not properly dequoted.

use strict;
use ExtUtils​::Manifest qw(maniread);


my $manifest_content = <<'END_MANIFEST';
'\'quoted name.txt\''
END_MANIFEST

{
  open my $fh, '>', 'MANIFEST' or die;
  print $fh $manifest_content;
}

my $manifest = maniread;

print "$_\n" for keys %$manifest;


This should print​:

  'quoted name.txt'

Instead, it prints​:

  quoted name.txt'

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Jul 27, 2014

From @jkeenan

Summary of my perl5 (revision 5 version 21 subversion 3) configuration​:
  Commit id​: ce5b0b8
  Platform​:
  osname=linux, osvers=3.13.0-32-generic, archname=x86_64-linux
  uname='linux zareason 3.13.0-32-generic #57-ubuntu smp tue jul 15 03​:51​:08 utc 2014 x86_64 x86_64 x86_64 gnulinux '
  config_args='-des -Dusedevel'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2',
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.8.2', 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 /usr/lib/gcc/x86_64-linux-gnu/4.8/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.19.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.19'
  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'

Characteristics of this binary (from libperl)​:
  Compile-time options​: HAS_TIMES PERLIO_LAYERS PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP
  PERL_NEW_COPY_ON_WRITE PERL_PRESERVE_IVUV
  PERL_USE_DEVEL USE_64_BIT_ALL USE_64_BIT_INT
  USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_LOCALE_TIME
  USE_PERLIO USE_PERL_ATOF
  Built under linux
  Compiled at Jul 27 2014 17​:53​:58
  %ENV​:
  PERL5LIB="/home/jkeenan/perl5/lib/perl5"
  PERLBREW_BASHRC_VERSION="0.67"
  PERLBREW_HOME="/home/jkeenan/.perlbrew"
  PERLBREW_MANPATH="/home/jkeenan/perl5/perlbrew/perls/perl-5.20.0/man"
  PERLBREW_PATH="/home/jkeenan/perl5/perlbrew/bin​:/home/jkeenan/perl5/perlbrew/perls/perl-5.20.0/bin"
  PERLBREW_PERL="perl-5.20.0"
  PERLBREW_ROOT="/home/jkeenan/perl5/perlbrew"
  PERLBREW_VERSION="0.67"
  PERL_LOCAL_LIB_ROOT="/home/jkeenan/perl5"
  PERL_MB_OPT="--install_base "/home/jkeenan/perl5""
  PERL_MM_OPT="INSTALL_BASE=/home/jkeenan/perl5"
  PERL_WORKDIR="gitwork/perl"
  @​INC​:
  lib
  /home/jkeenan/perl5/lib/perl5/x86_64-linux
  /home/jkeenan/perl5/lib/perl5
  /usr/local/lib/perl5/site_perl/5.21.3/x86_64-linux
  /usr/local/lib/perl5/site_perl/5.21.3
  /usr/local/lib/perl5/5.21.3/x86_64-linux
  /usr/local/lib/perl5/5.21.3
  .

@p5pRT
Copy link
Author

p5pRT commented Jul 27, 2014

From @jkeenan

On Sun Jul 27 16​:05​:08 2014, jkeenan wrote​:

[Since ExtUtils-Manifest is now primarily maintained by Perl 5
Porters, I am moving this ticket here from
https://rt.cpan.org/Ticket/Display.html?id=57586. -- jkeenan]

Please review the patch attached.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Jul 27, 2014

From @jkeenan

122415-0001-Have-maniread-properly-handle-whole-name-quoted-file.patch
From 7d52cf1a331ada2f5eadb4f466501dd84a5c4745 Mon Sep 17 00:00:00 2001
From: James E Keenan <[email protected]>
Date: Sun, 27 Jul 2014 19:11:35 -0400
Subject: [PATCH] Have maniread() properly handle whole-name-quoted filename.

Add test using existing test format.

For: RT #122415 (formerly cpan #57586)
---
 dist/ExtUtils-Manifest/lib/ExtUtils/Manifest.pm |  4 ++--
 dist/ExtUtils-Manifest/t/Manifest.t             | 16 +++++++++++++++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/dist/ExtUtils-Manifest/lib/ExtUtils/Manifest.pm b/dist/ExtUtils-Manifest/lib/ExtUtils/Manifest.pm
index 165a15c..bf92c8f 100644
--- a/dist/ExtUtils-Manifest/lib/ExtUtils/Manifest.pm
+++ b/dist/ExtUtils-Manifest/lib/ExtUtils/Manifest.pm
@@ -12,7 +12,7 @@ use warnings;
 
 use Exporter 5.57 'import';
 
-our $VERSION = '1.64';
+our $VERSION = '1.65';
 our @EXPORT_OK = qw(mkmanifest
                 manicheck  filecheck  fullcheck  skipcheck
                 manifind   maniread   manicopy   maniadd
@@ -354,7 +354,7 @@ sub maniread {
 
         # filename may contain spaces if enclosed in ''
         # (in which case, \\ and \' are escapes)
-        if (($file, $comment) = /^'(\\[\\']|.+)+'\s*(.*)/) {
+        if (($file, $comment) = /^'((?:\\[\\']|.+)+)'\s*(.*)/) {
             $file =~ s/\\([\\'])/$1/g;
         }
         else {
diff --git a/dist/ExtUtils-Manifest/t/Manifest.t b/dist/ExtUtils-Manifest/t/Manifest.t
index a6a89a2..cc61401 100644
--- a/dist/ExtUtils-Manifest/t/Manifest.t
+++ b/dist/ExtUtils-Manifest/t/Manifest.t
@@ -13,7 +13,7 @@ chdir 't';
 
 use strict;
 
-use Test::More tests => 96;
+use Test::More tests => 97;
 use Cwd;
 
 use File::Spec;
@@ -324,6 +324,20 @@ SKIP: {
     $funky_files{'space_quote_backslash'} = 'foo bar\\baz\'quux';
 }
 
+# test including a filename which is itself a quoted string
+# https://rt.perl.org/Ticket/Display.html?id=122415
+SKIP: {
+    my $quoted_filename = q{'quoted name.txt'};
+    my $description     = "quoted string";
+    add_file( $quoted_filename  => $description )
+        or skip "couldn't create $description test file", 1;
+    local $ExtUtils::Manifest::MANIFEST = "albatross";
+    maniadd({ $quoted_filename => $description });
+    is( maniread()->{$quoted_filename}, $description,
+	    'file whose name starts and ends with quotes' );
+    $funky_files{$description} = $quoted_filename;
+}
+
 my @funky_keys = qw(space space_quote space_backslash space_quote_backslash);
 # test including an external manifest.skip file in MANIFEST.SKIP
 {
-- 
1.9.1

@p5pRT
Copy link
Author

p5pRT commented Jul 27, 2014

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

@p5pRT
Copy link
Author

p5pRT commented Aug 4, 2014

From @jkeenan

On Sun Jul 27 16​:16​:02 2014, jkeenan wrote​:

On Sun Jul 27 16​:05​:08 2014, jkeenan wrote​:

[Since ExtUtils-Manifest is now primarily maintained by Perl 5
Porters, I am moving this ticket here from
https://rt.cpan.org/Ticket/Display.html?id=57586. -- jkeenan]

Please review the patch attached.

rjbs​: Although it appears the Toolchain Gang will ultimately take responsibility for ExtUtils-Manifest, could you review this patch, as you were the OP?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Aug 5, 2014

From @rjbs

On Mon Aug 04 16​:02​:49 2014, jkeenan wrote​:

rjbs​: Although it appears the Toolchain Gang will ultimately take
responsibility for ExtUtils-Manifest, could you review this patch, as
you were the OP?

With this patch, my original test code behaves as expected.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Aug 5, 2014

From @jkeenan

On Tue Aug 05 07​:36​:52 2014, rjbs wrote​:

On Mon Aug 04 16​:02​:49 2014, jkeenan wrote​:

rjbs​: Although it appears the Toolchain Gang will ultimately take
responsibility for ExtUtils-Manifest, could you review this patch, as
you were the OP?

With this patch, my original test code behaves as expected.

Thanks. I've asked the Toolchain Gang folks to clarify the ExtUtils-Manifest maintenance issue​:
Perl-Toolchain-Gang/ExtUtils-Manifest#1 (comment)

@p5pRT
Copy link
Author

p5pRT commented Aug 31, 2014

From @jkeenan

On Tue Aug 05 16​:17​:21 2014, jkeenan wrote​:

On Tue Aug 05 07​:36​:52 2014, rjbs wrote​:

On Mon Aug 04 16​:02​:49 2014, jkeenan wrote​:

rjbs​: Although it appears the Toolchain Gang will ultimately take
responsibility for ExtUtils-Manifest, could you review this patch,
as
you were the OP?

With this patch, my original test code behaves as expected.

Thanks. I've asked the Toolchain Gang folks to clarify the ExtUtils-
Manifest maintenance issue​:
https://github.com/Perl-Toolchain-Gang/ExtUtils-
Manifest/issues/1#issuecomment-51273451

Patch applied upstream​: Perl-Toolchain-Gang/ExtUtils-Manifest#1

Patch applied to blead as part of​:

commit 9a7aaf5
Author​: Chris 'BinGOs' Williams <chris@​bingosnet.co.uk>
Date​: Sun Aug 31 14​:52​:18 2014 +0100

  Update ExtUtils-Manifest to CPAN version 1.66

Marking ticket resolved.

@p5pRT p5pRT closed this as completed Aug 31, 2014
@p5pRT
Copy link
Author

p5pRT commented Aug 31, 2014

@jkeenan - 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