View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0005003 | libmicrohttpd | other | public | 2017-05-04 10:45 | 2017-05-28 22:50 |
| Reporter | dohmniq | Assigned To | Karlson2k | ||
| Priority | normal | Severity | crash | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| OS | FreeBSD | OS Version | 11 | ||
| Product Version | 0.9.53 | ||||
| Target Version | 0.9.55 | Fixed in Version | 0.9.55 | ||
| Summary | 0005003: queuing a non-upgrade response (e.g. 404 Not Found) to a client's websocket request causes segfault | ||||
| Description | If a client calls a ws:// URL but the MHD server wants to respond with a non-101 response (e.g. 404 Not Found) then MHD dies with segmentation fault. This is inside keepalive_possible() around line 885 in src/microhttpd/connection.c. The segfault is because inside of the test "NULL == connection->response->upgrade_handler" connection->response itself is NULL. Presumably connection->response has been 'used'/output and then cleaned up, i.e. free()d. I'm not sure about keepalive_possible's expected output regarding the RFCs so I'm not providing a patch for that. I am attaching a patch to test_upgrade.c which covers this case though. | ||||
| Steps To Reproduce | apply attached patch against test_upgrade.c, run gmake in src/microhttpd then run .libs/test_upgrade | ||||
| Additional Information | gnunet bugtracker could do with an additional option under "Category" with the value "websockets"? See also bug 4996 for possibly related issue do with wrongly emitting "Connection: Keep-Alive" header? | ||||
| Tags | No tags attached. | ||||
| Attached Files | test_upgrade.c.diff (6,503 bytes)
--- test_upgrade.c.orig 2017-05-04 09:03:02.300982000 +0100
+++ test_upgrade.c 2017-05-04 09:18:04.048613000 +0100
@@ -838,6 +838,33 @@
return ret;
}
+static int
+ahc_upgrade_404 (void *cls,
+ struct MHD_Connection *connection,
+ const char *url,
+ const char *method,
+ const char *version,
+ const char *upload_data,
+ size_t *upload_data_size,
+ void **con_cls)
+{
+ struct MHD_Response *resp;
+ int ret;
+
+ if (NULL == *con_cls)
+ abort ();
+ if (! pthread_equal (**((pthread_t**)con_cls), pthread_self ()))
+ abort ();
+
+ char page[] = "Not found";
+
+ resp = MHD_create_response_from_buffer( strlen(page), (void*) page, MHD_RESPMEM_MUST_COPY );
+ ret = MHD_queue_response( connection, 404, resp );
+
+ MHD_destroy_response (resp);
+ return ret;
+}
+
/**
* Run the MHD external event loop using select.
@@ -976,7 +1003,8 @@
*/
static int
test_upgrade (int flags,
- unsigned int pool)
+ unsigned int pool,
+ unsigned int return404)
{
struct MHD_Daemon *d = NULL;
wr_socket sock;
@@ -988,11 +1016,13 @@
done = false;
+ MHD_AccessHandlerCallback ahc = return404 ? &ahc_upgrade_404 : &ahc_upgrade;
+
if (!test_tls)
d = MHD_start_daemon (flags | MHD_USE_ERROR_LOG | MHD_ALLOW_UPGRADE,
1080,
NULL, NULL,
- &ahc_upgrade, NULL,
+ ahc, NULL,
MHD_OPTION_URI_LOG_CALLBACK, &log_cb, NULL,
MHD_OPTION_NOTIFY_COMPLETED, ¬ify_completed_cb, NULL,
MHD_OPTION_NOTIFY_CONNECTION, ¬ify_connection_cb, NULL,
@@ -1003,7 +1033,7 @@
d = MHD_start_daemon (flags | MHD_USE_ERROR_LOG | MHD_ALLOW_UPGRADE | MHD_USE_TLS,
1080,
NULL, NULL,
- &ahc_upgrade, NULL,
+ ahc, NULL,
MHD_OPTION_URI_LOG_CALLBACK, &log_cb, NULL,
MHD_OPTION_NOTIFY_COMPLETED, ¬ify_completed_cb, NULL,
MHD_OPTION_NOTIFY_CONNECTION, ¬ify_connection_cb, NULL,
@@ -1129,6 +1159,7 @@
printf ("Starting HTTP \"Upgrade\" tests with %s connections.\n", test_tls ? "TLS" : "plain");
/* try external select */
res = test_upgrade (0,
+ 0,
0);
error_count += res;
if (res)
@@ -1138,6 +1169,7 @@
/* Try external auto */
res = test_upgrade (MHD_USE_AUTO,
+ 0,
0);
error_count += res;
if (res)
@@ -1147,6 +1179,7 @@
#ifdef EPOLL_SUPPORT
res = test_upgrade (MHD_USE_EPOLL,
+ 0,
0);
error_count += res;
if (res)
@@ -1157,6 +1190,7 @@
/* Test thread-per-connection */
res = test_upgrade (MHD_USE_INTERNAL_POLLING_THREAD | MHD_USE_THREAD_PER_CONNECTION,
+ 0,
0);
error_count += res;
if (res)
@@ -1165,6 +1199,7 @@
printf ("PASSED: Upgrade with thread per connection.\n");
res = test_upgrade (MHD_USE_AUTO | MHD_USE_INTERNAL_POLLING_THREAD | MHD_USE_THREAD_PER_CONNECTION,
+ 0,
0);
error_count += res;
if (res)
@@ -1173,6 +1208,7 @@
printf ("PASSED: Upgrade with thread per connection and 'auto'.\n");
#ifdef HAVE_POLL
res = test_upgrade (MHD_USE_INTERNAL_POLLING_THREAD | MHD_USE_THREAD_PER_CONNECTION | MHD_USE_POLL,
+ 0,
0);
error_count += res;
if (res)
@@ -1183,6 +1219,7 @@
/* Test different event loops, with and without thread pool */
res = test_upgrade (MHD_USE_INTERNAL_POLLING_THREAD,
+ 0,
0);
error_count += res;
if (res)
@@ -1190,13 +1227,15 @@
else if (verbose)
printf ("PASSED: Upgrade with internal select.\n");
res = test_upgrade (MHD_USE_INTERNAL_POLLING_THREAD,
- 2);
+ 2,
+ 0);
error_count += res;
if (res)
fprintf (stderr, "FAILED: Upgrade with internal select with thread pool, return code %d.\n", res);
else if (verbose)
printf ("PASSED: Upgrade with internal select with thread pool.\n");
res = test_upgrade (MHD_USE_AUTO | MHD_USE_INTERNAL_POLLING_THREAD,
+ 0,
0);
error_count += res;
if (res)
@@ -1204,7 +1243,8 @@
else if (verbose)
printf ("PASSED: Upgrade with internal 'auto'.\n");
res = test_upgrade (MHD_USE_AUTO | MHD_USE_INTERNAL_POLLING_THREAD,
- 2);
+ 2,
+ 0);
error_count += res;
if (res)
fprintf (stderr, "FAILED: Upgrade with internal 'auto' with thread pool, return code %d.\n", res);
@@ -1212,6 +1252,7 @@
printf ("PASSED: Upgrade with internal 'auto' with thread pool.\n");
#ifdef HAVE_POLL
res = test_upgrade (MHD_USE_POLL_INTERNAL_THREAD,
+ 0,
0);
error_count += res;
if (res)
@@ -1219,7 +1260,8 @@
else if (verbose)
printf ("PASSED: Upgrade with internal poll.\n");
res = test_upgrade (MHD_USE_POLL_INTERNAL_THREAD,
- 2);
+ 2,
+ 0);
if (res)
fprintf (stderr, "FAILED: Upgrade with internal poll with thread pool, return code %d.\n", res);
else if (verbose)
@@ -1227,18 +1269,28 @@
#endif
#ifdef EPOLL_SUPPORT
res = test_upgrade (MHD_USE_EPOLL_INTERNAL_THREAD,
+ 0,
0);
if (res)
fprintf (stderr, "FAILED: Upgrade with internal epoll, return code %d.\n", res);
else if (verbose)
printf ("PASSED: Upgrade with internal epoll.\n");
res = test_upgrade (MHD_USE_EPOLL_INTERNAL_THREAD,
- 2);
+ 2,
+ 0);
if (res)
fprintf (stderr, "FAILED: Upgrade with internal epoll, return code %d.\n", res);
else if (verbose)
printf ("PASSED: Upgrade with internal epoll.\n");
#endif
+ res = test_upgrade (MHD_USE_AUTO | MHD_USE_INTERNAL_POLLING_THREAD | MHD_USE_THREAD_PER_CONNECTION,
+ 0,
+ 1);
+ error_count += res;
+ if (res)
+ fprintf (stderr, "FAILED: Upgrade with 404 response, return code %d.\n", res);
+ else if (verbose)
+ printf ("PASSED: Upgrade with 404 response.\n");
/* report result */
if (0 != error_count)
fprintf (stderr,
| ||||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2017-05-04 10:45 | dohmniq | New Issue | |
| 2017-05-04 10:45 | dohmniq | File Added: test_upgrade.c.diff | |
| 2017-05-04 22:44 | Christian Grothoff | Assigned To | => Karlson2k |
| 2017-05-04 22:44 | Christian Grothoff | Status | new => resolved |
| 2017-05-04 22:44 | Christian Grothoff | Fixed in Version | => 0.9.55 |
| 2017-05-04 22:44 | Christian Grothoff | Target Version | => 0.9.55 |
| 2017-05-28 22:50 | Christian Grothoff | Status | resolved => closed |
| 2017-05-28 22:50 | Christian Grothoff | Resolution | open => fixed |
