Fix socket creation when APR_UNSPEC is used

[uhliarik]

When APR_UNSPEC is used, socket family is automaticaly set to APR_INET6 in apr_socket_create() and therefore if apr_sockaddr_info_get() finds out that the given address is in IPv4 format, it fails when calling apr_socket_connect(). So we first call apr_sockaddr_info_get() to get info about addr family and then pass it to apr_socket_create().

[uhliarik]

Follow up to https://svn.apache.org/repos/asf/apr/apr/trunk@1907242

[uhliarik]

Reproducer:

run:

$ ./memcachecon 127.0.0.1 11211
$ ./memcachecon ::1 11211

#include <apr_memcache.h>

#include <stdlib.h>

int main(int argc, char **argv)
{
    apr_pool_t *pglobal;
    apr_status_t rv;
    char *mcversion;
    char *errstr = malloc(1024);

    apr_app_initialize(NULL, NULL, NULL);

    if (apr_pool_create(&pglobal, NULL)){
        fprintf(stderr, "could not create global pool");
        return 1;
    }

    if (argc != 3){
        fprintf(stderr, "Usage: ./connconnectipv6 HOST PORT\n");
        return 2;
    }

    apr_memcache_server_t *ms;

    rv = apr_memcache_server_create(pglobal, argv[1], atoi(argv[2]), 0, 2, 2, 60, &ms);
    if (rv != APR_SUCCESS){.
        fprintf(stderr, "Errno: %d - %s\n", errno, apr_strerror(errno, errstr, 1024));
        return 3;
    }

    rv = apr_memcache_version(ms, pglobal, &mcversion);
    if (rv != APR_SUCCESS){.
       fprintf(stderr, "Errno: %d - %s\n", errno, apr_strerror(errno, errstr, 1024));
       return 5;
    }

    fprintf(stdout, "memcached version: %s\n", mcversion);

    apr_pool_destroy(pglobal);
    free(errstr)

    return 0;
}

[notroj]

Ah, hmm. I think this is still incorrect. The proper way is to loop over the returned addresses until a connect() succeeds. e.g. this:

https://github.com/apache/httpd/blob/66254273e22a5b4051d15d7423b86a5d884b4ee5/modules/ssl/ssl_util_ocsp.c#L105

[uhliarik]

Ah, hmm. I think this is still incorrect. The proper way is to loop over the returned addresses until a connect() succeeds. e.g. this:

https://github.com/apache/httpd/blob/66254273e22a5b4051d15d7423b86a5d884b4ee5/modules/ssl/ssl_util_ocsp.c#L105

Ohh, I thought this job is done in apr_sockaddr_info_get --> find_addresses --> call_resolver():

https://github.com/apache/apr/blob/trunk/network_io/unix/sockaddr.c#L446

But this will only go through the list of addresses until the first AF_INET/AF_INET6 address is found. In case we pass hostname to it, it will indeed try only the first returned address which is not correct approach. I will to alter this patch and add the part where it will loop through all returned addresses until a connect() succeeds.

[uhliarik]

Altered the way how conn_connect() creates a socket connection. It should go now through all addresses returned by apr_sockaddr_info_get().

[notroj]

This needs to catch the error case where no loop iteration succeeds, if sa is NULL after the for() loop then it should return with an error.

[rpluem]

Shouldn't the below directly after the for() loop fix this?

Suggested change

	}
	}
	if (rv != APR_SUCCESS) {
	return rv;
	}

[uhliarik]

This would also work IMHO. I think, since we set apr_socket_timeout_set on closed socket, it would fail anyway... But it is the better way how to handle it - explicitly say that we were not able to connect to any of given addresses.

[notroj]

Thanks Lubos!