Re: [PATCH] fix download routines in Pakfire if behind upstream proxy

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 2870 bytes --]

Hi,

this should be two patches.

On Thu, 2018-10-18 at 21:38 +0200, Peter Müller 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.
> 
> Setting proxies with two code lines is boilerplate-style, but
> works much more robust.
> 
> Returned file size is now checked against values indicating
> download errors (0 or empty), avoiding successful returns after
> failed downloads.
> 
> Partially fixes #11900
> 
> Signed-off-by: Peter Müller <peter.mueller(a)link38.eu>
> ---
>  src/pakfire/lib/functions.pl | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> 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"] =>
> "http://$proxysettings{'UPSTREAM_USER'}:$proxysettings{'UPSTREAM_PASSWORD'}@".
> "$proxysettings{'UPSTREAM_PROXY'}/"]);
> +				$ua->proxy("http" =>
> "http://$proxysettings{'UPSTREAM_USER'}:$proxysettings{'UPSTREAM_PASSWORD'}\@$
> proxysettings{'UPSTREAM_PROXY'}/");
> +				$ua->proxy("https" =>
> "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"] =>
> "http://$proxysettings{'UPSTREAM_PROXY'}/"]);
> +				$ua->proxy("http" =>
> "http://$proxysettings{'UPSTREAM_PROXY'}/");
> +				$ua->proxy("https" =>
> "http://$proxysettings{'UPSTREAM_PROXY'}/");
>  			}
>  		}

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

> @@ -180,7 +182,14 @@ sub fetchfile {
>  		my $result = $ua->head($url);
>  		my $remote_headers = $result->headers;
>  		$total_size = $remote_headers->content_length;
> -		logger("DOWNLOAD INFO: $file has size of $total_size bytes");
> +
> +		# validate if file download was successful (size <= 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

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