Control: tag -1 patch Control: forwarded -1 https://github.com/libwww-perl/libwww-perl/pull/342
On Mon, Jun 15, 2020 at 10:40:48PM +0300, Niko Tyni wrote: > The change in libwww-perl had a side effect of making it > look at max_redirect only when the Location header is set. > For the sake of robustness I suppose libwww-perl should be fixed, so > reassigning. Here's a fix that I've proposed upstream. -- Niko
>From 3460bfbf463d4c7eb33e956d4710dedfaf0c27d0 Mon Sep 17 00:00:00 2001 From: Niko Tyni <nt...@debian.org> Date: Mon, 15 Jun 2020 23:13:53 +0300 Subject: [PATCH] Fix a looping regression for 303 responses without a Location header The change in 6.45 had a side effect of making it look at max_redirect only when the Location header is set. The libapache2-mod-perl2 test suite implements a test server that answers with HTTP code 303 without including such a header. RFC 7231 is not very clear on whether Location is required, although the response doesn't make much sense without it. Some googling reveals this is not a new question. https://stackoverflow.com/questions/16194988/for-which-3xx-http-codes-is-the-location-header-mandatory For the sake of robustness I suppose libwww-perl needs to be fixed. Bug-Debian: https://bugs.debian.org/962904 --- lib/LWP/UserAgent.pm | 8 +++++--- t/local/http.t | 10 +++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/LWP/UserAgent.pm b/lib/LWP/UserAgent.pm index 047495fe..4cc1812a 100644 --- a/lib/LWP/UserAgent.pm +++ b/lib/LWP/UserAgent.pm @@ -300,10 +300,12 @@ sub request { my $response = $self->simple_request($request, $arg, $size); $response->previous($previous) if $previous; - if ($response->header('Location') && $response->redirects >= $self->{max_redirect}) { - $response->header("Client-Warning" => + if ($response->redirects >= $self->{max_redirect}) { + if ($response->header('Location')) { + $response->header("Client-Warning" => "Redirect loop detected (max_redirect = $self->{max_redirect})" - ); + ); + } return $response; } diff --git a/t/local/http.t b/t/local/http.t index 260f1ec3..0700d8e2 100644 --- a/t/local/http.t +++ b/t/local/http.t @@ -63,7 +63,7 @@ sub _test { return plan skip_all => 'We could not talk to our daemon' unless $DAEMON; return plan skip_all => 'No base URI' unless $base; - plan tests => 125; + plan tests => 127; my $ua = LWP::UserAgent->new; $ua->agent("Mozilla/0.01 " . $ua->agent); @@ -221,6 +221,13 @@ sub _test { is($res->redirects, 0, 'redirect loop: zero redirects'); $ua->max_redirect(5); is($ua->max_redirect(), 5, 'redirect loop: max redirects set back to 5'); + + # Test that redirects without a Location header work and don't loop + $req->uri(url("/redirect4", $base)); + $ua->max_redirect(5); + is($ua->max_redirect(), 5, 'redirect loop: max redirect 5'); + $res = $ua->request($req); + isa_ok($res, 'HTTP::Response', 'redirect loop: good response object'); } { # basic auth my $req = HTTP::Request->new(GET => url("/basic", $base)); @@ -622,6 +629,7 @@ sub daemonize { }; $router{get_redirect2} = sub { shift->send_redirect("/redirect3/") }; $router{get_redirect3} = sub { shift->send_redirect("/redirect2/") }; + $router{get_redirect4} = sub { my $r = HTTP::Response->new(303); shift->send_response($r) }; $router{post_echo} = sub { my($c,$r) = @_; $c->send_basic_header; -- 2.26.1
