Re: [Tails-dev] minitube (Youtube client)

Delete this message

Reply to this message
Author: intrigeri
Date:  
To: The Tails public development discussion list
Subject: Re: [Tails-dev] minitube (Youtube client)
Hi,

Alessandro Grassi wrote (07 Dec 2012 13:07:30 GMT) :
> That was easy. Here's the patch, please test!


It seems to work for me in the context of Tails, congrats :)

See bellow for a minimal code review.

> --- minitube/src/global.h    2012-09-27 10:17:03.000000000 +0000
> +++ global.h    2012-12-07 12:45:16.000000000 +0000
> +            // parse username and password
> +            if (socks5_server.contains(QChar('@'))) {

[...]
> +            // parse hostname and port
> +            QStringList socks5_server_list = socks5_server.split(QChar(':'));


I'm worried to see this done by hand. Hasn't Qt (or some other widely
available library) functions to do so safely?

(Yeah, I've seen this was copy'n'pasted from upstream code who does
the same for HTTP proxy, but I can't say it makes me any less
worried...)

>      QNetworkAccessManager* networkAccessManager() {
>          if (!nam) {
> -            networkHttpProxySetting();
> -            maybeSetSystemProxy();
> +            if( std::getenv("socks5_server") || std::getenv("SOCKS5_SERVER") ) {
> +                networkSocks5ProxySetting();
> +            } else if ( std::getenv("http_proxy") || std::getenv("HTTP_PROXY") ) {
> +                networkHttpProxySetting();
> +            } else {
> +                maybeSetSystemProxy();
> +            }
>              nam = new QNetworkAccessManager();
>          }
>          return nam;


It seems like the maybeSetSystemProxy() is now called only if neither
a SOCKS5 nor a HTTP proxy is configured, which effectively ignores
"system" settings for specific URLs. If this change of behaviour is
needed, then it should be documented; and if it's not needed, then it
should be kept out. What do you think?

Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc