diff mbox series

fix download routines in Pakfire if behind upstream proxy

Message ID 20181018193854.7389-1-peter.mueller@link38.eu
State Superseded
Headers
Series fix download routines in Pakfire if behind upstream proxy |

Commit Message

Peter Müller Oct. 19, 2018, 6:38 a.m. UTC 
  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@link38.eu>
---
 src/pakfire/lib/functions.pl | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Michael Tremer Oct. 19, 2018, 11:51 p.m. UTC | #1 
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@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("");
diff mbox series

Patch

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'}/");
 			}
 		}
 
@@ -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");
+		}
 		
 		my $response = $ua->get($url, ':content_cb' => \&callback );
 		message("");