[PATCH v5 0/4] MR6216: httpapi: Implement HttpResponseSendEntityBody
This merge request implements the `httpapi.dll` `HttpResponseSendEntityBody` function and associated tests. API documentation was written by me by paraphrasing/re-writing what I had read for the function from the MSDN documentation. I am adding this function to enable the remote API component of the Space Engineers Dedicated Server. Implementation notes: - I mainly copied the relevant portion of `HttpSendHttpResponse` for `HttpResponseSendEntityBody`. - I updated the `http_response` struct to add a field `response_flags` - This field tracks the flags passed in to `HttpSendHttpResponse` and `HttpResponseSendEntityBody`. - This was needed because the clean-up steps in `http.sys` need to be skipped when `HTTP_SEND_RESPONSE_FLAG_MORE_DATA` is set. - I updated `HttpSendHttpResponse` to handle the `HTTP_SEND_RESPONSE_FLAG_MORE_DATA` flag by omitting the `Content-Length` header and setting the `response_flags` field in the `response` object passed to `http.sys`. - The `Content-Length` header is omitted by the Windows implementation and thus I omitted it here for conformance purposes. - I updated `http_send_response` in `http.sys` to skip the connection clean up steps if the `HTTP_SEND_RESPONSE_FLAG_MORE_DATA` flag was set. This works for both `HttpSendHttpResponse` and `HttpResponseSendEntityBody` as either could set the flag to continue to send more data. - It is not 100% clear to me what the cleanup steps are doing. In particular, I am uncertain what lines 1011-1016 and 1023-1027 are doing, but based on context it seemed appropriate to keep them with the other connection cleanup steps. For testing, I duplicated the existing `HttpSendHttpResponse` test but modified it to set the `HTTP_SEND_RESPONSE_FLAG_MORE_DATA` flag and then followed up with a call to `HttpResponseSendEntityBody` with new data. The validation ensures that data from both the initial response and follow up response are contained in the final response obtained via `recv()`. On Windows 11 this happens immediately without any delay, but in Wine I had to add a `Sleep(100)` to give enough time for the follow up response to be received in the socket buffer by the time the `recv()` call was made in the test. I could have looped the `recv()` call to collect all of the response, but that felt more complicated than adding a minor delay. The tests were verified against Windows 11 Pro (build 22621.3880). -- v5: httpapi: Implement HttpSendResponseEntityBody httpapi: Handle HTTP_SEND_RESPONSE_FLAG_MORE_DATA flag http.sys: Skip clean up if HTTP_SEND_RESPONSE_FLAG_MORE_DATA is set httpapi: Add tests for HttpResponseSendEntityBody https://gitlab.winehq.org/wine/wine/-/merge_requests/6216
From: Tom Helander <thomas.helander(a)gmail.com> Verified against Windows 11 Pro (build 22621.3880) --- dlls/httpapi/httpapi.spec | 2 +- dlls/httpapi/httpapi_main.c | 33 +++++++++++++ dlls/httpapi/tests/httpapi.c | 90 +++++++++++++++++++++++++++++++++++- include/http.h | 1 + 4 files changed, 124 insertions(+), 2 deletions(-) diff --git a/dlls/httpapi/httpapi.spec b/dlls/httpapi/httpapi.spec index df18a123389..7a5f2754163 100644 --- a/dlls/httpapi/httpapi.spec +++ b/dlls/httpapi/httpapi.spec @@ -25,7 +25,7 @@ @ stdcall HttpRemoveUrl(ptr wstr) @ stdcall HttpRemoveUrlFromUrlGroup(int64 wstr long) @ stdcall HttpSendHttpResponse(ptr int64 long ptr ptr ptr ptr long ptr ptr) -@ stub HttpSendResponseEntityBody +@ stdcall HttpSendResponseEntityBody(ptr int64 long long ptr ptr ptr long ptr ptr) @ stdcall HttpSetRequestQueueProperty(ptr long ptr long long ptr) @ stdcall HttpSetServerSessionProperty(int64 long ptr long) @ stdcall HttpSetServiceConfiguration(ptr long ptr long ptr) diff --git a/dlls/httpapi/httpapi_main.c b/dlls/httpapi/httpapi_main.c index b320e1c1195..3b034cd9c22 100644 --- a/dlls/httpapi/httpapi_main.c +++ b/dlls/httpapi/httpapi_main.c @@ -515,6 +515,39 @@ ULONG WINAPI HttpSendHttpResponse(HANDLE queue, HTTP_REQUEST_ID id, ULONG flags, return ret; } + +/*********************************************************************** + * HttpSendResponseEntityBody (HTTPAPI.@) + * + * Sends entity-body data for a response. + * + * PARAMS + * queue [I] The request queue handle + * id [I] The ID of the request to which this response corresponds + * flags [I] Flags to control the response + * entity_chunk_count [I] The number of entities pointed to by entity_chunks + * entity_chunks [I] The entities to be sent + * ret_size [O] The number of bytes sent + * reserved1 [I] Reserved, must be NULL + * reserved2 [I] Reserved, must be zero + * ovl [I] Must be set to an OVERLAP pointer when making async calls + * log_data [I] Optional log data structure for logging the call + * + * RETURNS + * NO_ERROR on success, or an error code on failure. + */ +ULONG WINAPI HttpSendResponseEntityBody(HANDLE queue, HTTP_REQUEST_ID id, + ULONG flags, USHORT entity_chunk_count, PHTTP_DATA_CHUNK entity_chunks, + ULONG *ret_size, void *reserved1, ULONG reserved2, OVERLAPPED *ovl, + HTTP_LOG_DATA *log_data) +{ + FIXME("queue %p, id %s, flags %#lx, entity_chunk_count %u, entity_chunks %p, " + "ret_size %p, reserved1 %p, reserved2 %#lx, ovl %p, log_data %p: stub!\n", + queue, wine_dbgstr_longlong(id), flags, entity_chunk_count, entity_chunks, + ret_size, reserved1, reserved2, ovl, log_data); + return ERROR_CALL_NOT_IMPLEMENTED; +} + struct url_group { struct list entry, session_entry; diff --git a/dlls/httpapi/tests/httpapi.c b/dlls/httpapi/tests/httpapi.c index 9db21635d3a..06d67f203bb 100644 --- a/dlls/httpapi/tests/httpapi.c +++ b/dlls/httpapi/tests/httpapi.c @@ -628,7 +628,7 @@ static void test_v1_entity_body(void) HTTP_RESPONSE_V1 response = {}; HTTP_DATA_CHUNK chunks[2] = {}; unsigned short port; - int ret, chunk_size; + int ret, chunk_size, len, max_attempts; char req_text[200]; unsigned int i; OVERLAPPED ovl; @@ -915,6 +915,94 @@ static void test_v1_entity_body(void) send_response_v1(queue, req->RequestId, s); + /* Test HttpSendResponseEntityBody(). */ + + ret = HttpSendResponseEntityBody(queue, HTTP_NULL_ID, 0, ARRAY_SIZE(chunks), chunks, NULL, NULL, 0, NULL, NULL); + ok(ret == ERROR_CONNECTION_INVALID, "Got error %u.\n", ret); + + sprintf(req_text, post_req, port); + ret = send(s, req_text, strlen(req_text) + 1, 0); + ok(ret == strlen(req_text) + 1, "send() returned %d.\n", ret); + + Sleep(100); + + memset(req_buffer, 0xcc, sizeof(req_buffer)); + ret = HttpReceiveHttpRequest(queue, HTTP_NULL_ID, 0, (HTTP_REQUEST *)req, sizeof(req_buffer), &ret_size, NULL); + ok(!ret, "Got error %u.\n", ret); + ok(ret_size > sizeof(*req), "Got size %lu.\n", ret_size); + + memset(&response, 0, sizeof(response)); + + response.StatusCode = 418; + response.pReason = "I'm a teapot"; + response.ReasonLength = 12; + response.EntityChunkCount = ARRAY_SIZE(chunks); + response.pEntityChunks = chunks; + chunks[0].DataChunkType = HttpDataChunkFromMemory; + chunks[0].FromMemory.pBuffer = (void *)"pong"; + chunks[0].FromMemory.BufferLength = 4; + chunks[1].DataChunkType = HttpDataChunkFromMemory; + chunks[1].FromMemory.pBuffer = (void *)"pang"; + chunks[1].FromMemory.BufferLength = 4; + ret = HttpSendHttpResponse(queue, req->RequestId, HTTP_SEND_RESPONSE_FLAG_MORE_DATA, (HTTP_RESPONSE *)&response, NULL, NULL, NULL, 0, NULL, NULL); + ok(!ret, "Got error %u.\n", ret); + + ret = HttpSendResponseEntityBody(queue, req->RequestId, HTTP_SEND_RESPONSE_FLAG_MORE_DATA, 0, NULL, NULL, NULL, 0, NULL, NULL); + ok(!ret, "Got error %u.\n", ret); + + ret_size = 0xdeadbeef; + memset(chunks, 0, sizeof(chunks)); + chunks[0].DataChunkType = HttpDataChunkFromMemory; + chunks[0].FromMemory.pBuffer = (void *)"foo"; + chunks[0].FromMemory.BufferLength = 3; + chunks[1].DataChunkType = HttpDataChunkFromMemory; + chunks[1].FromMemory.pBuffer = (void *)"bar"; + chunks[1].FromMemory.BufferLength = 3; + ret = HttpSendResponseEntityBody(queue, req->RequestId, 0, ARRAY_SIZE(chunks), chunks, &ret_size, NULL, 0, NULL, NULL); + ok(!ret, "Got error %u.\n", ret); + ok(ret_size == 6, "Got size %lu.\n", ret_size); + + memset(response_buffer, 0, sizeof(response_buffer)); + + /* + * Loop on recv() for at most 1 second to ensure we get the full response + * while ensuring we do not block forever if the expected response is never + * sent. + */ + len = 0; + max_attempts = 100; + for (i = 0; i < max_attempts && !strstr(response_buffer, "foobar"); ++i) + { + struct timeval tv; + fd_set rfds; + + tv.tv_sec = 0; + tv.tv_usec = 10000; + FD_ZERO(&rfds); + FD_SET(s, &rfds); + + ret = select(1, &rfds, NULL, NULL, &tv); + ok(ret >= 0, "select() failed, ret = %d.\n", ret); + + if (ret > 0) + { + ret = recv(s, response_buffer + len, sizeof(response_buffer) - len, 0); + ok(ret > 0, "recv() failed.\n"); + len += ret; + } + } + ret = len; + + if (winetest_debug > 1) + trace("%.*s\n", ret, response_buffer); + ok(!strncmp(response_buffer, "HTTP/1.1 418 I'm a teapot\r\n", 27), "Got incorrect status line.\n"); + ok(!strstr(response_buffer, "\r\nContent-Length:"), "Unexpected Content-Length header.\n"); + ok(!!strstr(response_buffer, "\r\nDate:"), "Missing Date header.\n"); + ok(!memcmp(response_buffer + ret - 18, "\r\n\r\npongpangfoobar", 18), "Response did not end with entity data.\n"); + + ret = HttpReceiveHttpRequest(queue, req->RequestId, 0, (HTTP_REQUEST *)req, sizeof(req_buffer), &ret_size, NULL); + ok(ret == ERROR_CONNECTION_INVALID, "Got error %u.\n", ret); + CloseHandle(ovl.hEvent); ret = remove_url_v1(queue, port); ok(!ret, "Got error %u.\n", ret); diff --git a/include/http.h b/include/http.h index ce2a1b0588c..5a45f8fea9a 100644 --- a/include/http.h +++ b/include/http.h @@ -484,6 +484,7 @@ HTTPAPI_LINKAGE ULONG WINAPI HttpReceiveRequestEntityBody(HANDLE queue, HTTP_REQ HTTPAPI_LINKAGE ULONG WINAPI HttpRemoveUrl(HANDLE queue, const WCHAR *url); HTTPAPI_LINKAGE ULONG WINAPI HttpRemoveUrlFromUrlGroup(HTTP_URL_GROUP_ID id, const WCHAR *url, ULONG flags); HTTPAPI_LINKAGE ULONG WINAPI HttpSendHttpResponse(HANDLE queue, HTTP_REQUEST_ID id, ULONG flags, HTTP_RESPONSE *response, HTTP_CACHE_POLICY *cache_policy, ULONG *ret_size, void *reserved1, ULONG reserved2, OVERLAPPED *ovl, HTTP_LOG_DATA *log_data); +HTTPAPI_LINKAGE ULONG WINAPI HttpSendResponseEntityBody(HANDLE queue, HTTP_REQUEST_ID id, ULONG flags, USHORT entity_chunk_count, PHTTP_DATA_CHUNK entity_chunks, ULONG *ret_size, void *reserved1, ULONG reserved2, OVERLAPPED *ovl, HTTP_LOG_DATA *log_data); HTTPAPI_LINKAGE ULONG WINAPI HttpSetRequestQueueProperty(HANDLE queue, HTTP_SERVER_PROPERTY property, void *value, ULONG length, ULONG reserved1, void *reserved2); HTTPAPI_LINKAGE ULONG WINAPI HttpSetServerSessionProperty(HTTP_SERVER_SESSION_ID id, HTTP_SERVER_PROPERTY property, void *value, ULONG size); HTTPAPI_LINKAGE ULONG WINAPI HttpSetServiceConfiguration(HANDLE,HTTP_SERVICE_CONFIG_ID,PVOID,ULONG,LPOVERLAPPED); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6216
From: Tom Helander <thomas.helander(a)gmail.com> --- dlls/http.sys/http.c | 35 ++++++++++++++++++++--------------- include/wine/http.h | 1 + 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/dlls/http.sys/http.c b/dlls/http.sys/http.c index 093a00f1b12..edf099b43ab 100644 --- a/dlls/http.sys/http.c +++ b/dlls/http.sys/http.c @@ -638,7 +638,7 @@ static void receive_data(struct connection *conn) if (conn->available) return; /* waiting for an HttpReceiveHttpRequest() call */ if (conn->req_id != HTTP_NULL_ID) - return; /* waiting for an HttpSendHttpResponse() call */ + return; /* waiting for an HttpSendHttpResponse() or HttpSendResponseEntityBody() call */ TRACE("Received %u bytes of data.\n", len); @@ -1006,23 +1006,28 @@ static NTSTATUS http_send_response(struct request_queue *queue, IRP *irp) { if (send(conn->socket, response->buffer, response->len, 0) >= 0) { - if (conn->content_len) + /* Clean up the connection if we are not sending more response data. */ + if (response->response_flags != HTTP_SEND_RESPONSE_FLAG_MORE_DATA) { - /* Discard whatever entity body is left. */ - memmove(conn->buffer, conn->buffer + conn->content_len, conn->len - conn->content_len); - conn->len -= conn->content_len; + if (conn->content_len) + { + /* Discard whatever entity body is left. */ + memmove(conn->buffer, conn->buffer + conn->content_len, conn->len - conn->content_len); + conn->len -= conn->content_len; + } + + conn->queue = NULL; + conn->req_id = HTTP_NULL_ID; + WSAEventSelect(conn->socket, request_event, FD_READ | FD_CLOSE); + + /* We might have another request already in the buffer. */ + if (parse_request(conn) < 0) + { + WARN("Failed to parse request; shutting down connection.\n"); + send_400(conn); + } } - - conn->queue = NULL; - conn->req_id = HTTP_NULL_ID; - WSAEventSelect(conn->socket, request_event, FD_READ | FD_CLOSE); irp->IoStatus.Information = response->len; - /* We might have another request already in the buffer. */ - if (parse_request(conn) < 0) - { - WARN("Failed to parse request; shutting down connection.\n"); - send_400(conn); - } } else { diff --git a/include/wine/http.h b/include/wine/http.h index d8039dea071..4a44392bdbf 100644 --- a/include/wine/http.h +++ b/include/wine/http.h @@ -46,6 +46,7 @@ struct http_receive_request_params struct http_response { HTTP_REQUEST_ID id; + ULONG response_flags; int len; char buffer[1]; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6216
From: Tom Helander <thomas.helander(a)gmail.com> --- dlls/httpapi/httpapi_main.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dlls/httpapi/httpapi_main.c b/dlls/httpapi/httpapi_main.c index 3b034cd9c22..19912428034 100644 --- a/dlls/httpapi/httpapi_main.c +++ b/dlls/httpapi/httpapi_main.c @@ -430,8 +430,8 @@ ULONG WINAPI HttpSendHttpResponse(HANDLE queue, HTTP_REQUEST_ID id, ULONG flags, queue, wine_dbgstr_longlong(id), flags, response, cache_policy, ret_size, reserved1, reserved2, ovl, log_data); - if (flags) - FIXME("Unhandled flags %#lx.\n", flags); + if (flags & ~HTTP_SEND_RESPONSE_FLAG_MORE_DATA) + FIXME("Unhandled flags %#lx.\n", flags & ~HTTP_SEND_RESPONSE_FLAG_MORE_DATA); if (response->s.Flags) FIXME("Unhandled response flags %#lx.\n", response->s.Flags); if (cache_policy) @@ -456,7 +456,7 @@ ULONG WINAPI HttpSendHttpResponse(HANDLE queue, HTTP_REQUEST_ID id, ULONG flags, len += 37; else if (response->s.Headers.KnownHeaders[i].RawValueLength) len += strlen(header_names[i]) + 2 + response->s.Headers.KnownHeaders[i].RawValueLength + 2; - else if (i == HttpHeaderContentLength) + else if (i == HttpHeaderContentLength && !(flags & HTTP_SEND_RESPONSE_FLAG_MORE_DATA)) { char dummy[12]; len += strlen(header_names[i]) + 2 + sprintf(dummy, "%d", body_len) + 2; @@ -472,6 +472,7 @@ ULONG WINAPI HttpSendHttpResponse(HANDLE queue, HTTP_REQUEST_ID id, ULONG flags, if (!(buffer = malloc(offsetof(struct http_response, buffer[len])))) return ERROR_OUTOFMEMORY; buffer->id = id; + buffer->response_flags = flags; buffer->len = len; sprintf(buffer->buffer, "HTTP/1.1 %u %.*s\r\n", response->s.StatusCode, response->s.ReasonLength, response->s.pReason); @@ -484,7 +485,7 @@ ULONG WINAPI HttpSendHttpResponse(HANDLE queue, HTTP_REQUEST_ID id, ULONG flags, else if (header->RawValueLength) sprintf(buffer->buffer + strlen(buffer->buffer), "%s: %.*s\r\n", header_names[i], header->RawValueLength, header->pRawValue); - else if (i == HttpHeaderContentLength) + else if (i == HttpHeaderContentLength && !(flags & HTTP_SEND_RESPONSE_FLAG_MORE_DATA)) sprintf(buffer->buffer + strlen(buffer->buffer), "Content-Length: %d\r\n", body_len); } for (i = 0; i < response->s.Headers.UnknownHeaderCount; ++i) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6216
From: Tom Helander <thomas.helander(a)gmail.com> --- dlls/httpapi/httpapi_main.c | 60 ++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/dlls/httpapi/httpapi_main.c b/dlls/httpapi/httpapi_main.c index 19912428034..d1c2667be2e 100644 --- a/dlls/httpapi/httpapi_main.c +++ b/dlls/httpapi/httpapi_main.c @@ -516,7 +516,6 @@ ULONG WINAPI HttpSendHttpResponse(HANDLE queue, HTTP_REQUEST_ID id, ULONG flags, return ret; } - /*********************************************************************** * HttpSendResponseEntityBody (HTTPAPI.@) * @@ -542,11 +541,64 @@ ULONG WINAPI HttpSendResponseEntityBody(HANDLE queue, HTTP_REQUEST_ID id, ULONG *ret_size, void *reserved1, ULONG reserved2, OVERLAPPED *ovl, HTTP_LOG_DATA *log_data) { - FIXME("queue %p, id %s, flags %#lx, entity_chunk_count %u, entity_chunks %p, " - "ret_size %p, reserved1 %p, reserved2 %#lx, ovl %p, log_data %p: stub!\n", + struct http_response *buffer; + OVERLAPPED dummy_ovl = {}; + ULONG ret = NO_ERROR; + int len = 0; + char *p; + USHORT i; + + TRACE("queue %p, id %s, flags %#lx, entity_chunk_count %u, entity_chunks %p, " + "ret_size %p, reserved1 %p, reserved2 %#lx, ovl %p, log_data %p\n", queue, wine_dbgstr_longlong(id), flags, entity_chunk_count, entity_chunks, ret_size, reserved1, reserved2, ovl, log_data); - return ERROR_CALL_NOT_IMPLEMENTED; + + if (!id) + return ERROR_CONNECTION_INVALID; + + if (flags & ~HTTP_SEND_RESPONSE_FLAG_MORE_DATA) + FIXME("Unhandled flags %#lx.\n", flags & ~HTTP_SEND_RESPONSE_FLAG_MORE_DATA); + if (log_data) + WARN("Ignoring log_data.\n"); + + /* Compute the length of the body. */ + for (i = 0; i < entity_chunk_count; ++i) + { + if (entity_chunks[i].DataChunkType != HttpDataChunkFromMemory) + { + FIXME("Unhandled data chunk type %u.\n", entity_chunks[i].DataChunkType); + return ERROR_CALL_NOT_IMPLEMENTED; + } + len += entity_chunks[i].FromMemory.BufferLength; + } + + if (!(buffer = malloc(offsetof(struct http_response, buffer[len])))) + return ERROR_OUTOFMEMORY; + buffer->id = id; + buffer->response_flags = flags; + buffer->len = len; + + p = buffer->buffer; + for (i = 0; i < entity_chunk_count; ++i) + { + const HTTP_DATA_CHUNK *chunk = &entity_chunks[i]; + memcpy(p, chunk->FromMemory.pBuffer, chunk->FromMemory.BufferLength); + p += chunk->FromMemory.BufferLength; + } + + if (!ovl) + { + ovl = &dummy_ovl; + if (ret_size) + *ret_size = len; + } + + if (!DeviceIoControl(queue, IOCTL_HTTP_SEND_RESPONSE, buffer, + offsetof(struct http_response, buffer[len]), NULL, 0, NULL, ovl)) + ret = GetLastError(); + + free(buffer); + return ret; } struct url_group -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6216
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147676 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000032F00E8, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
On Sat Aug 10 00:25:17 2024 +0000, Tom Helander wrote:
Oh, yeah, that'd do it. Thanks! I've rewritten the test to loop on `recv()` as you mentioned, but I kept a timeout on the loop via `select()` to avoid blocking the test forever in the event the data is never received, such as the case when only the stub of `HttpSendResponseEntityBody` is implemented. With this implementation, a failed test will take \~1 second, but a success case takes 30-40ms on my machine (4 iterations of the loop to get all of the response).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6216#note_78433
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6216
participants (4)
-
Elizabeth Figura (＠zfigura) -
Marvin -
Tom Helander
-
Tom Helander (＠thelande)