From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] fix download routines in Pakfire if behind upstream proxy
Date: Fri, 19 Oct 2018 13:51:51 +0100
Message-ID: <e8c35de655f0c40fed500c3e500c8e009c40051c.camel@ipfire.org>
In-Reply-To: <20181018193854.7389-1-peter.mueller@link38.eu>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1518644417367262465=="
List-Id: <development.lists.ipfire.org>

--===============1518644417367262465==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi,

this should be two patches.

On Thu, 2018-10-18 at 21:38 +0200, Peter M=C3=BCller wrote:
> Using an array for setting both HTTP and HTTPS proxy settings in
> functions.pl does not seem to work, the queries are still transferred
> directly.
>=20
> Setting proxies with two code lines is boilerplate-style, but
> works much more robust.
>=20
> Returned file size is now checked against values indicating
> download errors (0 or empty), avoiding successful returns after
> failed downloads.
>=20
> Partially fixes #11900
>=20
> Signed-off-by: Peter M=C3=BCller <peter.mueller(a)link38.eu>
> ---
>  src/pakfire/lib/functions.pl | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>=20
> diff --git a/src/pakfire/lib/functions.pl b/src/pakfire/lib/functions.pl
> index 12a405bd7..291a111b9 100644
> --- a/src/pakfire/lib/functions.pl
> +++ b/src/pakfire/lib/functions.pl
> @@ -157,10 +157,12 @@ sub fetchfile {
>  		if ($proxysettings{'UPSTREAM_PROXY'}) {
>  			logger("DOWNLOAD INFO: Upstream proxy:
> \"$proxysettings{'UPSTREAM_PROXY'}\"");
>  			if ($proxysettings{'UPSTREAM_USER'}) {
> -				$ua->proxy([["http", "https"] =3D>
> "http://$proxysettings{'UPSTREAM_USER'}:$proxysettings{'UPSTREAM_PASSWORD'}=
@".
> "$proxysettings{'UPSTREAM_PROXY'}/"]);
> +				$ua->proxy("http" =3D>
> "http://$proxysettings{'UPSTREAM_USER'}:$proxysettings{'UPSTREAM_PASSWORD'}=
\@$
> proxysettings{'UPSTREAM_PROXY'}/");
> +				$ua->proxy("https" =3D>
> "http://$proxysettings{'UPSTREAM_USER'}:$proxysettings{'UPSTREAM_PASSWORD'}=
\@$
> proxysettings{'UPSTREAM_PROXY'}/");
>  				logger("DOWNLOAD INFO: Logging in with:
> \"$proxysettings{'UPSTREAM_USER'}\" -
> \"$proxysettings{'UPSTREAM_PASSWORD'}\"");
>  			} else {
> -				$ua->proxy([["http", "https"] =3D>
> "http://$proxysettings{'UPSTREAM_PROXY'}/"]);
> +				$ua->proxy("http" =3D>
> "http://$proxysettings{'UPSTREAM_PROXY'}/");
> +				$ua->proxy("https" =3D>
> "http://$proxysettings{'UPSTREAM_PROXY'}/");
>  			}
>  		}

I tested it, but interesting to know. Probably would have also been elegant to
use a small loop here.=20

> @@ -180,7 +182,14 @@ sub fetchfile {
>  		my $result =3D $ua->head($url);
>  		my $remote_headers =3D $result->headers;
>  		$total_size =3D $remote_headers->content_length;
> -		logger("DOWNLOAD INFO: $file has size of $total_size bytes");
> +
> +		# validate if file download was successful (size <=3D 0)
> +		if ( $total_size eq "0" || not $total_size ) {
> +			logger("DOWNLOAD ERROR: download of $file failed with
> size '$total_size' bytes");
> +			return 1;
> +		} else {
> +			logger("DOWNLOAD INFO: $file has size of $total_size
> bytes");
> +		}

This solves a different problem and should therefore be a different patch.

Best,
-Michael

>  	=09
>  		my $response =3D $ua->get($url, ':content_cb' =3D> \&callback );
>  		message("");


--===============1518644417367262465==--