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

Blead Breaks CPAN: JETTERO/Games-RolePlay-MapGen-1.5008.tar.gz #16405

Closed
p5pRT opened this issue Feb 6, 2018 · 8 comments
Closed

Blead Breaks CPAN: JETTERO/Games-RolePlay-MapGen-1.5008.tar.gz #16405

p5pRT opened this issue Feb 6, 2018 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 6, 2018

Migrated from rt.perl.org#132822 (status was 'rejected')

Searchable as RT132822$

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2018

From @eserte

This is a bug report for perl from slaven@​rezic.de,
generated with the help of perlbug 1.41 running under perl 5.27.9.


The test suite of Games-RolePlay-MapGen-1.5008 started to fail
with perl 5.27.6, see
http​://matrix.cpantesters.org/?dist=Games-RolePlay-MapGen+1.5008

And it still fails with perl5.27.8-157-gef80cd9



Flags​:
  category=core
  severity=low


Site configuration information for perl 5.27.9​:

Configured by eserte at Tue Feb 6 19​:16​:51 CET 2018.

Summary of my perl5 (revision 5 version 27 subversion 9) configuration​:
  Commit id​: ef80cd9
  Platform​:
  osname=linux
  osvers=3.16.0-4-amd64
  archname=x86_64-linux
  uname='linux cabulja 3.16.0-4-amd64 #1 smp debian 3.16.51-3 (2017-12-13) x86_64 gnulinux '
  config_args='-D useshrplib=true -Dprefix=/opt/perl5.27.8-157-gef80cd9 -Dusemymalloc=n -D cc=ccache cc -D usedevel=define -Duse64bit -de'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=undef
  usemultiplicity=undef
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
  optimize='-O2'
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='4.9.2'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  prototype=define
  Linker and Libraries​:
  ld='ccache cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.9/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=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.19.so
  so=so
  useshrplib=true
  libperl=libperl.so
  gnulibc_version='2.19'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E -Wl,-rpath,/opt/perl5.27.8-157-gef80cd9/lib/5.27.9/x86_64-linux/CORE'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


@​INC for perl 5.27.9​:
  /opt/perl5.27.8-157-gef80cd9/lib/site_perl/5.27.9/x86_64-linux
  /opt/perl5.27.8-157-gef80cd9/lib/site_perl/5.27.9
  /opt/perl5.27.8-157-gef80cd9/lib/5.27.9/x86_64-linux
  /opt/perl5.27.8-157-gef80cd9/lib/5.27.9


Environment for perl 5.27.9​:
  HOME=/home/eserte
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/local/bin​:/usr/bin​:/bin​:/usr/local/sbin​:/usr/sbin​:/sbin​:/home/eserte/bin/linux-gnu​:/home/eserte/bin/sh​:/home/eserte/bin​:/home/eserte/bin/pistachio-perl/bin​:/usr/games​:/home/eserte/devel
  PERLDOC=-MPod​::Perldoc​::ToTextOverstrike
  PERL_BADLANG (unset)
  SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2018

From zefram@fysh.org

Bisects to commit a155eb0 "(perl #131895)
fail stat on names with \0 embedded".

The underlying issue is that the Games​::RolePlay​::MapGen module has a
method (load_map) that takes an argument that can be either the pathname
of a file in Storable format or a string of Storable data. The method
will interpret Storable data from either the file or the supplied string.
It decides which of these to treat the argument as by the rather poor
method of statting the argument, via -f. If -f returns true then it
treats the argument as a pathname, otherwise as a data string.

Prior to 5.27.6, a string containing a nul (as a Storable data string
generally will) would be passed to the stat(2) syscall, with only the
part up to the first nul taking effect. This was inconsistent with the
open() builtin, which has rejected nul-containing pathnames since 5.20.
5.27.6 made stat checks deliberately fail on nul-containing strings
in the same way as open(). This change actually makes G​:RP​:MG's logic
less dodgy than it previously was​: it means that data strings generally
won't be mistaken for pathnames, even if a file exists with a funny name
matching the start of the string.

When a string containing nul is rejected as a pathname Perl emits
a warning. Normally it would then go on to yield ENOENT from whatever
file-related builtin triggered this. But G​:RP​:MG makes a bunch of warning
categories fatal, by the use of the terribly misnamed common​::sense
pragma, and the "Invalid \0 character in pathname" warning is one of those
made fatal. So it has electively made its dodgy logic a fatal error.

There is nothing for the core to change here. It is for the module to
address the conflict between its use of -f on invalid pathnames and its
choice of warning pragmata.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2018

From @jkeenan

I started to look at this and was tracking down the problem when I saw Zefram's post. I agree with his diagnosis. In particular, commenting out 'use common​::sense;' throughout enables all tests to pass on Perl 5 blead. I have opened a ticket in the module's bug tracker.

Resolving ticket as Rejected. Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Feb 6, 2018

@jkeenan - Status changed from 'open' to 'rejected'

@p5pRT p5pRT closed this as completed Feb 6, 2018
@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2018

From @xsawyerx

On 02/06/2018 10​:27 PM, Zefram wrote​:

Bisects to commit a155eb0 "(perl #131895)
fail stat on names with \0 embedded".

[...]

There is nothing for the core to change here. It is for the module to
address the conflict between its use of -f on invalid pathnames and its
choice of warning pragmata.

Any chance you could provide a suggestion on how to cleanly resolve it
so we could communicate that to the module author?

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2018

From zefram@fysh.org

Sawyer X wrote​:

Any chance you could provide a suggestion on how to cleanly resolve it
so we could communicate that to the module author?

There are many options​:

* check for nul in the string before -f, always treating nul-containing
  strings as data strings;

* discriminate between pathnames and data entirely based on looking like
  a data string, with no -f check at all;

* suppress that category of warning in some narrow scope around -f;

* drop the dual-use interface in favour of separate interfaces for
  pathnames and data strings.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Feb 11, 2018

From @xsawyerx

On 02/08/2018 02​:25 PM, Zefram wrote​:

Sawyer X wrote​:

Any chance you could provide a suggestion on how to cleanly resolve it
so we could communicate that to the module author?
There are many options​:

* check for nul in the string before -f, always treating nul-containing
strings as data strings;

* discriminate between pathnames and data entirely based on looking like
a data string, with no -f check at all;

* suppress that category of warning in some narrow scope around -f;

* drop the dual-use interface in favour of separate interfaces for
pathnames and data strings.

This is helpful. Thank you. I've added them to the ticket upstream.

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