From 071236e3e252fd0dcf5102739adecb8e69a76c4c Mon Sep 17 00:00:00 2001 From: juga0 Date: Mon, 2 Apr 2018 14:16:04 +0000 Subject: [PATCH 1/8] Add a test for geoip_load_file() using geoip6 Signed-off-by: Isis Lovecruft --- src/test/test_geoip.c | 63 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index 20f6114dc0..b4719be6cf 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -444,6 +444,68 @@ test_geoip_load_file(void *arg) tor_free(dhex); } +static void +test_geoip6_load_file(void *arg) +{ + (void)arg; + struct in6_addr iaddr6; + char *contents = NULL; + char *dhex = NULL; + + /* A nonexistant filename should fail. */ + /* NOTE: this is probably not needed, as the path is the same, just using + AF_INET6 */ + tt_int_op(-1, OP_EQ, + geoip_load_file(AF_INET6, "/you/did/not/put/a/file/here/I/hope")); + + /* Any lookup attempt should say "-1" because we have no info */ + tor_inet_pton(AF_INET6, "2001:4860:4860::8888", &iaddr6); + tt_int_op(-1, OP_EQ, geoip_get_country_by_ipv6(&iaddr6)); + + /* Load geiop6 file */ + const char FNAME6[] = SRCDIR "/src/config/geoip6"; + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET6, FNAME6)); + + /* Check that we loaded some countries; this will fail if there are ever + * fewer than 50 countries in the world. */ + tt_int_op(geoip_get_n_countries(), OP_GE, 50); + + /* Let's see where 2001:4860:4860::8888 (google dns) is. */ + const char *caddr6 = "2001:4860:4860::8888"; + tor_inet_pton(AF_INET6, caddr6, &iaddr6); + int country6 = geoip_get_country_by_ipv6(&iaddr6); + tt_int_op(country6, OP_GE, 1); + + const char *cc6 = geoip_get_country_name(country6); + tt_int_op(strlen(cc6), OP_EQ, 2); + + /* The digest should be set.... */ + tt_str_op("0000000000000000000000000000000000000000", OP_NE, + geoip_db_digest(AF_INET6)); + + /* And it should be set correctly */ + contents = read_file_to_str(FNAME6, RFTS_BIN, NULL); + uint8_t d[DIGEST_LEN]; + crypto_digest((char*)d, contents, strlen(contents)); + dhex = tor_strdup(hex_str((char*)d, DIGEST_LEN)); + tt_str_op(dhex, OP_EQ, geoip_db_digest(AF_INET6)); + + /* Make sure geoip_free_all() works. */ + /* NOTE: is this needed here?, if geoip_free_all, it should not change with + ipv6 */ + geoip_free_all(); + tt_int_op(1, OP_EQ, geoip_get_n_countries()); + tt_str_op("??", OP_EQ, geoip_get_country_name(0)); + tor_inet_pton(AF_INET6, "::1:2:3:4", &iaddr6); + tt_int_op(-1, OP_EQ, geoip_get_country_by_ipv6(&iaddr6)); + tt_str_op("0000000000000000000000000000000000000000", OP_EQ, + geoip_db_digest(AF_INET6)); + + done: + tor_free(contents); + tor_free(dhex); +} + #define ENT(name) \ { #name, test_ ## name , 0, NULL, NULL } #define FORK(name) \ @@ -459,6 +521,7 @@ struct testcase_t geoip_tests[] = { { "geoip", test_geoip, TT_FORK, NULL, NULL }, { "geoip_with_pt", test_geoip_with_pt, TT_FORK, NULL, NULL }, { "load_file", test_geoip_load_file, TT_FORK|SKIP_ON_WINDOWS, NULL, NULL }, + { "load_file6", test_geoip6_load_file, TT_FORK, NULL, NULL }, END_OF_TESTCASES }; From 8be1ac8abe56e605009acc59429bd53e1ab6853f Mon Sep 17 00:00:00 2001 From: juga0 Date: Mon, 9 Apr 2018 09:20:12 +0000 Subject: [PATCH 2/8] Add test to check that loading a 2nd file replaces the 1st Signed-off-by: Isis Lovecruft --- src/test/test_geoip.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index b4719be6cf..b35997edc6 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -506,6 +506,33 @@ test_geoip6_load_file(void *arg) tor_free(dhex); } +static void +test_geoip_load_2nd_file(void *arg) +{ + (void)arg; + + /* Load 1nd geoip file */ + const char FNAME[] = SRCDIR "/src/config/geoip"; + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME)); + + int num_countries_geoip = geoip_get_n_countries(); + + /* Load 2st geoip (empty) file */ + const char FNAME2[] = SRCDIR "/src/test/geoip_dummy"; + tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME2)); + + int num_countries_geoip2 = geoip_get_n_countries(); + /* FIXME: should not this be different? */ + /* tt_int_op(num_countries_geoip, OP_NE, num_countries_geoip2); */ + + /* Check that there is no geoip information for 8.8.8.8, */ + /* since loading the empty 2nd file should have delete it. */ + int country = geoip_get_country_by_ipv4(0x08080808); + tt_int_op(country, OP_EQ, 0); + + done: ; +} + #define ENT(name) \ { #name, test_ ## name , 0, NULL, NULL } #define FORK(name) \ @@ -522,6 +549,7 @@ struct testcase_t geoip_tests[] = { { "geoip_with_pt", test_geoip_with_pt, TT_FORK, NULL, NULL }, { "load_file", test_geoip_load_file, TT_FORK|SKIP_ON_WINDOWS, NULL, NULL }, { "load_file6", test_geoip6_load_file, TT_FORK, NULL, NULL }, + { "load_2nd_file", test_geoip_load_2nd_file, TT_FORK, NULL, NULL }, END_OF_TESTCASES }; From 3f967bfbd13014764b9938c92981b7c3316b5c08 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 16 Apr 2018 19:06:56 +0000 Subject: [PATCH 3/8] tests: Skip two more geoip_load_file tests on Windows. * FIXES part of #25515: https://bugs.torproject.org/25515 --- src/test/test_geoip.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index b35997edc6..af36166538 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -548,8 +548,10 @@ struct testcase_t geoip_tests[] = { { "geoip", test_geoip, TT_FORK, NULL, NULL }, { "geoip_with_pt", test_geoip_with_pt, TT_FORK, NULL, NULL }, { "load_file", test_geoip_load_file, TT_FORK|SKIP_ON_WINDOWS, NULL, NULL }, - { "load_file6", test_geoip6_load_file, TT_FORK, NULL, NULL }, - { "load_2nd_file", test_geoip_load_2nd_file, TT_FORK, NULL, NULL }, + { "load_file6", test_geoip6_load_file, TT_FORK | SKIP_ON_WINDOWS, + NULL, NULL }, + { "load_2nd_file", test_geoip_load_2nd_file, TT_FORK | SKIP_ON_WINDOWS, + NULL, NULL }, END_OF_TESTCASES }; From 6a28a82998b239ea312289ab19e55330f801e3b9 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 16 Apr 2018 19:28:03 +0000 Subject: [PATCH 4/8] tests: Fix a couple typos and remove unnecessary inline comments. --- src/test/test_geoip.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index af36166538..fa5916c050 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -453,8 +453,6 @@ test_geoip6_load_file(void *arg) char *dhex = NULL; /* A nonexistant filename should fail. */ - /* NOTE: this is probably not needed, as the path is the same, just using - AF_INET6 */ tt_int_op(-1, OP_EQ, geoip_load_file(AF_INET6, "/you/did/not/put/a/file/here/I/hope")); @@ -491,8 +489,6 @@ test_geoip6_load_file(void *arg) tt_str_op(dhex, OP_EQ, geoip_db_digest(AF_INET6)); /* Make sure geoip_free_all() works. */ - /* NOTE: is this needed here?, if geoip_free_all, it should not change with - ipv6 */ geoip_free_all(); tt_int_op(1, OP_EQ, geoip_get_n_countries()); tt_str_op("??", OP_EQ, geoip_get_country_name(0)); @@ -511,13 +507,13 @@ test_geoip_load_2nd_file(void *arg) { (void)arg; - /* Load 1nd geoip file */ + /* Load 1st geoip file */ const char FNAME[] = SRCDIR "/src/config/geoip"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME)); int num_countries_geoip = geoip_get_n_countries(); - /* Load 2st geoip (empty) file */ + /* Load 2nd geoip (empty) file */ const char FNAME2[] = SRCDIR "/src/test/geoip_dummy"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME2)); From 96469b82f8d27f9c69d863799b624c93cd1ea6f0 Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 17 Apr 2018 16:38:24 +0000 Subject: [PATCH 5/8] Remove FIXME about comparing num countries, * remove the fixme since clearing the countries should be other issue * remove unused variables related to it since that cause travis to fail --- src/test/test_geoip.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index fa5916c050..f5949f7555 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -511,16 +511,10 @@ test_geoip_load_2nd_file(void *arg) const char FNAME[] = SRCDIR "/src/config/geoip"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME)); - int num_countries_geoip = geoip_get_n_countries(); - /* Load 2nd geoip (empty) file */ const char FNAME2[] = SRCDIR "/src/test/geoip_dummy"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME2)); - int num_countries_geoip2 = geoip_get_n_countries(); - /* FIXME: should not this be different? */ - /* tt_int_op(num_countries_geoip, OP_NE, num_countries_geoip2); */ - /* Check that there is no geoip information for 8.8.8.8, */ /* since loading the empty 2nd file should have delete it. */ int country = geoip_get_country_by_ipv4(0x08080808); @@ -551,4 +545,3 @@ struct testcase_t geoip_tests[] = { END_OF_TESTCASES }; - From d0ad74e0f67aa598e50c83df1f6b6832dabbb61a Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 17 Apr 2018 16:53:18 +0000 Subject: [PATCH 6/8] Add clarification about type of file expected --- src/test/test_geoip.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index f5949f7555..302af85771 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -512,6 +512,7 @@ test_geoip_load_2nd_file(void *arg) tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME)); /* Load 2nd geoip (empty) file */ + /* It has to be the same IP address family */ const char FNAME2[] = SRCDIR "/src/test/geoip_dummy"; tt_int_op(0, OP_EQ, geoip_load_file(AF_INET, FNAME2)); From 3d4bbf94c67e4bdd91d5378ca0321237fbe7d082 Mon Sep 17 00:00:00 2001 From: juga0 Date: Wed, 18 Apr 2018 08:51:39 +0000 Subject: [PATCH 7/8] tests: Add forgotten empty file required for geoip --- src/test/geoip_dummy | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 src/test/geoip_dummy diff --git a/src/test/geoip_dummy b/src/test/geoip_dummy new file mode 100644 index 0000000000..e69de29bb2 From f4ad30448ab419de2163b42c5447b51867042734 Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 24 Apr 2018 11:27:11 +0000 Subject: [PATCH 8/8] Recover newline at the EOF, removed by mistake in 071236e3e252fd0dcf5102739adecb8e69a76c4c. --- src/test/test_geoip.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/test_geoip.c b/src/test/test_geoip.c index 302af85771..0711a113eb 100644 --- a/src/test/test_geoip.c +++ b/src/test/test_geoip.c @@ -546,3 +546,4 @@ struct testcase_t geoip_tests[] = { END_OF_TESTCASES }; +