Too many redirects error when redirected to https only in axel compiled without ssl

[ghost]

Axel, when compiled without ssl and passed a http url argument that redirects to a https url, exits with Too many redirects. error.

Steps to reproduce the bug:

Configure axel with --without-ssl option and compile.

$ axel http://www.archlinux.org/releng/releases/2018.04.01/torrent/
Too many redirects.

Curl output to get an idea of the redirections :

$ curl -I -L http://www.archlinux.org/releng/releases/2018.04.01/torrent/
HTTP/1.1 301 Moved Permanently
Server: nginx/1.12.2
Date: Tue, 24 Apr 2018 10:38:13 GMT
Content-Type: text/html
Content-Length: 185
Connection: keep-alive
Location: https://www.archlinux.org/releng/releases/2018.04.01/torrent/

HTTP/1.1 200 OK
Server: nginx/1.12.2
Date: Tue, 24 Apr 2018 10:38:15 GMT
Content-Type: application/x-bittorrent
Content-Length: 37216
Connection: keep-alive
...

[ghost]

@ismaell I have an idea on where the bug exists, please correct me if i am wrong.

I believe when conn_set() fails here, the value of conn remains the same url instead of the redirected url and conn_exec() is performed on the same url till max redirects limit is reached, and then axel exits with Too many redirects. error.

This can be easily corrected by a test for failure of conn_set()

[ghost]

On further perusal i believe presence or absence of SSL support in axel should not have any influence on parsing a well made URL as done here in conn_set() function.

Instead it can be elegantly handled after parsing it in conn_set() by testing conn->proto for secure protocol like:

#ifndef HAVE_SSL
if (PROTO_IS_SECURE(conn->proto)) {
    sprintf(conn->message, _("Unsupported protocol\n"));
    /* return function with some negative value */
}
#endif

Is this the right way to do this ? of course failure of conn_set() needs to be handled too.

[ismaell]

@shankar your reasoning seems right.

Please note:

• The guard in the URL you mentioned prevents us from using PROTO_IS_SECURE without HAVE_SSL; that could be fixed, though.

• We should avoid #ifdefs in the future outside the realm of header files, so it could be:

  if (!HAVE_SSL && PROTO_IS_SECURE(conn->proto)) {
  ...

[ismaell]

The reason to avoid #ifdefs is that the code gets compiled and verified even if optimized-out, so we guarantee consistency.

[ghost]

@ismaell if we compile with --without-ssl option , HAVE_SSL macro would not be defined and

if (!HAVE_SSL && PROTO_IS_SECURE(conn->proto)) {
  ...

would not compile. Please advice

[ghost]

And also i have been thinking about what i said about handling secure protocol outside conn_set() when SSL support is absent. I was wrong. Since we would have to handle secure protocol everytime conn_set() is called, we might as well have it within conn_set(), the way it has already been done.

[ghost]

@ismaell here is my patch to fix the issue ( i have taken the liberty to improve error message)

diff --git a/src/conn.c b/src/conn.c
index 3acdf67..591d18b 100644
--- a/src/conn.c
+++ b/src/conn.c
@@ -70,7 +70,6 @@ conn_set(conn_t *conn, const char *set_url)
                        conn->proto = PROTO_HTTP;
                        conn->port = PROTO_HTTP_PORT;
                }
-#ifdef HAVE_SSL
                else if (strncmp(set_url, "ftps", proto_len) == 0) {
                        conn->proto = PROTO_FTPS;
                        conn->port = PROTO_FTPS_PORT;
@@ -78,10 +77,17 @@ conn_set(conn_t *conn, const char *set_url)
                        conn->proto = PROTO_HTTPS;
                        conn->port = PROTO_HTTPS_PORT;
                }
-#endif                         /* HAVE_SSL */
                else {
+                       sprintf(conn->message, _("Unsupported protocol\n"));
                        return 0;
                }
+#ifndef HAVE_SSL
+               if (PROTO_IS_SECURE(conn->proto)) {
+                       sprintf(conn->message,
+                               _("Secure protocol is not supported\n"));
+                       return 0;
+               }
+#endif
                strncpy(url, i + 3, sizeof(url) - 1);
                url[sizeof(url) - 1] = '\0';
        }
@@ -352,7 +358,10 @@ conn_info(conn_t *conn)
                                strncpy(s, conn->http->headers, sizeof(s) - 1);
                        }
                        s[sizeof(s) - 1] = '\0';
-                       conn_set(conn, s);
+
+                       if (!conn_set(conn, s)) {
+                               return 0;
+                       }

                        /* check if the download has been redirected to FTP and
                         * report it back to the caller */

Did it do it right ? do i need to make changes ?

[ismaell]

@shankar looks good; instead of "secure protocol is not supported" it could say "SSL support disabled".

Send a PR and I'll merge it.