parse versions statically when possible

[haarg]

This is incomplete, but is a start at addressing #288.

Parsing version declarations by evaling them is rather unsafe. It would
be possible to improve the security of that by using a Safe container.
However, Safe.pm is an XS module and EUMM has to work without any XS.
Safe.pm should only be unavailable when compiling perl core though, and
the versions in perl core can all be parsed statically.

This patch takes the first step, by implementing static parsing of
versions that follow common patterns. Later, a Safe container can be
added for the eval code path. We may consider adding a warning for
the more exotic forms, and possibly rejecting them in the far future.

There are currently some inconsistencies. Primarily, unquoted versions are extracted as strings rather than their parsed values. So the following version declarations will be parsed differently:

$VERSION = 0.0;
$VERSION = 1.200_003;
$VERSION = v1.2;
$VERSION = 1.2.3;

[karenetheridge]

Does it make sense to do this all in EUMM, as opposed to bundling a copy of (a subset of) Module::Metadata to do this work? Ideally we should just have one code path that can do a robust job of version parsing, so everything is consistent. We're pretty close to being able to get the branch merged which uses a Safe container, I think.

…

On Fri, Mar 3, 2017 at 10:37 AM, Graham Knop ***@***.***> wrote: This is incomplete, but is a start at addressing #288 <#288>. Parsing version declarations by evaling them is rather unsafe. It would be possible to improve the security of that by using a Safe container. However, Safe.pm is an XS module and EUMM has to work without any XS. Safe.pm should only be unavailable when compiling perl core though, and the versions in perl core can all be parsed statically. This patch takes the first step, by implementing static parsing of versions that follow common patterns. Later, a Safe container can be added for the eval code path. We may consider adding a warning for the more exotic forms, and possibly rejecting them in the far future. There are currently some inconsistencies. Primarily, unquoted versions are extracted as strings rather than their parsed values. So the following version declarations will be parsed differently: $VERSION = 0.0; $VERSION = 1.200_003; $VERSION = v1.2; $VERSION = 1.2.3; ------------------------------ You can view, comment on, or merge this pull request online at: #291 Commit Summary - parse versions statically when possible File Changes - *M* lib/ExtUtils/MM_Unix.pm <https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/pull/291/files#diff-0> (71) Patch Links: - https://github.com/Perl-Toolchain-Gang/ExtUtils- MakeMaker/pull/291.patch - https://github.com/Perl-Toolchain-Gang/ExtUtils- MakeMaker/pull/291.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#291>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASfy62Ttq5VeYJXi1HafygtW_VDElhfks5riF3dgaJpZM4MSmTr> .

[haarg]

I don't love the idea of adding another prereq to EUMM.

[haarg]

@karenetheridge Maybe we could move this code (and any further version parsing code) into an ExtUtils::ParseVersion or something that could be packaged with EUMM and both EUMM and Module::Metadata could use it. Then we can share the code without dealing with more bundling or prereqs in EUMM.

[karenetheridge]

@haarg that would be eminently feasible, and make it a lot easier to wrangle future fixes to Module::Metadata as well.

I'll take some of my rambling thoughts to #toolchain for now (and we have the hackathon coming up soon too to work things out)..