Skip to content

Commit

Permalink
%ENV access hardening, use Test2::Util::_env_get() to access %ENV
Browse files Browse the repository at this point in the history
_env_get() ensures that if %ENV is locked we wont throw an exception
accessing a missing key. Tests might lock %ENV.
  • Loading branch information
demerphq committed Mar 14, 2023
1 parent 3d3fc82 commit 692606e
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 18 deletions.
5 changes: 3 additions & 2 deletions lib/Test/Tester.pm
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ BEGIN
use Test::Builder;
use Test::Tester::CaptureRunner;
use Test::Tester::Delegate;
use Test2::Util qw(_env_get);

require Exporter;

Expand All @@ -30,7 +31,7 @@ $Delegator->{Object} = $Test;

my $runner = Test::Tester::CaptureRunner->new;

my $want_space = $ENV{TESTTESTERSPACE};
my $want_space = _env_get('TESTTESTERSPACE');

sub show_space
{
Expand All @@ -40,7 +41,7 @@ sub show_space
my $colour = '';
my $reset = '';

if (my $want_colour = $ENV{TESTTESTERCOLOUR} || $ENV{TESTTESTERCOLOR})
if (my $want_colour = _env_get('TESTTESTERCOLOUR') || _env_get('TESTTESTERCOLOR'))
{
if (eval { require Term::ANSIColor; 1 })
{
Expand Down
6 changes: 3 additions & 3 deletions lib/Test2/API/Instance.pm
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ our @CARP_NOT = qw/Test2::API Test2::API::Instance Test2::IPC::Driver Test2::For
use Carp qw/confess carp/;
use Scalar::Util qw/reftype/;

use Test2::Util qw/get_tid USE_THREADS CAN_FORK pkg_to_file try CAN_SIGSYS/;
use Test2::Util qw/get_tid USE_THREADS CAN_FORK pkg_to_file try CAN_SIGSYS _env_get/;

use Test2::EventFacet::Trace();
use Test2::API::Stack();
Expand Down Expand Up @@ -104,7 +104,7 @@ sub post_preload_reset {

$self->{+FINALIZED} = undef;
$self->{+IPC} = undef;
$self->{+IPC_DISABLED} = $ENV{T2_NO_IPC} ? 1 : 0;
$self->{+IPC_DISABLED} = _env_get('T2_NO_IPC') ? 1 : 0;

$self->{+IPC_TIMEOUT} = DEFAULT_IPC_TIMEOUT() unless defined $self->{+IPC_TIMEOUT};

Expand All @@ -131,7 +131,7 @@ sub reset {

$self->{+FINALIZED} = undef;
$self->{+IPC} = undef;
$self->{+IPC_DISABLED} = $ENV{T2_NO_IPC} ? 1 : 0;
$self->{+IPC_DISABLED} = _env_get('T2_NO_IPC') ? 1 : 0;

$self->{+IPC_TIMEOUT} = DEFAULT_IPC_TIMEOUT() unless defined $self->{+IPC_TIMEOUT};

Expand Down
6 changes: 3 additions & 3 deletions lib/Test2/Event.pm
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use Carp qw/croak/;

use Test2::Util::HashBase qw/trace -amnesty uuid -_eid -hubs/;
use Test2::Util::ExternalMeta qw/meta get_meta set_meta delete_meta/;
use Test2::Util qw/pkg_to_file gen_uid/;
use Test2::Util qw/pkg_to_file gen_uid _env_get/;

use Test2::EventFacet::About();
use Test2::EventFacet::Amnesty();
Expand Down Expand Up @@ -294,7 +294,7 @@ sub nested {
my $self = shift;

Carp::cluck("Use of Test2::Event->nested() is deprecated, use Test2::Event->trace->nested instead")
if $ENV{AUTHOR_TESTING};
if _env_get('AUTHOR_TESTING');

if (my $hubs = $self->{+HUBS}) {
return $hubs->[0]->{nested} if @$hubs;
Expand All @@ -308,7 +308,7 @@ sub in_subtest {
my $self = shift;

Carp::cluck("Use of Test2::Event->in_subtest() is deprecated, use Test2::Event->trace->hid instead")
if $ENV{AUTHOR_TESTING};
if _env_get('AUTHOR_TESTING');

my $hubs = $self->{+HUBS};
if ($hubs && @$hubs) {
Expand Down
6 changes: 3 additions & 3 deletions lib/Test2/Formatter/TAP.pm
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use warnings;

our $VERSION = '1.302195';

use Test2::Util qw/clone_io/;
use Test2::Util qw/clone_io _env_get/;

use Test2::Util::HashBase qw{
no_numbers handles _encoding _last_fh
Expand Down Expand Up @@ -116,7 +116,7 @@ sub write {
my $io = $handles->[$hid] or next;

print $io "\n"
if $ENV{HARNESS_ACTIVE}
if _env_get('HARNESS_ACTIVE')
&& $hid == OUT_ERR
&& $self->{+_LAST_FH} != $io
&& $msg =~ m/^#\s*Failed( \(TODO\))? test /;
Expand Down Expand Up @@ -294,7 +294,7 @@ sub assert_tap {
# In a verbose harness we indent the extra since they will appear
# inside the subtest braces. This helps readability. In a non-verbose
# harness we do not do this because it is less readable.
if ($ENV{HARNESS_IS_VERBOSE} || !$ENV{HARNESS_ACTIVE}) {
if (_env_get("HARNESS_IS_VERBOSE") || !_env_get("HARNESS_ACTIVE")) {
$extra_indent = " ";
$extra_space = ' ';
}
Expand Down
14 changes: 7 additions & 7 deletions lib/Test2/IPC/Driver/Files.pm
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use Storable();
use File::Spec();
use POSIX();

use Test2::Util qw/try get_tid pkg_to_file IS_WIN32 ipc_separator do_rename do_unlink try_sig_mask/;
use Test2::Util qw/try get_tid pkg_to_file IS_WIN32 ipc_separator do_rename do_unlink try_sig_mask _env_get/;
use Test2::API qw/test2_ipc_set_pending/;

sub is_viable { 1 }
Expand All @@ -23,7 +23,7 @@ sub init {
my $self = shift;

my $tmpdir = File::Temp::tempdir(
$ENV{T2_TEMPDIR_TEMPLATE} || "test2" . ipc_separator . $$ . ipc_separator . "XXXXXX",
_env_get('T2_TEMPDIR_TEMPLATE') || "test2" . ipc_separator . $$ . ipc_separator . "XXXXXX",
CLEANUP => 0,
TMPDIR => 1,
);
Expand All @@ -33,7 +33,7 @@ sub init {
$self->{+TEMPDIR} = File::Spec->canonpath($tmpdir);

print STDERR "\nIPC Temp Dir: $tmpdir\n\n"
if $ENV{T2_KEEP_TEMPDIR};
if _env_get('T2_KEEP_TEMPDIR');

$self->{+EVENT_IDS} = {};
$self->{+READ_IDS} = {};
Expand Down Expand Up @@ -107,7 +107,7 @@ sub drop_hub {
$self->abort_trace("A hub file can only be closed by the thread that started it\nExpected $tid, got " . get_tid())
unless get_tid() == $tid;

if ($ENV{T2_KEEP_TEMPDIR}) {
if (_env_get('T2_KEEP_TEMPDIR')) {
my ($ok, $err) = do_rename($hfile, File::Spec->canonpath("$hfile.complete"));
$self->abort_trace("Could not rename file '$hfile' -> '$hfile.complete': $err") unless $ok
}
Expand Down Expand Up @@ -266,7 +266,7 @@ sub cull {
# Do not remove global events
next if $info->{global};

if ($ENV{T2_KEEP_TEMPDIR}) {
if (_env_get('T2_KEEP_TEMPDIR')) {
my $complete = File::Spec->canonpath("$full.complete");
my ($ok, $err) = do_rename($full, $complete);
$self->abort("Could not rename IPC file '$full', '$complete': $err") unless $ok;
Expand Down Expand Up @@ -406,7 +406,7 @@ sub DESTROY {
if ($aborted || $file =~ m/^(GLOBAL|HUB$sep)/) {
$full =~ m/^(.*)$/;
$full = $1; # Untaint it
next if $ENV{T2_KEEP_TEMPDIR};
next if _env_get('T2_KEEP_TEMPDIR');
my ($ok, $err) = do_unlink($full);
$self->abort("Could not unlink IPC file '$full': $err") unless $ok;
next;
Expand All @@ -416,7 +416,7 @@ sub DESTROY {
}
closedir($dh);

if ($ENV{T2_KEEP_TEMPDIR}) {
if (_env_get('T2_KEEP_TEMPDIR')) {
print STDERR "# Not removing temp dir: $tempdir\n";
return;
}
Expand Down

0 comments on commit 692606e

Please sign in to comment.