From 7448fde157c6d22f57b90c0cef11957b0aadf4b3 Mon Sep 17 00:00:00 2001 From: David Steele Date: Sun, 10 Mar 2024 11:36:39 +1300 Subject: [PATCH] Improved support for dual stack connections. Connections are established using the "happy eyeballs" approach from RFC 8305, i.e. new addresses (if available) are tried if the prior address has already had a reasonable time to connect. This prevents waiting too long on a failed connection but does not try all the addresses at once. Prior connections that are still waiting are rechecked periodically if no subsequent connection is successful. This improves substantially on 39bb8a0, which failed to take into account connection attempts that do not fail (but never connect) and use up all the available time. --- doc/xml/release/2024/2.51.xml | 13 ++ src/common/io/socket/address.c | 182 +++++++++++++++++++--- src/common/io/socket/address.h | 19 ++- src/common/io/socket/client.c | 224 ++++++++++++++++++++++++---- src/common/io/socket/common.c | 53 ------- src/common/io/socket/common.h | 3 - src/common/io/socket/server.c | 2 +- test/define.yaml | 4 + test/src/common/harnessSocket.c | 54 ++++++- test/src/common/harnessSocket.h | 3 + test/src/module/common/ioHttpTest.c | 8 + test/src/module/common/ioTlsTest.c | 182 +++++++++++++++++++--- 12 files changed, 617 insertions(+), 130 deletions(-) diff --git a/doc/xml/release/2024/2.51.xml b/doc/xml/release/2024/2.51.xml index e3e3ce088..7193ed823 100644 --- a/doc/xml/release/2024/2.51.xml +++ b/doc/xml/release/2024/2.51.xml @@ -34,6 +34,19 @@ + + + + + + + + + + +

Improved support for dual stack connections.

