Skip to content

Commit

Permalink
Fix RT #81381 - Make LWP::UserAgent robust to 5.17.6/5.18 hash random…
Browse files Browse the repository at this point in the history
…ization

Audited and fixed any potential hash order dependency bugs in
LWP::UserAgent. I replaced all the uses of each() with keys() to avoid
hash iterator state bugs, and made sure that keys are sorted where
their order might matter, which was as far as I could tell only in
how proxy configuration was read from the envrionment.

There was ambiguity as to which of $ENV{http_proxy} and
$ENV{HTTP_PROXY} would be chosen by LWP::UserAgent->env_proxy(). We now
choose HTTP_PROXY if both are set, and if they differ we warn about
the conflicting configuration.

This patch includes tests to check that we warn on conflicting config
and that we correctly handle setting via either.

This includes a version bump to 6.05.

See also:

libwww-perl RT Ticket
    https://rt.cpan.org/Ticket/Display.html?id=81381

bleadperl 5.17.6 patch (queued for 5.18):
    http://perl5.git.perl.org/perl.git/commit/7dc8663964c66a698d31bbdc8e8abed69bddeec3

Eliminating the "rehash" mechanism for 5.18
    http://www.nntp.perl.org/group/perl.perl5.porters/2012/10/msg194813.html

Switch perl's hash function to MurmurHash-32 (v3) and hash randomization by default.
    http://www.nntp.perl.org/group/perl.perl5.porters/2012/11/msg195492.html
  • Loading branch information
demerphq authored and oalders committed Dec 14, 2020
1 parent c9ee037 commit 607706f
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 19 deletions.
37 changes: 25 additions & 12 deletions lib/LWP/UserAgent.pm
Expand Up @@ -734,7 +734,8 @@ sub ssl_opts {
return $old;
}

return keys %{$self->{ssl_opts}};
my @opts= sort keys %{$self->{ssl_opts}};
return @opts;
}

sub parse_head {
Expand Down Expand Up @@ -886,9 +887,8 @@ sub get_my_handler {
$init->(\%spec);
}
elsif (ref($init) eq "HASH") {
while (my($k, $v) = each %$init) {
$spec{$k} = $v;
}
$spec{$_}= $init->{$_}
for keys %$init;
}
$spec{callback} ||= sub {};
$spec{line} ||= join(":", (caller)[1,2]);
Expand Down Expand Up @@ -1098,15 +1098,28 @@ sub env_proxy {
my ($self) = @_;
require Encode;
require Encode::Locale;
my($k,$v);
while(($k, $v) = each %ENV) {
if ($ENV{REQUEST_METHOD}) {
# Need to be careful when called in the CGI environment, as
# the HTTP_PROXY variable is under control of that other guy.
next if $k =~ /^HTTP_/;
$k = "HTTP_PROXY" if $k eq "CGI_HTTP_PROXY";
}
my $env_request_method= $ENV{REQUEST_METHOD};
my %seen;
foreach my $k (sort keys %ENV) {
my $real_key= $k;
my $v= $ENV{$k}
or next;
if ( $env_request_method ) {
# Need to be careful when called in the CGI environment, as
# the HTTP_PROXY variable is under control of that other guy.
next if $k =~ /^HTTP_/;
$k = "HTTP_PROXY" if $k eq "CGI_HTTP_PROXY";
}
$k = lc($k);
if (my $from_key= $seen{$k}) {
warn "Environment contains multiple differing definitions for '$k'.\n".
"Using value from '$from_key' ($ENV{$from_key}) and ignoring '$real_key' ($v)"
if $v ne $ENV{$from_key};
next;
} else {
$seen{$k}= $real_key;
}

next unless $k =~ /^(.*)_proxy$/;
$k = $1;
if ($k eq 'no') {
Expand Down
33 changes: 26 additions & 7 deletions t/base/ua.t
Expand Up @@ -4,8 +4,6 @@ use HTTP::Request ();
use LWP::UserAgent ();
use Test::More;

plan tests => 46;

# Prevent environment from interfering with test:
delete $ENV{PERL_LWP_SSL_VERIFY_HOSTNAME};
delete $ENV{HTTPS_CA_FILE};
Expand Down Expand Up @@ -164,11 +162,30 @@ is($ua->ssl_opts("verify_hostname"), 1, '$ua->ssl_opts("verify_hostname")');

delete @ENV{grep /_proxy$/i, keys %ENV}; # clean out any proxy vars

$ENV{http_proxy} = "http://example.com";
$ua = LWP::UserAgent->new;
is($ua->proxy('http'), undef, "\$ua->proxy('http')");
$ua = LWP::UserAgent->new(env_proxy => 1);;
is($ua->proxy('http'), "http://example.com", "\$ua->proxy('http')");
{
$ENV{HTTP_PROXY}= "http://example.com";
$ENV{http_proxy}= "http://otherexample.com";
my @warn;
local $SIG{__WARN__}= sub { my ($msg)= @_; $msg=~s/ at .*\z//s; push @warn, $msg };
# test that we get "HTTP_PROXY" when it is set and differs from "http_proxy".
$ua = LWP::UserAgent->new;
is($ua->proxy('http'), undef);
$ua = LWP::UserAgent->new(env_proxy => 1);
is($ua->proxy('http'), "http://example.com", q{proxy('http') returns URL});
is($warn[0],"Environment contains multiple differing definitions for 'http_proxy'.\n"
."Using value from 'HTTP_PROXY' (http://example.com) and ignoring 'http_proxy' (http://otherexample.com)");
}

# test that if only one of the two is set we can handle either.
for my $type ('http_proxy', 'HTTP_PROXY') {
delete $ENV{HTTP_PROXY};
delete $ENV{http_proxy};
$ENV{$type} = "http://example.com";
$ua = LWP::UserAgent->new;
is($ua->proxy('http'), undef, q{proxy('http') returns undef} );
$ua = LWP::UserAgent->new(env_proxy => 1);
is($ua->proxy('http'), "http://example.com", q{proxy('http') returns URL});
}

$ENV{PERL_LWP_ENV_PROXY} = 1;
$ua = LWP::UserAgent->new();
Expand All @@ -184,3 +201,5 @@ $ua = LWP::UserAgent->new(keep_alive => 0);
is($ua->conn_cache, undef, "\$ua->conn_cache");
$ua = LWP::UserAgent->new(keep_alive => 1);
is($ua->conn_cache->total_capacity, 1, "\$ua->conn_cache->total_capacity");

done_testing();

0 comments on commit 607706f

Please sign in to comment.