libwww-perl / WWW-Mechanize

Handy web browsing in a Perl object
https://metacpan.org/pod/WWW::Mechanize
Other
68 stars 52 forks source link

untaint.t test fails if environment variable Test is defined #376

Closed dbcooper closed 6 months ago

dbcooper commented 6 months ago

The LWP::UserAgent (6.76) module's environment variable case sensitivity warning[^1] causes WWW::Mechanize's (2.17) untaint.t test to fail if the environment variable Test is defined. I assume it will also break on other capitalization variants as long as it's not entirely lowercase (test).

[WWW-Mechanize-2.17]$ perl -MLWP::UserAgent -e 'print $LWP::UserAgent::VERSION,"\n";'
6.76
[WWW-Mechanize-2.17]$ Test=foo perl -Ilib t/untaint.t
1..2
ok 1 - use WWW::Mechanize;
Environment contains multiple differing definitions for 'test'.
Using value from 'Test' (foo) and ignoring 'test' (14) at /usr/local/perlbrew/perls/perl-5.36.3/lib/site_perl/5.36.3/LWP/UserAgent.pm line 1172.
not ok 2 - no (unexpected) warnings (via END block)
#   Failed test 'no (unexpected) warnings (via END block)'
#   at /usr/local/perlbrew/perls/perl-5.36.3/lib/5.36.3/Test/Builder.pm line 193.
# Looks like you failed 1 test of 2.
[WWW-Mechanize-2.17]$

YMMV if you consider this a bug. However, I doubt I'm the only one who might initiate a module install/build w/ an environment variable named "TEST".

~I also ran into another [different] Perl module test failure due to the foobar environment variable being defined, but I'll have to lookup that link on a different computer.~

Edit: Apparently my fuzzy memory really is fuzzy 🙄. The environment variable was actually foo and it caused Perl build to fail. https://www.spec.org/cpu2006/Docs/errata.html#problem21

[^1]: I'm not necessarily agreeing w/ the annoying part but I saw some background on it at libwww-perl/libwww-perl#372 .

oalders commented 6 months ago

I think the line $ENV{test} = 14; could be removed entirely from that test. Having said that, I'm not sure we need this test file at all.

oalders commented 6 months ago

@petdance any thoughts on this? Is this test still useful?

petdance commented 6 months ago

I'm guessing that I wrote t/untaint.t with the intent of adding a feature to untaint external data, but I don't know. Thirteen years is a while back.

t/taint.t seems to be covering the taintedness feature, and I don't see that t/untaint.t adds any value.