From 435fecd75121bd4e025290b456f68f1177836176 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 9 May 2014 13:28:06 +0200 Subject: [PATCH 1/7] unit-tests: Make sure plugins in the builddir are loaded When running the tests in GDB the working directory apparently is different. With the relative path used previously the plugins would not be found and those installed on the system would get used. --- src/libstrongswan/tests/Makefile.am | 2 +- src/libtls/tests/Makefile.am | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstrongswan/tests/Makefile.am b/src/libstrongswan/tests/Makefile.am index 331a5480d..b5fb45a1f 100644 --- a/src/libstrongswan/tests/Makefile.am +++ b/src/libstrongswan/tests/Makefile.am @@ -52,7 +52,7 @@ tests_SOURCES = tests.h tests.c \ tests_CFLAGS = \ -I$(top_srcdir)/src/libstrongswan \ -I$(top_srcdir)/src/libstrongswan/tests \ - -DPLUGINDIR=\""$(top_builddir)/src/libstrongswan/plugins\"" \ + -DPLUGINDIR=\""$(abs_top_builddir)/src/libstrongswan/plugins\"" \ -DPLUGINS=\""${s_plugins}\"" \ @COVERAGE_CFLAGS@ diff --git a/src/libtls/tests/Makefile.am b/src/libtls/tests/Makefile.am index 1c0e2f941..456383f02 100644 --- a/src/libtls/tests/Makefile.am +++ b/src/libtls/tests/Makefile.am @@ -11,7 +11,7 @@ tls_tests_CFLAGS = \ -I$(top_srcdir)/src/libtls \ -I$(top_srcdir)/src/libstrongswan \ -I$(top_srcdir)/src/libstrongswan/tests \ - -DPLUGINDIR=\""$(top_builddir)/src/libstrongswan/plugins\"" \ + -DPLUGINDIR=\""$(abs_top_builddir)/src/libstrongswan/plugins\"" \ -DPLUGINS=\""${s_plugins}\"" \ @COVERAGE_CFLAGS@ From 032dcb89895b6658399a326fa2064f47002f0f4d Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 9 May 2014 17:42:37 +0200 Subject: [PATCH 2/7] unit-tests: Defer failures by worker threads In some cases the main thread is not ready to immediately call siglongjmp(), e.g. if it currently holds a mutex that is later required during shutdown. Therefore, we delay handling errors in worker threads until the main thread performs the next check itself (or the test function ends). The same issue remains with SIGALRM. --- src/libstrongswan/tests/test_suite.c | 23 +++++++++++++++++++++-- src/libstrongswan/tests/test_suite.h | 18 +++++++++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/libstrongswan/tests/test_suite.c b/src/libstrongswan/tests/test_suite.c index fb40b05c1..a636d6f7c 100644 --- a/src/libstrongswan/tests/test_suite.c +++ b/src/libstrongswan/tests/test_suite.c @@ -40,6 +40,11 @@ static int failure_line; */ static backtrace_t *failure_backtrace; +/** + * Flag to indicate if a worker thread failed + */ +static bool worker_failed; + /** * Longjump restore point when failing */ @@ -169,6 +174,17 @@ void test_fail_msg(const char *file, int line, char *fmt, ...) test_failure(); } +/** + * See header. + */ +void test_fail_if_worker_failed() +{ + if (pthread_self() == main_thread && worker_failed) + { + test_failure(); + } +} + /** * Signal handler catching critical and alarm signals */ @@ -180,8 +196,9 @@ static void test_sighandler(int signal) switch (signal) { case SIGUSR1: - /* a different thread failed, abort test */ - return test_failure(); + /* a different thread failed, abort test at the next opportunity */ + worker_failed = TRUE; + return; case SIGSEGV: signame = "SIGSEGV"; break; @@ -251,6 +268,8 @@ void test_setup_timeout(int s) sigaction(SIGUSR1, &action, NULL); alarm(s); + + worker_failed = FALSE; } /** diff --git a/src/libstrongswan/tests/test_suite.h b/src/libstrongswan/tests/test_suite.h index 2b1a64c2d..4bef6ff37 100644 --- a/src/libstrongswan/tests/test_suite.h +++ b/src/libstrongswan/tests/test_suite.h @@ -236,6 +236,12 @@ void test_fail_vmsg(const char *file, int line, char *fmt, va_list args); */ void test_fail_msg(const char *file, int line, char *fmt, ...); +/** + * Let a test fail if one of the worker threads has failed (only if called from + * the main thread). + */ +void test_fail_if_worker_failed(); + /** * Check if two integers equal, fail test if not * @@ -246,6 +252,7 @@ void test_fail_msg(const char *file, int line, char *fmt, ...); ({ \ typeof(a) _a = a; \ typeof(b) _b = b; \ + test_fail_if_worker_failed(); \ if (_a != _b) \ { \ test_fail_msg(__FILE__, __LINE__, #a " != " #b " (%d != %d)", _a, _b); \ @@ -262,6 +269,7 @@ void test_fail_msg(const char *file, int line, char *fmt, ...); ({ \ char* _a = (char*)a; \ char* _b = (char*)b; \ + test_fail_if_worker_failed(); \ if (!_a || !_b || !streq(_a, _b)) \ { \ test_fail_msg(__FILE__, __LINE__, \ @@ -279,6 +287,7 @@ void test_fail_msg(const char *file, int line, char *fmt, ...); ({ \ chunk_t _a = (chunk_t)a; \ chunk_t _b = (chunk_t)b; \ + test_fail_if_worker_failed(); \ if (_a.len != _b.len || !memeq(a.ptr, b.ptr, a.len)) \ { \ test_fail_msg(__FILE__, __LINE__, \ @@ -293,6 +302,7 @@ void test_fail_msg(const char *file, int line, char *fmt, ...); */ #define test_assert(x) \ ({ \ + test_fail_if_worker_failed(); \ if (!(x)) \ { \ test_fail_msg(__FILE__, __LINE__, #x); \ @@ -308,6 +318,7 @@ void test_fail_msg(const char *file, int line, char *fmt, ...); */ #define test_assert_msg(x, fmt, ...) \ ({ \ + test_fail_if_worker_failed(); \ if (!(x)) \ { \ test_fail_msg(__FILE__, __LINE__, #x ": " fmt, ##__VA_ARGS__); \ @@ -327,6 +338,7 @@ void test_fail_msg(const char *file, int line, char *fmt, ...); #define fail(fmt, ...) test_fail_msg(__FILE__, __LINE__, fmt, ##__VA_ARGS__) #define fail_if(x, fmt, ...) \ ({ \ + test_fail_if_worker_failed(); \ if (x) \ { \ test_fail_msg(__FILE__, __LINE__, #x ": " fmt, ##__VA_ARGS__); \ @@ -341,10 +353,10 @@ void test_fail_msg(const char *file, int line, char *fmt, ...); #define tcase_set_timeout test_case_set_timeout #define suite_add_tcase test_suite_add_case #define START_TEST(name) static void name (int _i) { -#define END_TEST } +#define END_TEST test_fail_if_worker_failed(); } #define START_SETUP(name) static void name() { -#define END_SETUP } +#define END_SETUP test_fail_if_worker_failed(); } #define START_TEARDOWN(name) static void name() { -#define END_TEARDOWN } +#define END_TEARDOWN test_fail_if_worker_failed(); } #endif /** TEST_SUITE_H_ @}*/ From 9a1859375290fb2d6f5d29ebc648d472684fc0c0 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 7 May 2014 16:59:07 +0200 Subject: [PATCH 3/7] fetcher: Add option to retrieve response code from a fetcher --- src/libstrongswan/fetcher/fetcher.h | 6 ++++++ src/libstrongswan/fetcher/fetcher_manager.c | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/libstrongswan/fetcher/fetcher.h b/src/libstrongswan/fetcher/fetcher.h index 890258c3c..6043dac2e 100644 --- a/src/libstrongswan/fetcher/fetcher.h +++ b/src/libstrongswan/fetcher/fetcher.h @@ -81,6 +81,12 @@ enum fetcher_option_t { */ FETCH_TIMEOUT, + /** + * Sets a pointer to a variable that receives the request's response code. + * Additional argument is a u_int*. + */ + FETCH_RESPONSE_CODE, + /** * Callback to invoke with each chunk of data. * Additional argument fetch_callback_t. diff --git a/src/libstrongswan/fetcher/fetcher_manager.c b/src/libstrongswan/fetcher/fetcher_manager.c index 2fad486e0..f36cfcf82 100644 --- a/src/libstrongswan/fetcher/fetcher_manager.c +++ b/src/libstrongswan/fetcher/fetcher_manager.c @@ -113,6 +113,10 @@ METHOD(fetcher_manager_t, fetch, status_t, good = fetcher->set_option(fetcher, opt, va_arg(args, fetcher_callback_t)); continue; + case FETCH_RESPONSE_CODE: + good = fetcher->set_option(fetcher, opt, + va_arg(args, u_int*)); + continue; case FETCH_SOURCEIP: host = va_arg(args, host_t*); if (host && !host->is_anyaddr(host)) From deb8975bd2cabc57422af05989c59940b695ee78 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 9 May 2014 18:34:25 +0200 Subject: [PATCH 4/7] unit-tests: Add a test case for HTTP response codes --- .../tests/suites/test_fetch_http.c | 65 ++++++++++++++++--- 1 file changed, 57 insertions(+), 8 deletions(-) diff --git a/src/libstrongswan/tests/suites/test_fetch_http.c b/src/libstrongswan/tests/suites/test_fetch_http.c index 8749ff375..eeef3587c 100644 --- a/src/libstrongswan/tests/suites/test_fetch_http.c +++ b/src/libstrongswan/tests/suites/test_fetch_http.c @@ -18,6 +18,8 @@ #include #include +#define HTTP_SUCCESS(status) ((status) >= 200 && (status) < 300) + /** * HTTP test definition */ @@ -42,6 +44,8 @@ typedef struct { void *res; /* length of response data */ int res_len; + /* status code, defaults to 200 */ + u_int code; } test_service_t; static char large[] = { @@ -147,8 +151,13 @@ static bool servicing(void *data, stream_t *stream) ck_assert(memeq(body, test->req, test->req_len)); } + if (!test->code) + { + test->code = 200; + } + /* response headers */ - snprintf(buf, sizeof(buf), "HTTP/1.%u 200 OK\r\n", test->minor); + snprintf(buf, sizeof(buf), "HTTP/1.%u %u OK\r\n", test->minor, test->code); ck_assert(stream->write_all(stream, buf, strlen(buf))); t = time(NULL); gmtime_r(&t, &tm); @@ -174,13 +183,13 @@ static bool servicing(void *data, stream_t *stream) static test_service_t gtests[] = { { "GET", 1, "127.0.0.1", 6543, "/a/test/?b=c", NULL, - NULL, 0, "\x12\x34", 2 }, + NULL, 0, "\x12\x34", 2, 0 }, { "GET", 0, "localhost", 6543, "/", NULL, - NULL, 0, NULL, 0 }, + NULL, 0, NULL, 0, 0 }, { "GET", 0, "127.0.0.1", 6543, "/largefile", NULL, - NULL, 0, large, sizeof(large) }, + NULL, 0, large, sizeof(large), 0 }, { "GET", 1, "[::1]", 6543, "/ipv6-url", NULL, - NULL, 0, "\x00\r\n\r\x00testdatablabla", 20 }, + NULL, 0, "\x00\r\n\r\x00testdatablabla", 20, 0 }, }; START_TEST(test_get) @@ -215,11 +224,11 @@ END_TEST static test_service_t ptests[] = { { "POST", 1, "127.0.0.1", 6543, "/a/test/?b=c", "application/binary", - "\x23\x45", 2, "\x12\x34", 2 }, + "\x23\x45", 2, "\x12\x34", 2, 0 }, { "POST", 0, "localhost", 6543, "/largefile", "application/x-large", - large, sizeof(large), large, sizeof(large) }, + large, sizeof(large), large, sizeof(large), 0 }, { "POST", 1, "[::1]", 6543, "/ipv6-url", "text/plain", - "\x00\r\n\r\x00testdatablabla", 20, "\x00\r\n\r\x00testdatablabla", 20 }, + "\x00\r\n\r\x00testdatablabla", 20, "\x00\r\n\r\x00testdatablabla", 20, 0 }, }; START_TEST(test_post) @@ -254,6 +263,42 @@ START_TEST(test_post) } END_TEST + +static test_service_t rtests[] = { + { "GET", 1, "localhost", 6544, "/", NULL, NULL, 0, NULL, 0, 200 }, + { "GET", 1, "localhost", 6544, "/", NULL, NULL, 0, NULL, 0, 204 }, + { "GET", 1, "localhost", 6544, "/", NULL, NULL, 0, NULL, 0, 400 }, + { "GET", 1, "localhost", 6544, "/", NULL, NULL, 0, NULL, 0, 404 }, + { "GET", 1, "localhost", 6544, "/", NULL, NULL, 0, NULL, 0, 500 }, +}; + +START_TEST(test_response_code) +{ + stream_service_t *service; + status_t status; + chunk_t data; + char uri[256]; + u_int code; + + lib->processor->set_threads(lib->processor, 8); + + snprintf(uri, sizeof(uri), "tcp://%s:%u", rtests[_i].host, rtests[_i].port); + service = lib->streams->create_service(lib->streams, uri, 1); + ck_assert(service != NULL); + service->on_accept(service, servicing, &rtests[_i], JOB_PRIO_HIGH, 0); + + snprintf(uri, sizeof(uri), "http://%s:%u%s", + rtests[_i].host, rtests[_i].port, rtests[_i].path); + status = lib->fetcher->fetch(lib->fetcher, uri, &data, + FETCH_RESPONSE_CODE, &code, FETCH_END); + ck_assert_int_eq(status, HTTP_SUCCESS(rtests[_i].code) ? SUCCESS : FAILED); + ck_assert_int_eq(code, rtests[_i].code); + free(data.ptr); + + service->destroy(service); +} +END_TEST + Suite *fetch_http_suite_create() { Suite *s; @@ -269,5 +314,9 @@ Suite *fetch_http_suite_create() tcase_add_loop_test(tc, test_post, 0, countof(ptests)); suite_add_tcase(s, tc); + tc = tcase_create("response code"); + tcase_add_loop_test(tc, test_response_code, 0, countof(rtests)); + suite_add_tcase(s, tc); + return s; } From 703a0b4c3e11ceee936c795efdb630fbc45faa75 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 9 May 2014 18:35:20 +0200 Subject: [PATCH 5/7] curl: Add support to return the response code --- src/libstrongswan/plugins/curl/curl_fetcher.c | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/libstrongswan/plugins/curl/curl_fetcher.c b/src/libstrongswan/plugins/curl/curl_fetcher.c index 644f27709..573c4c369 100644 --- a/src/libstrongswan/plugins/curl/curl_fetcher.c +++ b/src/libstrongswan/plugins/curl/curl_fetcher.c @@ -49,6 +49,11 @@ struct private_curl_fetcher_t { */ fetcher_callback_t cb; + /** + * Variable that receives the response code + */ + u_int *result; + /** * Timeout for a transfer */ @@ -82,6 +87,7 @@ METHOD(fetcher_t, fetch, status_t, { char error[CURL_ERROR_SIZE], *enc_uri; status_t status; + long result = 0; cb_data_t data = { .cb = this->cb, .user = userdata, @@ -123,10 +129,25 @@ METHOD(fetcher_t, fetch, status_t, status = NOT_SUPPORTED; break; case CURLE_OK: + if (this->result) + { + curl_easy_getinfo(this->curl, CURLINFO_RESPONSE_CODE, + &result); + *this->result = result; + } status = SUCCESS; break; default: - DBG1(DBG_LIB, "libcurl http request failed: %s", error); + if (this->result) + { /* don't log an error in this case */ + curl_easy_getinfo(this->curl, CURLINFO_RESPONSE_CODE, + &result); + *this->result = result; + } + else + { + DBG1(DBG_LIB, "libcurl http request failed: %s", error); + } status = FAILED; break; } @@ -188,6 +209,11 @@ METHOD(fetcher_t, set_option, bool, this->cb = va_arg(args, fetcher_callback_t); break; } + case FETCH_RESPONSE_CODE: + { + this->result = va_arg(args, u_int*); + break; + } case FETCH_SOURCEIP: { char buf[64]; From 350c1dead93d2db0d439bdfdb30a89e599d55660 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 9 May 2014 18:44:17 +0200 Subject: [PATCH 6/7] unit-tests: Allow some HTTP write operations to fail Because CURLOPT_FAILONERROR is enabled in the curl plugin an error code will often (not always) cause the client to close the TCP connection before the server has written the complete response. --- .../tests/suites/test_fetch_http.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libstrongswan/tests/suites/test_fetch_http.c b/src/libstrongswan/tests/suites/test_fetch_http.c index eeef3587c..42743c787 100644 --- a/src/libstrongswan/tests/suites/test_fetch_http.c +++ b/src/libstrongswan/tests/suites/test_fetch_http.c @@ -159,25 +159,30 @@ static bool servicing(void *data, stream_t *stream) /* response headers */ snprintf(buf, sizeof(buf), "HTTP/1.%u %u OK\r\n", test->minor, test->code); ck_assert(stream->write_all(stream, buf, strlen(buf))); + + /* if the response code indicates an error the following write operations + * might fail because the client already terminated the TCP connection */ +#define may_fail(test, op) ck_assert(op || !HTTP_SUCCESS(test->code)) + t = time(NULL); gmtime_r(&t, &tm); strftime(buf, sizeof(buf), "%a, %d %b %Y %T %z", &tm); - ck_assert(stream->write_all(stream, buf, strlen(buf))); + may_fail(test, stream->write_all(stream, buf, strlen(buf))); snprintf(buf, sizeof(buf), "Server: strongSwan unit test\r\n"); - ck_assert(stream->write_all(stream, buf, strlen(buf))); + may_fail(test, stream->write_all(stream, buf, strlen(buf))); /* rest of response headers */ snprintf(buf, sizeof(buf), "Content-Type: text/plain\r\n"); - ck_assert(stream->write_all(stream, buf, strlen(buf))); + may_fail(test, stream->write_all(stream, buf, strlen(buf))); snprintf(buf, sizeof(buf), "Content-Length: %u\r\n", test->res_len); - ck_assert(stream->write_all(stream, buf, strlen(buf))); + may_fail(test, stream->write_all(stream, buf, strlen(buf))); snprintf(buf, sizeof(buf), "Connection: close\r\n"); - ck_assert(stream->write_all(stream, buf, strlen(buf))); + may_fail(test, stream->write_all(stream, buf, strlen(buf))); snprintf(buf, sizeof(buf), "\r\n"); - ck_assert(stream->write_all(stream, buf, strlen(buf))); + may_fail(test, stream->write_all(stream, buf, strlen(buf))); /* response body */ - ck_assert(stream->write_all(stream, test->res, test->res_len)); + may_fail(test, stream->write_all(stream, test->res, test->res_len)); return FALSE; } From 271c2dd24ec6e3f4b30bdd3ca2b3edc1a53c425f Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 9 May 2014 19:02:28 +0200 Subject: [PATCH 7/7] soup: Add support to retrieve the response code --- src/libstrongswan/plugins/soup/soup_fetcher.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/libstrongswan/plugins/soup/soup_fetcher.c b/src/libstrongswan/plugins/soup/soup_fetcher.c index 681a3c357..99525cb79 100644 --- a/src/libstrongswan/plugins/soup/soup_fetcher.c +++ b/src/libstrongswan/plugins/soup/soup_fetcher.c @@ -63,6 +63,11 @@ struct private_soup_fetcher_t { * Fetcher callback function */ fetcher_callback_t cb; + + /** + * Response status + */ + u_int *result; }; /** @@ -119,12 +124,16 @@ METHOD(fetcher_t, fetch, status_t, DBG2(DBG_LIB, "sending http request to '%s'...", uri); soup_session_send_message(data.session, message); + if (this->result) + { + *this->result = message->status_code; + } if (SOUP_STATUS_IS_SUCCESSFUL(message->status_code)) { status = SUCCESS; } - else - { + else if (!this->result) + { /* only log an error if the code is not returned */ DBG1(DBG_LIB, "HTTP request failed: %s", message->reason_phrase); } g_object_unref(G_OBJECT(message)); @@ -157,6 +166,9 @@ METHOD(fetcher_t, set_option, bool, case FETCH_CALLBACK: this->cb = va_arg(args, fetcher_callback_t); break; + case FETCH_RESPONSE_CODE: + this->result = va_arg(args, u_int*); + break; default: supported = FALSE; break;