From b015f74bed510aa6d3f6c8e1454784d6346b98c1 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Wed, 29 Nov 2023 21:01:02 -0600 Subject: [PATCH] Bugfix: f_network_from_ip_string() and f_network_to_ip_string() design problems. Change the inet_pton() and inet_ntop() function calls to use the explicit .v4 and .v6 union members. Being explicit is much safer. The inet_pton() function also returns zero without the errno set when the address is invalid. Handle this case, returning F_address_not (with error bit) when zero is returned by inet_pton(). The unit tests for these functions are incorrect and are not all called (which is why their incorrectness was not previously discovered). Make sure to use h_errno where appropriate. Make sure the mock function for inet_pton() allows for returning zero or non-zero. Make sure the family type is set so that the expected return codes are returned. Make sure the ip string is set so that the expected return codes are returned. Handle the newly added F_address_not case. Add the missing unit test function calls: - test__f_network_from_ip_address__fails - test__f_network_from_ip_name__fails - test__f_network_from_ip_string__fails - test__f_network_to_ip_string__fails This could use more review and testing. Such extra review will be performed while I continue to write TacocaT. --- level_0/f_network/c/network.c | 31 ++++++++++++++++------ level_0/f_network/c/network.h | 1 + level_0/f_network/tests/unit/c/mock-network.c | 6 ++--- .../tests/unit/c/test-network-from_ip_address.c | 20 ++++++++++---- .../tests/unit/c/test-network-from_ip_name.c | 4 ++- .../tests/unit/c/test-network-from_ip_string.c | 22 ++++++++++++++- .../tests/unit/c/test-network-to_ip_string.c | 18 ++++++++++--- level_0/f_network/tests/unit/c/test-network.c | 6 +++++ 8 files changed, 86 insertions(+), 22 deletions(-) diff --git a/level_0/f_network/c/network.c b/level_0/f_network/c/network.c index 7724647..fb0018f 100644 --- a/level_0/f_network/c/network.c +++ b/level_0/f_network/c/network.c @@ -92,13 +92,22 @@ extern "C" { if (!from.used || to->type == f_network_family_none_e) return F_data_not; - if (inet_pton(to->type == f_network_family_ip_4_e ? AF_INET : AF_INET6, from.string, (void *) & to->address) == -1) { - if (errno == EAFNOSUPPORT) return F_status_set_error(F_support_not); + { + const int result = (to->type == f_network_family_ip_4_e) + ? inet_pton(AF_INET, from.string, (void *) & to->address.v4) + : inet_pton(AF_INET6, from.string, (void *) & to->address.v6); - return F_status_set_error(F_failure); + if (result == -1) { + if (errno == EAFNOSUPPORT) return F_status_set_error(F_support_not); + + return F_status_set_error(F_failure); + } + else if (result) { + return F_okay; + } } - return F_okay; + return F_status_set_error(F_address_not); } #endif // _di_f_network_from_ip_string_ @@ -459,11 +468,17 @@ extern "C" { if (F_status_is_error(status)) return status; } - if (!inet_ntop(from.type == f_network_family_ip_4_e ? AF_INET : AF_INET6, (void *) & from.address, to->string + to->used, from.type == f_network_family_ip_4_e ? INET_ADDRSTRLEN : INET6_ADDRSTRLEN)) { - if (errno == EAFNOSUPPORT) return F_status_set_error(F_support_not); - if (errno == ENOSPC) return F_status_set_error(F_space_not); + { + const char * const result = (from.type == f_network_family_ip_4_e) + ? inet_ntop(AF_INET, (void *) & from.address.v4, to->string + to->used, INET_ADDRSTRLEN) + : inet_ntop(AF_INET6, (void *) & from.address.v6, to->string + to->used, INET6_ADDRSTRLEN); - return F_status_set_error(F_failure); + if (!result) { + if (errno == EAFNOSUPPORT) return F_status_set_error(F_support_not); + if (errno == ENOSPC) return F_status_set_error(F_space_not); + + return F_status_set_error(F_failure); + } } while (to->used < to->size && to->string[to->used]) { diff --git a/level_0/f_network/c/network.h b/level_0/f_network/c/network.h index 3368120..224ed44 100644 --- a/level_0/f_network/c/network.h +++ b/level_0/f_network/c/network.h @@ -141,6 +141,7 @@ extern "C" { * F_okay on success. * F_data_not on success but there is nothing to convert (to.type is f_network_family_none_e or from.used is 0). * + * F_address_not (with error bit) if from is not a valid address. * F_parameter (with error bit) if a parameter is invalid. * F_space_not (with error bit) if not enough space is available in to.string. * F_support_not (with error bit) if an invalid address family type is passed to inet_pton(). diff --git a/level_0/f_network/tests/unit/c/mock-network.c b/level_0/f_network/tests/unit/c/mock-network.c index 8ffc86e..25940f0 100644 --- a/level_0/f_network/tests/unit/c/mock-network.c +++ b/level_0/f_network/tests/unit/c/mock-network.c @@ -9,7 +9,7 @@ struct hostent *__wrap_gethostbyaddr(const void *addr, socklen_t len, int type) const bool failure = mock_type(bool); if (failure) { - errno = mock_type(int); + h_errno = mock_type(int); return 0; } @@ -22,7 +22,7 @@ struct hostent *__wrap_gethostbyname(const char *name) { const bool failure = mock_type(bool); if (failure) { - errno = mock_type(int); + h_errno = mock_type(int); return 0; } @@ -81,7 +81,7 @@ int __wrap_inet_pton(int af, const char *src, void *dst) { return -1; } - return 0; + return mock_type(int); } #ifdef __cplusplus diff --git a/level_0/f_network/tests/unit/c/test-network-from_ip_address.c b/level_0/f_network/tests/unit/c/test-network-from_ip_address.c index 1f8a48e..955d4e9 100644 --- a/level_0/f_network/tests/unit/c/test-network-from_ip_address.c +++ b/level_0/f_network/tests/unit/c/test-network-from_ip_address.c @@ -27,16 +27,26 @@ void test__f_network_from_ip_address__fails(void **state) { F_failure, }; + int types[] = { + f_network_family_ip_4_e, + f_network_family_ip_6_e, + }; + for (uint8_t i = 0; i < 6; ++i) { - struct hostent host; + for (uint8_t j = 0; j < 2; ++j) { - will_return(__wrap_gethostbyaddr, true); - will_return(__wrap_gethostbyaddr, errnos[i]); + family.type = types[j]; - const f_status_t status = f_network_from_ip_address(family, &host); + struct hostent host; + + will_return(__wrap_gethostbyaddr, true); + will_return(__wrap_gethostbyaddr, errnos[i]); + + const f_status_t status = f_network_from_ip_address(family, &host); - assert_int_equal(status, F_status_set_error(statuss[i])); + assert_int_equal(status, F_status_set_error(statuss[i])); + } // for } // for } diff --git a/level_0/f_network/tests/unit/c/test-network-from_ip_name.c b/level_0/f_network/tests/unit/c/test-network-from_ip_name.c index af7c426..bb1a3bf 100644 --- a/level_0/f_network/tests/unit/c/test-network-from_ip_name.c +++ b/level_0/f_network/tests/unit/c/test-network-from_ip_name.c @@ -7,6 +7,8 @@ extern "C" { void test__f_network_from_ip_name__fails(void **state) { + const f_string_static_t ip = macro_f_string_static_t_initialize_1("localhost", 0, 9); + int errnos[] = { HOST_NOT_FOUND, NO_DATA, @@ -32,7 +34,7 @@ void test__f_network_from_ip_name__fails(void **state) { will_return(__wrap_gethostbyname, true); will_return(__wrap_gethostbyname, errnos[i]); - const f_status_t status = f_network_from_ip_name(f_string_empty_s, &host); + const f_status_t status = f_network_from_ip_name(ip, &host); assert_int_equal(status, F_status_set_error(statuss[i])); } // for diff --git a/level_0/f_network/tests/unit/c/test-network-from_ip_string.c b/level_0/f_network/tests/unit/c/test-network-from_ip_string.c index fcfddb0..effc9ff 100644 --- a/level_0/f_network/tests/unit/c/test-network-from_ip_string.c +++ b/level_0/f_network/tests/unit/c/test-network-from_ip_string.c @@ -7,6 +7,8 @@ extern "C" { void test__f_network_from_ip_string__fails(void **state) { + const f_string_static_t ip = macro_f_string_static_t_initialize_1("127.0.0.1", 0, 9); + int errnos[] = { EAFNOSUPPORT, mock_errno_generic, @@ -21,13 +23,29 @@ void test__f_network_from_ip_string__fails(void **state) { f_network_family_ip_t family = f_network_family_ip_t_initialize; + family.type = f_network_family_ip_4_e; + will_return(__wrap_inet_pton, true); will_return(__wrap_inet_pton, errnos[i]); - const f_status_t status = f_network_from_ip_string(f_string_empty_s, &family); + const f_status_t status = f_network_from_ip_string(ip, &family); assert_int_equal(status, F_status_set_error(statuss[i])); } // for + + // Should return F_address_not (with error bit) for when non-errno return result of inet_pton() is 0. + { + f_network_family_ip_t family = f_network_family_ip_t_initialize; + + family.type = f_network_family_ip_4_e; + + will_return(__wrap_inet_pton, false); + will_return(__wrap_inet_pton, 0); + + const f_status_t status = f_network_from_ip_string(ip, &family); + + assert_int_equal(status, F_status_set_error(F_address_not)); + } } void test__f_network_from_ip_string__parameter_checking(void **state) { @@ -78,6 +96,7 @@ void test__f_network_from_ip_string__works(void **state) { { will_return(__wrap_inet_pton, false); + will_return(__wrap_inet_pton, 1); const f_status_t status = f_network_from_ip_string(ip, &family); @@ -88,6 +107,7 @@ void test__f_network_from_ip_string__works(void **state) { { will_return(__wrap_inet_pton, false); + will_return(__wrap_inet_pton, 1); const f_status_t status = f_network_from_ip_string(ip, &family); diff --git a/level_0/f_network/tests/unit/c/test-network-to_ip_string.c b/level_0/f_network/tests/unit/c/test-network-to_ip_string.c index cdc00f2..c428e42 100644 --- a/level_0/f_network/tests/unit/c/test-network-to_ip_string.c +++ b/level_0/f_network/tests/unit/c/test-network-to_ip_string.c @@ -22,14 +22,24 @@ void test__f_network_to_ip_string__fails(void **state) { F_failure, }; + int types[] = { + f_network_family_ip_4_e, + f_network_family_ip_6_e, + }; + for (uint8_t i = 0; i < 3; ++i) { - will_return(__wrap_inet_pton, true); - will_return(__wrap_inet_pton, errnos[i]); + for (uint8_t j = 0; j < 2; ++j) { - const f_status_t status = f_network_to_ip_string(family, &ip); + family.type = types[j]; + + will_return(__wrap_inet_ntop, true); + will_return(__wrap_inet_ntop, errnos[i]); + + const f_status_t status = f_network_to_ip_string(family, &ip); - assert_int_equal(status, F_status_set_error(statuss[i])); + assert_int_equal(status, F_status_set_error(statuss[i])); + } // for } // for free(ip.string); diff --git a/level_0/f_network/tests/unit/c/test-network.c b/level_0/f_network/tests/unit/c/test-network.c index c56d1b5..21e91c3 100644 --- a/level_0/f_network/tests/unit/c/test-network.c +++ b/level_0/f_network/tests/unit/c/test-network.c @@ -12,6 +12,7 @@ int setup(void **state) { int setdown(void **state) { errno = 0; + h_errno = 0; return 0; } @@ -19,6 +20,11 @@ int setdown(void **state) { int main(void) { const struct CMUnitTest tests[] = { + cmocka_unit_test(test__f_network_from_ip_address__fails), + cmocka_unit_test(test__f_network_from_ip_name__fails), + cmocka_unit_test(test__f_network_from_ip_string__fails), + cmocka_unit_test(test__f_network_to_ip_string__fails), + cmocka_unit_test(test__f_network_from_ip_address__returns_data_not), cmocka_unit_test(test__f_network_from_ip_name__returns_data_not), cmocka_unit_test(test__f_network_from_ip_string__returns_data_not), -- 1.8.3.1