+ + diff --git a/src/common/io/socket/address.c b/src/common/io/socket/address.c index 8f92ca7ec..3573248ac 100644 --- a/src/common/io/socket/address.c +++ b/src/common/io/socket/address.c @@ -18,8 +18,25 @@ Object type struct AddressInfo { AddressInfoPub pub; // Publicly accessible variables + struct addrinfo *info; // Linked list of addresses }; +/*********************************************************************************************************************************** +Local variables +***********************************************************************************************************************************/ +// Store preferred addresses for hosts +typedef struct AddressInfoPreference +{ + const String *const host; // Host + String *address; // Preferred address for host +} AddressInfoPreference; + +static struct AddressInfoLocal +{ + MemContext *memContext; // Mem context + List *prefList; // List of preferred addresses for hosts +} addressInfoLocal; + /*********************************************************************************************************************************** Free addrinfo linked list allocated by getaddrinfo() ***********************************************************************************************************************************/ @@ -34,11 +51,90 @@ addrInfoFreeResource(THIS_VOID) ASSERT(this != NULL); - freeaddrinfo(*(struct addrinfo **)lstGet(this->pub.list, 0)); + freeaddrinfo(this->info); FUNCTION_LOG_RETURN_VOID(); } +/**********************************************************************************************************************************/ +FN_EXTERN void +addrInfoSort(AddressInfo *const this) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(ADDRESS_INFO, this); + FUNCTION_TEST_END(); + + // Only sort if there is more than one address + if (lstSize(this->pub.list) > 1) + { + // By default start with IPv6 and first address + int family = AF_INET6; + unsigned int addrIdx = 0; + + // If a preferred address is in the list then move it to the first position and update family + const AddressInfoPreference *const addrPref = lstFind(addressInfoLocal.prefList, &this->pub.host); + + if (addrPref != NULL) + { + AddressInfoItem *const addrFindItem = lstFind(this->pub.list, &addrPref->address); + + if (addrFindItem != NULL) + { + // Swap with first address if the address is not already first + AddressInfoItem *const addrFirstItem = lstGet(this->pub.list, 0); + + if (addrFirstItem != addrFindItem) + { + const AddressInfoItem addrCopyItem = *addrFirstItem; + *addrFirstItem = *addrFindItem; + *addrFindItem = addrCopyItem; + } + + // Set family and skip first address + family = addrFirstItem->info->ai_family == AF_INET6 ? AF_INET : AF_INET6; + addrIdx++; + } + } + + // Alternate IPv6 and IPv4 addresses + for (; addrIdx < lstSize(this->pub.list); addrIdx++) + { + AddressInfoItem *const addrItem = lstGet(this->pub.list, addrIdx); + + // If not the family we expect, search for one + if (addrItem->info->ai_family != family) + { + unsigned int addrFindIdx = addrIdx + 1; + + for (; addrFindIdx < lstSize(this->pub.list); addrFindIdx++) + { + AddressInfoItem *const addrFindItem = lstGet(this->pub.list, addrFindIdx); + + if (addrFindItem->info->ai_family == family) + { + // Swap addresses + const AddressInfoItem addrCopyItem = *addrItem; + *addrItem = *addrFindItem; + *addrFindItem = addrCopyItem; + + // Move on to next address + break; + } + } + + // Address of the required family not found so sorting is done + if (addrFindIdx == lstSize(this->pub.list)) + break; + } + + // Switch family + family = family == AF_INET6 ? AF_INET : AF_INET6; + } + } + + FUNCTION_TEST_RETURN_VOID(); +} + /**********************************************************************************************************************************/ FN_EXTERN AddressInfo * addrInfoNew(const String *const host, unsigned int port) @@ -51,6 +147,21 @@ addrInfoNew(const String *const host, unsigned int port) ASSERT(host != NULL); ASSERT(port != 0); + // Initialize mem context and preference list + if (addressInfoLocal.memContext == NULL) + { + MEM_CONTEXT_BEGIN(memContextTop()) + { + MEM_CONTEXT_NEW_BEGIN(AddressInfo, .childQty = MEM_CONTEXT_QTY_MAX) + { + addressInfoLocal.memContext = MEM_CONTEXT_NEW(); + addressInfoLocal.prefList = lstNewP(sizeof(AddressInfoPreference), .comparator = lstComparatorStr); + } + MEM_CONTEXT_NEW_END(); + } + MEM_CONTEXT_END(); + } + OBJ_NEW_BEGIN(AddressInfo, .childQty = MEM_CONTEXT_QTY_MAX, .callbackQty = 1) { *this = (AddressInfo) @@ -59,7 +170,7 @@ addrInfoNew(const String *const host, unsigned int port) { .host = strDup(host), .port = port, - .list = lstNewP(sizeof(struct addrinfo *)), + .list = lstNewP(sizeof(AddressInfoItem), .comparator = lstComparatorStr), }, }; @@ -79,23 +190,28 @@ addrInfoNew(const String *const host, unsigned int port) cvtUIntToZ(port, portZ, sizeof(portZ)); // Do the lookup - struct addrinfo *result; int error; - if ((error = getaddrinfo(strZ(host), portZ, &hints, &result)) != 0) + if ((error = getaddrinfo(strZ(host), portZ, &hints, &this->info)) != 0) THROW_FMT(HostConnectError, "unable to get address for '%s': [%d] %s", strZ(host), error, gai_strerror(error)); - // Set free callback to ensure address info is freed - memContextCallbackSet(objMemContext(this), addrInfoFreeResource, this); - // Convert address linked list to list - lstAdd(this->pub.list, &result); - - while (result->ai_next != NULL) + MEM_CONTEXT_OBJ_BEGIN(this->pub.list) { - lstAdd(this->pub.list, &result->ai_next); - result = result->ai_next; + struct addrinfo *info = this->info; + + lstAdd(this->pub.list, &(AddressInfoItem){.name = addrInfoToStr(info), .info = info}); + + // Set free callback to ensure address info is freed + memContextCallbackSet(objMemContext(this), addrInfoFreeResource, this); + + while (info->ai_next != NULL) + { + info = info->ai_next; + lstAdd(this->pub.list, &(AddressInfoItem){.name = addrInfoToStr(info), .info = info}); + } } + MEM_CONTEXT_OBJ_END(); } MEM_CONTEXT_TEMP_END(); } @@ -104,6 +220,41 @@ addrInfoNew(const String *const host, unsigned int port) FUNCTION_LOG_RETURN(ADDRESS_INFO, this); } +/**********************************************************************************************************************************/ +FN_EXTERN void +addrInfoPrefer(AddressInfo *this, unsigned int index) +{ + FUNCTION_LOG_BEGIN(logLevelDebug); + FUNCTION_LOG_PARAM(ADDRESS_INFO, this); + FUNCTION_LOG_PARAM(UINT, index); + FUNCTION_LOG_END(); + + AddressInfoPreference *const addrPref = lstFind(addressInfoLocal.prefList, &this->pub.host); + const String *const address = addrInfoGet(this, index)->name; + + MEM_CONTEXT_OBJ_BEGIN(addressInfoLocal.prefList) + { + // If no preference set yet + if (addrPref == NULL) + { + lstAdd(addressInfoLocal.prefList, &(AddressInfoPreference){.host = strDup(this->pub.host), .address = strDup(address)}); + lstSort(addressInfoLocal.prefList, sortOrderAsc); + } + // Else update address + else + { + // Free the old address + strFree(addrPref->address); + + // Assign new address + addrPref->address = strDup(address); + } + } + MEM_CONTEXT_OBJ_END(); + + FUNCTION_LOG_RETURN_VOID(); +} + /*********************************************************************************************************************************** Convert address to a zero-terminated string ***********************************************************************************************************************************/ @@ -166,21 +317,18 @@ addrInfoToStr(const struct addrinfo *const addrInfo) FN_EXTERN void addrInfoToLog(const AddressInfo *const this, StringStatic *const debugLog) { - char address[48]; - strStcFmt(debugLog, "{host: "); strToLog(addrInfoHost(this), debugLog); strStcFmt(debugLog, ", port: %u, list: [", addrInfoPort(this)); for (unsigned int listIdx = 0; listIdx < addrInfoSize(this); listIdx++) { - const struct addrinfo *const addrInfo = addrInfoGet(this, listIdx); + const AddressInfoItem *const addrItem = addrInfoGet(this, listIdx); if (listIdx != 0) strStcCat(debugLog, ", "); - addrInfoToZ(addrInfo, address, sizeof(address)); - strStcCat(debugLog, address); + strStcCat(debugLog, strZ(addrItem->name)); } strStcCat(debugLog, "]}"); diff --git a/src/common/io/socket/address.h b/src/common/io/socket/address.h index 82c448a8d..f52f54cca 100644 --- a/src/common/io/socket/address.h +++ b/src/common/io/socket/address.h @@ -25,6 +25,12 @@ FN_EXTERN AddressInfo *addrInfoNew(const String *const host, unsigned int port); /*********************************************************************************************************************************** Getters/Setters ***********************************************************************************************************************************/ +typedef struct AddressInfoItem +{ + String *name; // Address as a string + struct addrinfo *info; // Full address info +} AddressInfoItem; + typedef struct AddressInfoPub { const String *host; // Host for address info lookup @@ -33,10 +39,10 @@ typedef struct AddressInfoPub } AddressInfoPub; // Get address -FN_INLINE_ALWAYS const struct addrinfo * +FN_INLINE_ALWAYS const AddressInfoItem * addrInfoGet(const AddressInfo *const this, const unsigned int index) { - return *(const struct addrinfo **)lstGet(THIS_PUB(AddressInfo)->list, index); + return (const AddressInfoItem *)lstGet(THIS_PUB(AddressInfo)->list, index); } // Get lookup host @@ -60,6 +66,15 @@ addrInfoSize(const AddressInfo *const this) return lstSize(THIS_PUB(AddressInfo)->list); } +/*********************************************************************************************************************************** +Functions +***********************************************************************************************************************************/ +// Sort addresses alternating between IPv6 and IPv4 +FN_EXTERN void addrInfoSort(AddressInfo *this); + +// Set preferred address for host, which will be preferred in future lookups +FN_EXTERN void addrInfoPrefer(AddressInfo *this, unsigned int index); + /*********************************************************************************************************************************** Destructor ***********************************************************************************************************************************/ diff --git a/src/common/io/socket/client.c b/src/common/io/socket/client.c index 15f92c1bc..4a31c3f0a 100644 --- a/src/common/io/socket/client.c +++ b/src/common/io/socket/client.c @@ -11,6 +11,7 @@ Socket Client #include "common/debug.h" #include "common/error/retry.h" #include "common/io/client.h" +#include "common/io/fd.h" #include "common/io/socket/address.h" #include "common/io/socket/client.h" #include "common/io/socket/common.h" @@ -57,7 +58,64 @@ sckClientToLog(const THIS_VOID, StringStatic *const debugLog) #define FUNCTION_LOG_SOCKET_CLIENT_FORMAT(value, buffer, bufferSize) \ FUNCTION_LOG_OBJECT_FORMAT(value, sckClientToLog, buffer, bufferSize) -/**********************************************************************************************************************************/ +/*********************************************************************************************************************************** +Connections are established using the "happy eyeballs" approach from RFC 8305, i.e. new addresses (if available) are tried if the +prior address has already had a reasonable time to connect. This prevents waiting too long on a failed connection but does not try +all the addresses at once. Prior connections that are still waiting are rechecked periodically if no subsequent connection is +successful. +***********************************************************************************************************************************/ +typedef struct SckClientOpenData +{ + const struct addrinfo *const address; // IP address + const char *name; // Combination of host, address, port + int fd; // File descriptor for socket + int errNo; // Current error (may just indicate to keep waiting) +} SckClientOpenData; + +// Helper to determine if non-blocking connection has been established +static bool +sckClientOpenWait(SckClientOpenData *const openData, const TimeMSec timeout) +{ + ASSERT(openData != NULL); + + FUNCTION_LOG_BEGIN(logLevelTrace); + FUNCTION_LOG_PARAM(STRINGZ, openData->name); + FUNCTION_LOG_PARAM(INT, openData->fd); + FUNCTION_LOG_PARAM(INT, openData->errNo); + FUNCTION_LOG_PARAM(TIME_MSEC, timeout); + FUNCTION_LOG_END(); + + ASSERT(openData->name != NULL); + ASSERT(openData->fd > 0); + ASSERT(openData->errNo != 0); + + bool result = true; + + // The connection has started but since we are in non-blocking mode it has not completed yet + if (openData->errNo == EINPROGRESS || openData->errNo == EINTR) + { + // Wait for write-ready + if (fdReadyWrite(openData->fd, timeout)) + { + // Check for success or error. If the connection was successful this will set errNo to 0. + socklen_t errNoLen = sizeof(openData->errNo); + + THROW_ON_SYS_ERROR_FMT( + getsockopt(openData->fd, SOL_SOCKET, SO_ERROR, &openData->errNo, &errNoLen) == -1, HostConnectError, + "unable to get socket error for '%s'", openData->name); + } + // Else still waiting on connection + else + result = false; + } + + // Throw error if it is still set + if (result && openData->errNo != 0) + THROW_SYS_ERROR_CODE_FMT(openData->errNo, HostConnectError, "unable to connect to '%s'", openData->name); + + FUNCTION_LOG_RETURN(BOOL, result); +} + static IoSession * sckClientOpen(THIS_VOID) { @@ -77,47 +135,124 @@ sckClientOpen(THIS_VOID) ErrorRetry *const errRetry = errRetryNew(); bool retry; - // Get an address list for the host - const AddressInfo *const addrInfo = addrInfoNew(this->host, this->port); - unsigned int addrInfoIdx = 0; + // Get an address list for the host and sort it + AddressInfo *const addrInfo = addrInfoNew(this->host, this->port); + addrInfoSort(addrInfo); + + // Try addresses until success or timeout + List *const openDataList = lstNewP(sizeof(SckClientOpenData)); + volatile unsigned int addrInfoIdx = 0; do { // Assume there will be no retry + SckClientOpenData *volatile openData = NULL; + const ErrorType *errLastType = NULL; + String *errLastMessage = NULL; retry = false; - volatile int fd = -1; + // Try connection TRY_BEGIN() { - // Get next address for the host - const struct addrinfo *const addressFound = addrInfoGet(addrInfo, addrInfoIdx); - - // Connect to the host - fd = socket(addressFound->ai_family, addressFound->ai_socktype, addressFound->ai_protocol); - THROW_ON_SYS_ERROR(fd == -1, HostConnectError, "unable to create socket"); - - sckOptionSet(fd); - sckConnect(fd, this->host, this->port, addressFound, this->timeoutConnect); - - // Create the session - MEM_CONTEXT_PRIOR_BEGIN() + // Iterate waiting connections and see if any of them have completed + for (unsigned int openDataIdx = 0; openDataIdx < lstSize(openDataList); openDataIdx++) { - result = sckSessionNew(ioSessionRoleClient, fd, this->host, this->port, this->timeoutSession); - } - MEM_CONTEXT_PRIOR_END(); + SckClientOpenData *const openDataWait = lstGet(openDataList, openDataIdx); - // Update client name to include address - strTrunc(this->name); - strCat(this->name, addrInfoToName(this->host, this->port, addressFound)); + if (openDataWait->fd != 0 && sckClientOpenWait(openDataWait, 0)) + { + openData = openDataWait; + break; + } + } + + // Try or retry a connection since none of the waiting connections completed + if (openData == NULL) + { + // If connection does not exist yet then create it + if (addrInfoIdx == lstSize(openDataList)) + { + openData = lstAdd(openDataList, &(SckClientOpenData){.address = addrInfoGet(addrInfo, addrInfoIdx)->info}); + openData->name = strZ(addrInfoToName(this->host, this->port, openData->address)); + } + // Else search for a connection that is not waiting + else + { + for (; addrInfoIdx < lstSize(openDataList); addrInfoIdx++) + { + SckClientOpenData *const openDataNoWait = lstGet(openDataList, addrInfoIdx); + + if (openDataNoWait->fd == 0) + { + openData = openDataNoWait; + break; + } + } + } + + // Attempt connection to host + if (openData != NULL) + { + openData->fd = socket( + openData->address->ai_family, openData->address->ai_socktype, openData->address->ai_protocol); + THROW_ON_SYS_ERROR(openData->fd == -1, HostConnectError, "unable to create socket"); + + sckOptionSet(openData->fd); + + if (connect(openData->fd, openData->address->ai_addr, openData->address->ai_addrlen) == -1) + { + // Save the error and wait for connection. The initial wait time will be fixed at 250ms, the default + // value recommended by RFC 8305. + openData->errNo = errno; + + sckClientOpenWait(openData, 250); + } + } + } + + // Connection was successful so open session and set result + if (openData != NULL && openData->errNo == 0) + { + // Create the session + MEM_CONTEXT_PRIOR_BEGIN() + { + result = sckSessionNew(ioSessionRoleClient, openData->fd, this->host, this->port, this->timeoutSession); + } + MEM_CONTEXT_PRIOR_END(); + + // Update client name to include address + strTrunc(this->name); + strCatZ(this->name, openData->name); + + // Set preferred address + addrInfoPrefer(addrInfo, addrInfoIdx); + + // Clear socket so it will not be freed later + openData->fd = 0; + } } CATCH_ANY() { // Close socket - close(fd); + ASSERT(openData != NULL); + close(openData->fd); + + // Clear socket so the connection can be retried + openData->fd = 0; + openData->errNo = 0; // Add the error retry info errRetryAddP(errRetry); + // Store error info for later logging + errLastType = errorType(); + errLastMessage = strNewZ(errorMessage()); + } + TRY_END(); + + // If the connection was not completed then wait and/or retry + if (result == NULL) + { // Increment address info index and reset if the end has been reached. When the end has been reached sleep for a bit // to hopefully have better chance at succeeding, otherwise continue right to the next address. addrInfoIdx++; @@ -130,18 +265,47 @@ sckClientOpen(THIS_VOID) else retry = true; - // Error when no retries remain + // General timeout errors when no retries remain if (!retry) - THROWP(errRetryType(errRetry), strZ(errRetryMessage(errRetry))); + { + for (unsigned int openDataIdx = 0; openDataIdx < lstSize(openDataList); openDataIdx++) + { + SckClientOpenData *const openDataTimeout = lstGet(openDataList, openDataIdx); - // Log retry - LOG_DEBUG_FMT("retry %s: %s", errorTypeName(errorType()), errorMessage()); - statInc(SOCKET_STAT_RETRY_STR); + if (openDataTimeout->fd != 0) + { + errRetryAddP( + errRetry, .type = &HostConnectError, + .message = strNewFmt("timeout connecting to '%s'", openDataTimeout->name)); + } + } + } + + // Log retry when there was an error + if (errLastType != NULL) + { + LOG_DEBUG_FMT("retry %s: %s", errorTypeName(errLastType), strZ(errLastMessage)); + strFree(errLastMessage); + + statInc(SOCKET_STAT_RETRY_STR); + } } - TRY_END(); } while (retry); + // Free any connections that were in progress but never completed + for (unsigned int openDataIdx = 0; openDataIdx < lstSize(openDataList); openDataIdx++) + { + SckClientOpenData *const openDataFree = lstGet(openDataList, openDataIdx); + + if (openDataFree->fd != 0) + close(openDataFree->fd); + } + + // Error when no result + if (result == NULL) + THROWP(errRetryType(errRetry), strZ(errRetryMessage(errRetry))); + statInc(SOCKET_STAT_SESSION_STR); } MEM_CONTEXT_TEMP_END(); diff --git a/src/common/io/socket/common.c b/src/common/io/socket/common.c index d41312d3b..0fc905c32 100644 --- a/src/common/io/socket/common.c +++ b/src/common/io/socket/common.c @@ -9,7 +9,6 @@ Socket Common Functions #include #include "common/debug.h" -#include "common/io/fd.h" #include "common/io/socket/address.h" #include "common/io/socket/common.h" #include "common/log.h" @@ -137,55 +136,3 @@ sckOptionSet(int fd) FUNCTION_TEST_RETURN_VOID(); } - -/**********************************************************************************************************************************/ -static bool -sckConnectInProgress(int errNo) -{ - return errNo == EINPROGRESS || errNo == EINTR; -} - -FN_EXTERN void -sckConnect(int fd, const String *host, unsigned int port, const struct addrinfo *hostAddress, TimeMSec timeout) -{ - FUNCTION_LOG_BEGIN(logLevelTrace); - FUNCTION_LOG_PARAM(INT, fd); - FUNCTION_LOG_PARAM(STRING, host); - FUNCTION_LOG_PARAM(UINT, port); - FUNCTION_LOG_PARAM_P(VOID, hostAddress); - FUNCTION_LOG_PARAM(TIME_MSEC, timeout); - FUNCTION_LOG_END(); - - ASSERT(host != NULL); - ASSERT(hostAddress != NULL); - - // Attempt connection - if (connect(fd, hostAddress->ai_addr, hostAddress->ai_addrlen) == -1) - { - // Save the error - int errNo = errno; - - // The connection has started but since we are in non-blocking mode it has not completed yet - if (sckConnectInProgress(errNo)) - { - // Wait for write-ready - if (!fdReadyWrite(fd, timeout)) - THROW_FMT(HostConnectError, "timeout connecting to '%s'", strZ(addrInfoToName(host, port, hostAddress))); - - // Check for success or error. If the connection was successful this will set errNo to 0. - socklen_t errNoLen = sizeof(errNo); - - THROW_ON_SYS_ERROR( - getsockopt(fd, SOL_SOCKET, SO_ERROR, &errNo, &errNoLen) == -1, HostConnectError, "unable to get socket error"); - } - - // Throw error if it is still set - if (errNo != 0) - { - THROW_SYS_ERROR_CODE_FMT( - errNo, HostConnectError, "unable to connect to '%s'", strZ(addrInfoToName(host, port, hostAddress))); - } - } - - FUNCTION_LOG_RETURN_VOID(); -} diff --git a/src/common/io/socket/common.h b/src/common/io/socket/common.h index 5b9a734ef..8d2d17c70 100644 --- a/src/common/io/socket/common.h +++ b/src/common/io/socket/common.h @@ -18,7 +18,4 @@ FN_EXTERN void sckInit(bool block, bool keepAlive, int tcpKeepAliveCount, int tc // Set options on a socket FN_EXTERN void sckOptionSet(int fd); -// Connect socket to an IP address -FN_EXTERN void sckConnect(int fd, const String *host, unsigned int port, const struct addrinfo *hostAddress, TimeMSec timeout); - #endif diff --git a/src/common/io/socket/server.c b/src/common/io/socket/server.c index 17ff7736d..c59ac62a9 100644 --- a/src/common/io/socket/server.c +++ b/src/common/io/socket/server.c @@ -165,7 +165,7 @@ sckServerNew(const String *const address, const unsigned int port, const TimeMSe }; // Lookup address - const struct addrinfo *const addressFound = addrInfoGet(addrInfoNew(this->address, this->port), 0); + const struct addrinfo *const addressFound = addrInfoGet(addrInfoNew(this->address, this->port), 0)->info; // Create socket THROW_ON_SYS_ERROR( diff --git a/test/define.yaml b/test/define.yaml index 9334c6316..54895973e 100644 --- a/test/define.yaml +++ b/test/define.yaml @@ -371,6 +371,7 @@ unit: common/io/socket/client: function: - sckClientOpen + - sckClientOpenWait coverage: - common/io/client @@ -404,6 +405,9 @@ unit: - common/io/http/session - common/io/http/url + include: + - common/io/socket/address + # ---------------------------------------------------------------------------------------------------------------------------- - name: exec total: 2 diff --git a/test/src/common/harnessSocket.c b/test/src/common/harnessSocket.c index 3c1cfd91e..c262dac95 100644 --- a/test/src/common/harnessSocket.c +++ b/test/src/common/harnessSocket.c @@ -3,7 +3,6 @@ Harness for Io Testing ***********************************************************************************************************************************/ #include "build.auto.h" -#include "common/harnessConfig.h" #include "common/harnessDebug.h" #include "common/harnessSocket.h" @@ -19,8 +18,61 @@ static struct { // Shim clientOpen() to use a fake file descriptor bool localShimSckClientOpen; + + // Shim sckClientOpenWait() to return false for an address one time + const char *clientOpenWaitAddress; } hrnIoStatic; +/**********************************************************************************************************************************/ +void +hrnSckClientOpenWaitShimInstall(const char *const address) +{ + FUNCTION_HARNESS_BEGIN(); + FUNCTION_HARNESS_PARAM(STRINGZ, address); + FUNCTION_HARNESS_END(); + + hrnIoStatic.clientOpenWaitAddress = address; + + FUNCTION_HARNESS_RETURN_VOID(); +} + +/*********************************************************************************************************************************** +Shim sckClientOpen() +***********************************************************************************************************************************/ +static bool +sckClientOpenWait(SckClientOpenData *const openData, const TimeMSec timeout) +{ + ASSERT(openData != NULL); + + FUNCTION_HARNESS_BEGIN(); + FUNCTION_HARNESS_PARAM(STRINGZ, openData->name); + FUNCTION_HARNESS_PARAM(INT, openData->fd); + FUNCTION_HARNESS_PARAM(INT, openData->errNo); + FUNCTION_HARNESS_PARAM(TIME_MSEC, timeout); + FUNCTION_HARNESS_END(); + + bool result = sckClientOpenWait_SHIMMED(openData, timeout); + + // If this is the shimmed address then return false and update error code + if (hrnIoStatic.clientOpenWaitAddress != NULL) + { + String *const address = addrInfoToStr(openData->address); + + if (strcmp(hrnIoStatic.clientOpenWaitAddress, strZ(address)) == 0) + { + result = false; + openData->errNo = EINPROGRESS; + + // Reset the shim + hrnIoStatic.clientOpenWaitAddress = NULL; + } + + strFree(address); + } + + FUNCTION_HARNESS_RETURN(BOOL, result); +} + /*********************************************************************************************************************************** Shim sckClientOpen() ***********************************************************************************************************************************/ diff --git a/test/src/common/harnessSocket.h b/test/src/common/harnessSocket.h index 361cdc8f3..01b567837 100644 --- a/test/src/common/harnessSocket.h +++ b/test/src/common/harnessSocket.h @@ -11,6 +11,9 @@ Constants /*********************************************************************************************************************************** Functions ***********************************************************************************************************************************/ +// Install shim for sckClientOpenWait() to return false for an address one time +void hrnSckClientOpenWaitShimInstall(const char *address); + // Install/uninstall shim for clientOpen() to use a fake file descriptor void hrnSckClientOpenShimInstall(void); void hrnSckClientOpenShimUninstall(void); diff --git a/test/src/module/common/ioHttpTest.c b/test/src/module/common/ioHttpTest.c index 223791df8..80ccaacc4 100644 --- a/test/src/module/common/ioHttpTest.c +++ b/test/src/module/common/ioHttpTest.c @@ -5,6 +5,7 @@ Test HTTP #include "common/io/fdRead.h" #include "common/io/fdWrite.h" +#include "common/io/socket/address.h" #include "common/io/socket/client.h" #include "common/io/tls/client.h" @@ -25,6 +26,13 @@ testRun(void) { FUNCTION_HARNESS_VOID(); + // Ensure that the ipv4 loopback address will be selected + const String *const ipLoop4 = STRDEF("127.0.0.1"); + AddressInfo *addrInfo = NULL; + + TEST_ASSIGN(addrInfo, addrInfoNew(STRDEF("localhost"), 443), "localhost addr list"); + TEST_RESULT_VOID(addrInfoPrefer(addrInfo, lstFindIdx(addrInfo->pub.list, &ipLoop4)), "prefer 127.0.0.1"); + // ***************************************************************************************************************************** if (testBegin("httpUriEncode() and httpUriDecode()")) { diff --git a/test/src/module/common/ioTlsTest.c b/test/src/module/common/ioTlsTest.c index eb1ed052e..8a87d9e44 100644 --- a/test/src/module/common/ioTlsTest.c +++ b/test/src/module/common/ioTlsTest.c @@ -155,20 +155,22 @@ testRun(void) if (testBegin("AddressInfo")) { #ifdef TEST_CONTAINER_REQUIRED - HRN_SYSTEM("echo \"127.0.0.1 test-addr-loop.pgbackrest.org\" | sudo tee -a /etc/hosts > /dev/null"); - HRN_SYSTEM("echo \"::1 test-addr-loop.pgbackrest.org\" | sudo tee -a /etc/hosts > /dev/null"); + #define TEST_ADDR_LOOP_HOST "test-addr-loop.pgbackrest.org" + + HRN_SYSTEM("echo \"127.0.0.1 " TEST_ADDR_LOOP_HOST "\" | sudo tee -a /etc/hosts > /dev/null"); + HRN_SYSTEM("echo \"::1 " TEST_ADDR_LOOP_HOST "\" | sudo tee -a /etc/hosts > /dev/null"); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("lookup address info"); AddressInfo *addrInfo = NULL; - TEST_ASSIGN(addrInfo, addrInfoNew(STRDEF("test-addr-loop.pgbackrest.org"), 443), "addr list"); - TEST_RESULT_STR_Z(addrInfoHost(addrInfo), "test-addr-loop.pgbackrest.org", "check host"); + TEST_ASSIGN(addrInfo, addrInfoNew(STRDEF(TEST_ADDR_LOOP_HOST), 443), "addr list"); + TEST_RESULT_STR_Z(addrInfoHost(addrInfo), TEST_ADDR_LOOP_HOST, "check host"); TEST_RESULT_UINT(addrInfoPort(addrInfo), 443, "check port"); TEST_RESULT_UINT(addrInfoSize(addrInfo), 2, "check size"); // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("addrInfoToLog"); + TEST_TITLE("addrInfoToLog()"); char logBuf[STACK_TRACE_PARAM_MAX]; @@ -176,21 +178,94 @@ testRun(void) TEST_RESULT_Z( logBuf, zNewFmt( - "{host: {\"test-addr-loop.pgbackrest.org\"}, port: 443, list: [%s, %s]}", - strZ(addrInfoToStr(addrInfoGet(addrInfo, 0))), strZ(addrInfoToStr(addrInfoGet(addrInfo, 1)))), + "{host: {\"" TEST_ADDR_LOOP_HOST "\"}, port: 443, list: [%s, %s]}", + strZ(addrInfoGet(addrInfo, 0)->name), strZ(addrInfoGet(addrInfo, 1)->name)), "check log"); + addrInfoSort(addrInfo); + + TEST_RESULT_VOID(FUNCTION_LOG_OBJECT_FORMAT(addrInfo, addrInfoToLog, logBuf, sizeof(logBuf)), "addrInfoToLog"); + TEST_RESULT_Z(logBuf, "{host: {\"" TEST_ADDR_LOOP_HOST "\"}, port: 443, list: [::1, 127.0.0.1]}", "check log"); + // Munge address so it is invalid - (*(struct addrinfo **)lstGet(addrInfo->pub.list, 0))->ai_addr = NULL; + ((AddressInfoItem *)lstGet(addrInfo->pub.list, 0))->info->ai_addr = NULL; + TEST_RESULT_STR_Z(addrInfoToStr(addrInfoGet(addrInfo, 0)->info), "invalid", "check invalid"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("addrInfoSort() break early"); + + struct addrinfo addr4 = {.ai_family = AF_INET}; + struct addrinfo addr6 = {.ai_family = AF_INET6}; + + MEM_CONTEXT_OBJ_BEGIN(addrInfo) + { + addrInfo->pub.host = strNewZ("test"); + lstClear(addrInfo->pub.list); + + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("127.0.0.1"), .info = &addr4}); + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("::1"), .info = &addr6}); + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("127.0.0.2"), .info = &addr4}); + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("::2"), .info = &addr6}); + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("::3"), .info = &addr6}); + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("::4"), .info = &addr6}); + addrInfoSort(addrInfo); + } + MEM_CONTEXT_OBJ_END(); TEST_RESULT_VOID(FUNCTION_LOG_OBJECT_FORMAT(addrInfo, addrInfoToLog, logBuf, sizeof(logBuf)), "addrInfoToLog"); TEST_RESULT_Z( - logBuf, - zNewFmt( - "{host: {\"test-addr-loop.pgbackrest.org\"}, port: 443, list: [invalid, %s]}", - strZ(addrInfoToStr(addrInfoGet(addrInfo, 1)))), - "check log"); - TEST_RESULT_STR_Z(addrInfoToStr(addrInfoGet(addrInfo, 0)), "invalid", "check invalid"); + logBuf, zNewFmt("{host: {\"test\"}, port: 443, list: [::1, 127.0.0.1, ::2, 127.0.0.2, ::3, ::4]}"), "check log"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("addrInfoSort()"); + + // Set a preference that won't be found + addrInfoPrefer(addrInfo, 5); + + MEM_CONTEXT_OBJ_BEGIN(addrInfo) + { + lstClear(addrInfo->pub.list); + + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("127.0.0.1"), .info = &addr4}); + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("127.0.0.2"), .info = &addr4}); + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("::1"), .info = &addr6}); + lstAdd(addrInfo->pub.list, &(AddressInfoItem){.name = strNewZ("::2"), .info = &addr6}); + addrInfoSort(addrInfo); + } + MEM_CONTEXT_OBJ_END(); + + TEST_RESULT_VOID(FUNCTION_LOG_OBJECT_FORMAT(addrInfo, addrInfoToLog, logBuf, sizeof(logBuf)), "addrInfoToLog"); + TEST_RESULT_Z( + logBuf, zNewFmt("{host: {\"test\"}, port: 443, list: [::1, 127.0.0.2, ::2, 127.0.0.1]}"), "check log"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("addrInfoSort() prefer v4"); + + addrInfoPrefer(addrInfo, 1); + addrInfoSort(addrInfo); + + TEST_RESULT_VOID(FUNCTION_LOG_OBJECT_FORMAT(addrInfo, addrInfoToLog, logBuf, sizeof(logBuf)), "addrInfoToLog"); + TEST_RESULT_Z( + logBuf, zNewFmt("{host: {\"test\"}, port: 443, list: [127.0.0.2, ::1, 127.0.0.1, ::2]}"), "check log"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("addrInfoSort() prefer v4 (already first)"); + + addrInfoSort(addrInfo); + + TEST_RESULT_VOID(FUNCTION_LOG_OBJECT_FORMAT(addrInfo, addrInfoToLog, logBuf, sizeof(logBuf)), "addrInfoToLog"); + TEST_RESULT_Z( + logBuf, zNewFmt("{host: {\"test\"}, port: 443, list: [127.0.0.2, ::1, 127.0.0.1, ::2]}"), "check log"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("addrInfoSort() prefer v6"); + + addrInfoPrefer(addrInfo, 3); + addrInfoSort(addrInfo); + + TEST_RESULT_VOID(FUNCTION_LOG_OBJECT_FORMAT(addrInfo, addrInfoToLog, logBuf, sizeof(logBuf)), "addrInfoToLog"); + TEST_RESULT_Z( + logBuf, zNewFmt("{host: {\"test\"}, port: 443, list: [::2, 127.0.0.1, ::1, 127.0.0.2]}"), "check log"); // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("free"); @@ -343,11 +418,6 @@ testRun(void) TEST_ERROR( ioClientOpen(socketClient), HostConnectError, "unable to connect to '127.0.0.1:7777': [111] Connection refused"); socketLocal.block = false; - - // --------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("uncovered conditions for sckConnect()"); - - TEST_RESULT_BOOL(sckConnectInProgress(EINTR), true, "connection in progress (EINTR)"); } FINALLY() { @@ -365,6 +435,11 @@ testRun(void) if (testBegin("SocketClient/SocketServer")) { IoClient *client = NULL; + AddressInfo *addrInfo = NULL; + const String *const ipLoop4 = STRDEF("127.0.0.1"); + + TEST_ASSIGN(addrInfo, addrInfoNew(STRDEF("localhost"), 443), "localhost addr list"); + TEST_RESULT_VOID(addrInfoPrefer(addrInfo, lstFindIdx(addrInfo->pub.list, &ipLoop4)), "prefer 127.0.0.1"); TEST_ASSIGN(client, sckClientNew(STRDEF("localhost"), HRN_SERVER_PORT_BOGUS, 100, 100), "new client"); TEST_ERROR( @@ -374,10 +449,71 @@ testRun(void) // This address should not be in use in a test environment -- if it is the test will fail TEST_ASSIGN(client, sckClientNew(STRDEF("172.31.255.255"), HRN_SERVER_PORT_BOGUS, 100, 100), "new client"); - TEST_ERROR( - ioClientOpen(client), HostConnectError, - "timeout connecting to '172.31.255.255:34342'\n" - "[RETRY DETAIL OMITTED]"); + TEST_ERROR(ioClientOpen(client), HostConnectError, "timeout connecting to '172.31.255.255:34342'"); + + // ------------------------------------------------------------------------------------------------------------------------- +#ifdef TEST_CONTAINER_REQUIRED + #define TEST_ADDR_CONN_HOST "test-addr-conn.pgbackrest.org" + + HRN_SYSTEM("echo \"127.0.0.1 " TEST_ADDR_CONN_HOST "\" | sudo tee -a /etc/hosts > /dev/null"); + HRN_SYSTEM("echo \"::1 " TEST_ADDR_CONN_HOST "\" | sudo tee -a /etc/hosts > /dev/null"); + HRN_SYSTEM("echo \"172.31.255.255 " TEST_ADDR_CONN_HOST "\" | sudo tee -a /etc/hosts > /dev/null"); + + // Explicitly set the order of the address list for the test below. The first IP must not respond, the second must error, + // and only the last will succeed. + const String *ipPrefer = STRDEF("172.31.255.255"); + TEST_ASSIGN(addrInfo, addrInfoNew(STRDEF(TEST_ADDR_CONN_HOST), 443), "localhost addr list"); + TEST_RESULT_VOID(addrInfoPrefer(addrInfo, lstFindIdx(addrInfo->pub.list, &ipPrefer)), "prefer 172.31.255.255"); + + TEST_ASSIGN(addrInfo, addrInfoNew(STRDEF(TEST_ADDR_CONN_HOST), 443), "localhost addr list"); + TEST_RESULT_VOID(addrInfoSort(addrInfo), "sort addresses"); + TEST_RESULT_STR(addrInfoToStr(addrInfoGet(addrInfo, 0)->info), ipPrefer, "first 172.31.255.255"); + TEST_RESULT_STR_Z(addrInfoToStr(addrInfoGet(addrInfo, 1)->info), "::1", "second ::1"); + TEST_RESULT_STR_Z(addrInfoToStr(addrInfoGet(addrInfo, 2)->info), "127.0.0.1", "third 127.0.0.1"); + + HRN_FORK_BEGIN() + { + const unsigned int testPort = hrnServerPortNext(); + + HRN_FORK_CHILD_BEGIN(.prefix = "test server", .timeout = 5000) + { + TEST_RESULT_VOID(hrnServerRunP(HRN_FORK_CHILD_READ(), hrnServerProtocolSocket, testPort), "socket server"); + } + HRN_FORK_CHILD_END(); + + HRN_FORK_PARENT_BEGIN() + { + IoWrite *server = hrnServerScriptBegin(HRN_FORK_PARENT_WRITE(0)); + IoClient *client; + IoSession *session; + + // ----------------------------------------------------------------------------------------------------------------- + TEST_TITLE("connect to 127.0.0.1 when ::1 errors and 172.31.255.255 never responds"); + + TEST_ASSIGN(client, sckClientNew(STRDEF(TEST_ADDR_CONN_HOST), testPort, 1000, 1000), "new client"); + + hrnServerScriptAccept(server); + hrnServerScriptClose(server); + + // Shim the server address to return false one time for write ready. This tests connections that take longer. + hrnSckClientOpenWaitShimInstall("127.0.0.1"); + + TEST_ASSIGN(session, ioClientOpen(client), "connection established"); + TEST_RESULT_VOID( + sckClientOpenWait( + &(SckClientOpenData){.name = "test", .fd = ((SocketSession *)session->pub.driver)->fd, .errNo = EINTR}, 99), + "check EINTR wait condition"); + TEST_RESULT_VOID(ioSessionFree(session), "connection closed"); + + // ----------------------------------------------------------------------------------------------------------------- + TEST_TITLE("end server process"); + + hrnServerScriptEnd(server); + } + HRN_FORK_PARENT_END(); + } + HRN_FORK_END(); +#endif // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("sckServerAccept() returns NULL on interrupt");