[PATCH] opkg_download: add https proxy support

[Mathias Kunert]

The environment variable 'https_proxy' is required by libcurl to use a
proxy for https targets.
The patch also uses CURLOPT_PROXYUSERNAME and CURLOPT_PROXYPASSWORD
instead of CURLOPT_PROXYUSERPWD to set the user credentials for the
proxy.

Signed-off-by: Mathias Kunert <mathias...@dezem.de>
---
libopkg/opkg_conf.c | 1 +
libopkg/opkg_conf.h | 1 +
libopkg/opkg_download.c | 15 +++++++++------
3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/libopkg/opkg_conf.c b/libopkg/opkg_conf.c
index 843f509..779906e 100644
--- a/libopkg/opkg_conf.c
+++ b/libopkg/opkg_conf.c
@@ -60,6 +60,7 @@ static opkg_option_t options[] = {
{ "check_signature", OPKG_OPT_TYPE_BOOL, &_conf.check_signature },
{ "ftp_proxy", OPKG_OPT_TYPE_STRING, &_conf.ftp_proxy },
{ "http_proxy", OPKG_OPT_TYPE_STRING, &_conf.http_proxy },
+ { "https_proxy", OPKG_OPT_TYPE_STRING, &_conf.https_proxy },
{ "no_proxy", OPKG_OPT_TYPE_STRING, &_conf.no_proxy },
{ "test", OPKG_OPT_TYPE_BOOL, &_conf.noaction },
{ "noaction", OPKG_OPT_TYPE_BOOL, &_conf.noaction },
diff --git a/libopkg/opkg_conf.h b/libopkg/opkg_conf.h
index 87a4d84..5b40d2c 100644
--- a/libopkg/opkg_conf.h
+++ b/libopkg/opkg_conf.h
@@ -122,6 +122,7 @@ typedef struct opkg_conf {
/* proxy options */
char *http_proxy;
+ char *https_proxy;
char *ftp_proxy;
char *no_proxy;
char *proxy_user;
diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c
index 48be76d..cbde790 100644
--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c
@@ -365,6 +365,11 @@ opkg_download_internal(const char *src, const char *dest,
opkg_config->http_proxy);
setenv("http_proxy", opkg_config->http_proxy, 1);
}
+ if (opkg_config->https_proxy) {
+ opkg_msg(DEBUG, "Setting environment variable: https_proxy = %s.\n",
+ opkg_config->https_proxy);
+ setenv("https_proxy", opkg_config->https_proxy, 1);
+ }
if (opkg_config->ftp_proxy) {
opkg_msg(DEBUG, "Setting environment variable: ftp_proxy = %s.\n",
opkg_config->ftp_proxy);
@@ -880,13 +885,11 @@ opkg_curl_init(curl_progress_func cb, void *data)
curl_easy_setopt (curl, CURLOPT_FOLLOWLOCATION, 1);
curl_easy_setopt (curl, CURLOPT_FAILONERROR, 1);
- if (opkg_config->http_proxy || opkg_config->ftp_proxy)
+ if (opkg_config->http_proxy || opkg_config->ftp_proxy || opkg_config->https_proxy)
{
- char *userpwd;
- sprintf_alloc (&userpwd, "%s:%s", opkg_config->proxy_user,
- opkg_config->proxy_passwd);
- curl_easy_setopt(curl, CURLOPT_PROXYUSERPWD, userpwd);
- free (userpwd);
+ curl_easy_setopt(curl, CURLOPT_PROXYUSERNAME, opkg_config->proxy_user);
+ curl_easy_setopt(curl, CURLOPT_PROXYPASSWORD, opkg_config->proxy_passwd);
+ curl_easy_setopt(curl, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
}
}
-- 1.8.3.2

[Paul Barker]

We should really be checking the return value of setenv() for error.
If you have time, a further patch (after this one is merged) to add
this checking to this and the other setenv call(s) in this function
would be very welcome!

> + }
> if (opkg_config->ftp_proxy) {
> opkg_msg(DEBUG, "Setting environment variable: ftp_proxy = %s.\n",
> opkg_config->ftp_proxy);
> @@ -880,13 +885,11 @@ opkg_curl_init(curl_progress_func cb, void *data)
> curl_easy_setopt (curl, CURLOPT_FOLLOWLOCATION, 1);
> curl_easy_setopt (curl, CURLOPT_FAILONERROR, 1);
> - if (opkg_config->http_proxy || opkg_config->ftp_proxy)
> + if (opkg_config->http_proxy || opkg_config->ftp_proxy || opkg_config->https_proxy)
> {
> - char *userpwd;
> - sprintf_alloc (&userpwd, "%s:%s", opkg_config->proxy_user,
> - opkg_config->proxy_passwd);
> - curl_easy_setopt(curl, CURLOPT_PROXYUSERPWD, userpwd);
> - free (userpwd);
> + curl_easy_setopt(curl, CURLOPT_PROXYUSERNAME, opkg_config->proxy_user);
> + curl_easy_setopt(curl, CURLOPT_PROXYPASSWORD, opkg_config->proxy_passwd);
> + curl_easy_setopt(curl, CURLOPT_PROXYAUTH, CURLAUTH_ANY);

Could you give me a very brief explanation of why this bit of the
change is needed? It would be better to split this change into a
separate patch so we have one patch which adds https_proxy support and
one which changes the way CURLOPT_* values are set for proxies, with
an explanation in the commit message of why that is needed.

> }
> }
> -- 1.8.3.2
>
> --
> You received this message because you are subscribed to the Google Groups "opkg-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opkg-devel+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Many thanks,

--
Paul Barker

Email: pa...@paulbarker.me.uk
http://www.paulbarker.me.uk

[Mathias Kunert]

Using CURLOPT_PROXYUSERNAME and CURLOPT_PROXYPASSWORD
instead of CURLOPT_PROXYUSERPWD makes the calls to sprintf_alloc/free
redundant. This also avoids the splitting of the colon-separated
username/password string inside libcurl.
CURLOPT_PROXYUSERNAME allows the username to contain a colon.

I will create separated patches for it. I also noticed that
CURLOPT_FOLLOWLOCATION is set in opkg_download_internal and in
opkg_curl_init. I suggest to set curl properties taken from opkg_config only
in opkg_curl_init. I will also create a patch for that.

Regards,
Mathias Kunert

[Mathias Kunert]

The environment variable 'https_proxy' is required by libcurl to use a
proxy for https targets.

Signed-off-by: Mathias Kunert <mathias...@dezem.de>
---
libopkg/opkg_conf.c | 1 +
libopkg/opkg_conf.h | 1 +

libopkg/opkg_download.c | 7 ++++++-
3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/libopkg/opkg_conf.c b/libopkg/opkg_conf.c
index 843f509..779906e 100644
--- a/libopkg/opkg_conf.c
+++ b/libopkg/opkg_conf.c
@@ -60,6 +60,7 @@ static opkg_option_t options[] = {
{ "check_signature", OPKG_OPT_TYPE_BOOL, &_conf.check_signature },
{ "ftp_proxy", OPKG_OPT_TYPE_STRING, &_conf.ftp_proxy },
{ "http_proxy", OPKG_OPT_TYPE_STRING, &_conf.http_proxy },
+ { "https_proxy", OPKG_OPT_TYPE_STRING, &_conf.https_proxy },
{ "no_proxy", OPKG_OPT_TYPE_STRING, &_conf.no_proxy },
{ "test", OPKG_OPT_TYPE_BOOL, &_conf.noaction },
{ "noaction", OPKG_OPT_TYPE_BOOL, &_conf.noaction },
diff --git a/libopkg/opkg_conf.h b/libopkg/opkg_conf.h
index 87a4d84..5b40d2c 100644
--- a/libopkg/opkg_conf.h
+++ b/libopkg/opkg_conf.h
@@ -122,6 +122,7 @@ typedef struct opkg_conf {
/* proxy options */
char *http_proxy;
+ char *https_proxy;
char *ftp_proxy;
char *no_proxy;
char *proxy_user;
diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c

index 48be76d..d051be2 100644

--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c
@@ -365,6 +365,11 @@ opkg_download_internal(const char *src, const char *dest,
opkg_config->http_proxy);
setenv("http_proxy", opkg_config->http_proxy, 1);
}
+ if (opkg_config->https_proxy) {
+ opkg_msg(DEBUG, "Setting environment variable: https_proxy = %s.\n",
+ opkg_config->https_proxy);
+ setenv("https_proxy", opkg_config->https_proxy, 1);
+ }
if (opkg_config->ftp_proxy) {
opkg_msg(DEBUG, "Setting environment variable: ftp_proxy = %s.\n",
opkg_config->ftp_proxy);

@@ -880,7 +885,7 @@ opkg_curl_init(curl_progress_func cb, void *data)

curl_easy_setopt (curl, CURLOPT_FOLLOWLOCATION, 1);
curl_easy_setopt (curl, CURLOPT_FAILONERROR, 1);
- if (opkg_config->http_proxy || opkg_config->ftp_proxy)
+ if (opkg_config->http_proxy || opkg_config->ftp_proxy || opkg_config->https_proxy)
{

char *userpwd;

sprintf_alloc (&userpwd, "%s:%s", opkg_config->proxy_user,

--
1.8.3.2

[Mathias Kunert]

Replace CURLOPT_PROXYUSERPWD with CURLOPT_PROXYUSERNAME and
CURLOPT_PROXYPASSWORD. In comparison to CURLOPT_PROXYUSERPWD the

CURLOPT_PROXYUSERNAME allows the username to contain a colon.

This also makes the calls to sprintf_alloc/free redundant. The

splitting of the colon-separated username/password string inside

libcurl is also avoided.
Set CURLOPT_PROXYAUTH to CURLAUTH_ANY to let libcurl pick any
authentication method it finds suitable.

Signed-off-by: Mathias Kunert <mathias...@dezem.de>
---

libopkg/opkg_download.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c
index d051be2..cbde790 100644
--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c
@@ -887,11 +887,9 @@ opkg_curl_init(curl_progress_func cb, void *data)
curl_easy_setopt (curl, CURLOPT_FAILONERROR, 1);

[Mathias Kunert]

Moved curl_easy_setopt for CURLOPT_CONNECTTIMEOUT_MS, CURLOPT_TIMEOUT_MS and
CURLOPT_FOLLOWLOCATION to opkg_curl_init.

Signed-off-by: Mathias Kunert <mathias...@dezem.de>
---

libopkg/opkg_download.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c
index cbde790..e605946 100644
--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c
@@ -413,19 +413,6 @@ opkg_download_internal(const char *src, const char *dest,
}
#endif /* HAVE_SSLCURL */
- if (opkg_config->connect_timeout_ms > 0) {
- long timeout_ms = opkg_config->connect_timeout_ms;
- curl_easy_setopt (curl, CURLOPT_CONNECTTIMEOUT_MS, timeout_ms);
- }
-
- if (opkg_config->transfer_timeout_ms > 0) {
- long timeout_ms = opkg_config->transfer_timeout_ms;
- curl_easy_setopt (curl, CURLOPT_TIMEOUT_MS, timeout_ms);
- }
-
- if (opkg_config->follow_location)
- curl_easy_setopt (curl, CURLOPT_FOLLOWLOCATION, 1);
-
if (use_cache) {
ret = opkg_validate_cached_file(src, dest);
if (ret <= 0)
@@ -883,7 +870,19 @@ opkg_curl_init(curl_progress_func cb, void *data)
}
#endif
+ if (opkg_config->connect_timeout_ms > 0) {
+ long timeout_ms = opkg_config->connect_timeout_ms;
+ curl_easy_setopt (curl, CURLOPT_CONNECTTIMEOUT_MS, timeout_ms);
+ }
+
+ if (opkg_config->transfer_timeout_ms > 0) {
+ long timeout_ms = opkg_config->transfer_timeout_ms;
+ curl_easy_setopt (curl, CURLOPT_TIMEOUT_MS, timeout_ms);
+ }
+
+ if (opkg_config->follow_location)
curl_easy_setopt (curl, CURLOPT_FOLLOWLOCATION, 1);
+
curl_easy_setopt (curl, CURLOPT_FAILONERROR, 1);

if (opkg_config->http_proxy || opkg_config->ftp_proxy || opkg_config->https_proxy)
{

--
1.8.3.2

[Paul Barker]

On 22 April 2014 14:13, Mathias Kunert <mathias...@dezem.de> wrote:
> The environment variable 'https_proxy' is required by libcurl to use a
> proxy for https targets.
>
> Signed-off-by: Mathias Kunert <mathias...@dezem.de>

I can't seem to get any of these patches to apply, they look to have
been corrupted. Could you use 'git send-email' to resend them or send
them as attachments rather than inline?

All 3 patches look good and just need a quick test here, they should
be good to merge as soon as I can get them to apply.

Thanks,

[Mathias Kunert]

The environment variable 'https_proxy' is required by libcurl to use a
proxy for https targets.

Signed-off-by: Mathias Kunert <mathias...@dezem.de>

[Mathias Kunert]

Replace CURLOPT_PROXYUSERPWD with CURLOPT_PROXYUSERNAME and
CURLOPT_PROXYPASSWORD. In comparison to CURLOPT_PROXYUSERPWD the
CURLOPT_PROXYUSERNAME allows the username to contain a colon.
This also makes the calls to sprintf_alloc/free redundant. The
splitting of the colon-separated username/password string inside
libcurl is also avoided.
Set CURLOPT_PROXYAUTH to CURLAUTH_ANY to let libcurl pick any
authentication method it finds suitable.

Signed-off-by: Mathias Kunert <mathias...@dezem.de>
---

libopkg/opkg_download.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c
index d051be2..cbde790 100644
--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c
@@ -887,11 +887,9 @@ opkg_curl_init(curl_progress_func cb, void *data)
curl_easy_setopt (curl, CURLOPT_FAILONERROR, 1);

if (opkg_config->http_proxy || opkg_config->ftp_proxy || opkg_config->https_proxy)
{

[Mathias Kunert]

Moved curl_easy_setopt for CURLOPT_CONNECTTIMEOUT_MS, CURLOPT_TIMEOUT_MS and
CURLOPT_FOLLOWLOCATION to opkg_curl_init.

Signed-off-by: Mathias Kunert <mathias...@dezem.de>
---

libopkg/opkg_download.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c
index cbde790..e605946 100644
--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c

if (opkg_config->http_proxy || opkg_config->ftp_proxy || opkg_config->https_proxy)
{

--
1.8.3.2

[Paul Barker]

On 23 April 2014 10:40, Paul Barker <pa...@paulbarker.me.uk> wrote:
> On 22 April 2014 14:13, Mathias Kunert <mathias...@dezem.de> wrote:
>> The environment variable 'https_proxy' is required by libcurl to use a
>> proxy for https targets.
>>
>> Signed-off-by: Mathias Kunert <mathias...@dezem.de>
>
> I can't seem to get any of these patches to apply, they look to have
> been corrupted. Could you use 'git send-email' to resend them or send
> them as attachments rather than inline?
>
> All 3 patches look good and just need a quick test here, they should
> be good to merge as soon as I can get them to apply.
>
> Thanks,
>

Ping.

These changes look good, I'd like to get them merged, just need them
in a form git can apply.

[Mathias Kunert]

Hello,

I sent the patches again using 'git send-email' on 24.04.2014 starting
with Message-Id:
<1398351098-8551-1-git-s...@dezem.de>. These
patches should work now.

Regards,
--
Mathias Kunert

[Paul Barker]

Very strange, gmail seems to have let me down there. I've grabbed them
from the Google groups web interface. They apply fine and pass the
test suite so I've merged them to master.

Cheers,