[PATCH v4 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). -- v4: 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 | 80 +++++++++++++++++++++++++++++++++++- include/http.h | 1 + 4 files changed, 114 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..e0bd5ee5939 100644 --- a/dlls/httpapi/tests/httpapi.c +++ b/dlls/httpapi/tests/httpapi.c @@ -628,12 +628,14 @@ 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, retval; char req_text[200]; unsigned int i; OVERLAPPED ovl; DWORD ret_size; HANDLE queue; + struct timeval tv; + fd_set rfds; SOCKET s; static const char post_req[] = @@ -915,6 +917,82 @@ 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); + + memset(response_buffer, 0, sizeof(response_buffer)); + ret = recv(s, response_buffer, sizeof(response_buffer), 0); + ok(ret > 0, "recv() failed.\n"); + + /* Wait up to 100ms for more data to arrive. */ + tv.tv_sec = 0; + tv.tv_usec = 100000; + FD_ZERO(&rfds); + FD_SET(s, &rfds); + retval = select(1, &rfds, NULL, NULL, &tv); + ok(retval >= 0, "select() failed.\n"); + + if (retval) + { + retval = recv(s, response_buffer + ret, sizeof(response_buffer) - ret, 0); + ok(retval > 0, "recv() failed.\n"); + ret += retval; + } + + 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"); + ok(ret_size == 6, "Got size %lu.\n", ret_size); + + 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
On Fri Aug 9 20:45:55 2024 +0000, Tom Helander wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/6216/diffs?diff_id=125796&start_sha=ed15e30a86a3dee6d5a86050da0bbc3f36b52238#eb720f0cc2b484359eb2ef3a34246987db056a5f_1010_1010) Fixed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6216#note_78238
Thanks for the notes. I fixed the braces and made a change to the test, but thinking about it more, I don't think my changes to the test fully address the concerns you raised (in particular the 100ms not being enough in some cases). I'll need to think of a better approach. I'll update this thread and re-request your review when I'm ready. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6216#note_78241
I'd suggest something like while (!strstr(response_buffer, "foobar")) recv(...) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6216#note_78247
On Sat Aug 10 00:25:17 2024 +0000, Elizabeth Figura wrote:
I'd suggest something like while (!strstr(response_buffer, "foobar")) recv(...) Oh, yeah, that'd do it. Thanks!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6216#note_78271
participants (3)
-
Elizabeth Figura (＠zfigura) -
Tom Helander
-
Tom Helander (＠thelande)