View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0007757 | libmicrohttpd | libmicrohttpd API | public | 2023-03-09 07:06 | 2023-09-03 12:05 |
| Reporter | fengyingjie | Assigned To | Christian Grothoff | ||
| Priority | immediate | Severity | major | Reproducibility | always |
| Status | resolved | Resolution | reopened | ||
| Platform | linux | OS | centos | OS Version | 7.4.1708 |
| Product Version | 0.9.75 | ||||
| Fixed in Version | Git master | ||||
| Summary | 0007757: MHD_add_response_header set by MHD_create_response_from_callback continues to execute MHD_queue_response and cannot continue to | ||||
| Description | MHD_add_response_header set by MHD_create_response_from_callback continues to execute MHD_queue_response and cannot continue to modify the response after receiving the callback asynchronously header | ||||
| Steps To Reproduce | // Create the service instance 1. MHD_start_daemon 2. struct MHD_Response *response = MHD_create_response_from_callback(MHD_SIZE_UNKNOWN, 500, &janus_http_response_callback, msg, NULL); // The callback to get the response data and copy the body of the callback into the pyload of the response janus_http_response_callback /* Suspend the connection */ 3. MHD_suspend_connection(msg-> connection); /* add response header */ 4. MHD_add_response_header(response, "Content-Type", "application/json"); /* add cors header */ 5. janus_http_add_cors_headers(msg, response); // Queue the response for transmission to the client as soon as possible, but only after MHD_AccessHandlerCallback returns. This function checks if it is legal to queue a response for a given connection at this time. It also increments the internal reference counter of the response object (the counter is automatically decremented once the response is transmitted). 6. ret = MHD_queue_response(msg-> connection, MHD_HTTP_OK, response); // Destroy the response object and associated resources (decrement the reference count) 7. MHD_destroy_response(response); The callback is received to process send_messages Assign the received response_text to the response's pyload /* resume the connection */ 9. MHD_resume_connection(msg-> connection); 10. View the microhttpd library source At step 6, the connection-> state is set from MHD_CONNECTION_HEADERS_PROCESSED to MHD_CONNECTION_FOOTERS_RECEIVED, In MHD_CONNECTION_FOOTERS_RECEIVED the thread copies the header into the header of the sent packet So at step 8, there is no way to change the header information by reruning steps 4 and 6. | ||||
| Tags | No tags attached. | ||||
|
|
At step 6, there is no way to change the header information by reexecuting steps 3 and 6. In step 6, the header is updated again; calling MHD_queue_response causes a deadlock. |
|
|
You're not allowed to call MHD_queue_response() outside of the access handler callback, and you write above that you called it just after returning from it. That's an API violation. Alas, we should guard against this instead of deadlocking. I'm adding a few lines to ensure MHD_queue_response() returns MHD_NO instead of allowing you to queue a response at an "illegal" time. |
|
|
Fix committed to master branch. |
|
|
Fix pushed in 8586d919..eb21a977. Note that now the MHD_queue_response() will simply fail (return MHD_NO), it will not allow your code to "work". |
|
|
We already have similar connection->in_idle. Shouldn't it be reused instead of new connection->in_access_handler? |
|
|
in_idle is about preventing recursion on the idle function, and it would not be nearly as tight as it is set at times when we are not in the access handler callback at all. I think we can afford an extra bit to make the check tight. |
|
|
efc136b4..c690b40e fixes the introduced regression. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2023-03-09 07:06 | fengyingjie | New Issue | |
| 2023-03-09 07:55 | fengyingjie | Note Added: 0019939 | |
| 2023-09-03 00:22 | Christian Grothoff | Note Added: 0020466 | |
| 2023-09-03 00:25 | Christian Grothoff | Changeset attached | => libmicrohttpd master eb21a977 |
| 2023-09-03 00:25 | Christian Grothoff | Note Added: 0020467 | |
| 2023-09-03 00:25 | Christian Grothoff | Assigned To | => Christian Grothoff |
| 2023-09-03 00:25 | Christian Grothoff | Status | new => resolved |
| 2023-09-03 00:25 | Christian Grothoff | Resolution | open => fixed |
| 2023-09-03 00:26 | Christian Grothoff | Note Added: 0020468 | |
| 2023-09-03 10:14 | Karlson2k | Status | resolved => feedback |
| 2023-09-03 10:14 | Karlson2k | Resolution | fixed => reopened |
| 2023-09-03 10:14 | Karlson2k | Note Added: 0020477 | |
| 2023-09-03 11:43 | Christian Grothoff | Note Added: 0020478 | |
| 2023-09-03 12:05 | Christian Grothoff | Status | feedback => resolved |
| 2023-09-03 12:05 | Christian Grothoff | Fixed in Version | => Git master |
| 2023-09-03 12:05 | Christian Grothoff | Note Added: 0020480 |
