[PATCH v3 0/2] MR3269: wininet: Handle http headers correctly when length is 0

[Hans Lehnert (＠hanslehnert)]

In HttpSendRequestW and HttpSendRequestExW, if the header pointer is not
null but the length parameter is 0, the header length should be derived
from the string length instead.

In HttpSendRequestA and HttpSendRequestExA, on the same scenario, the
function should fail instead.

--
  v3: wininet: Handle http headers correctly when length is 0
      wininet/tests: Edge cases for header lengths in http requests

https://gitlab.winehq.org/wine/wine/-/merge_requests/3269

[PATCH v3 1/2] wininet/tests: Edge cases for header lengths in http requests

[Hans Lehnert]

From: Hans Lehnert

Add tests for using 0 in header length passed to HttpSendRequestA and
HttpSendRequestW
---
 dlls/wininet/tests/http.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
index 6edab2141ae..08ae8b9e91d 100644
--- a/dlls/wininet/tests/http.c
+++ b/dlls/wininet/tests/http.c
@@ -6574,6 +6574,29 @@ static void test_large_content(int port)
     close_connection();
 }

+static void test_header_length(int port)
+{
+    test_request_t req;
+    BOOL ret;
+    char buf[1000];
+    char header[] = "Test-Header: winetest";
+
+    open_simple_request(&req, "localhost", port, "GET", "/echo_request");
+
+    ret = HttpSendRequestA(req.request, header, 0, NULL, 0);
+    ok(ret == FALSE, "HttpSendRequestA should have failed\n");
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "Expected ERROR_INVALID_PARAMETER\n");
+
+    ret = HttpSendRequestW(req.request, L"Test-Header: winetest", 0, NULL, 0);
+    ok(ret, "HttpSendRequestW failed: %lu\n", GetLastError());
+    test_status_code(req.request, 200);
+
+    receive_simple_request(req.request, buf, sizeof(buf));
+    ok(strstr(buf, header) != NULL, "custom header was not sent: %s\n", buf);
+
+    close_request(&req);
+}
+
 static void test_http_connection(void)
 {
     struct server_info si;
@@ -6640,6 +6663,7 @@ static void test_http_connection(void)
     test_persistent_connection(si.port);
     test_remove_dot_segments(si.port);
     test_large_content(si.port);
+    test_header_length(si.port);

/* send the basic request again to shutdown the server thread */
     test_basic_request(si.port, "GET", "/quit");

[PATCH v3 2/2] wininet: Handle http headers correctly when length is 0

[Hans Lehnert]

From: Hans Lehnert

In HttpSendRequestW and HttpSendRequestExW, if the header pointer is not
null but the length parameter is 0, the header length should be derived
from the string length instead.

In HttpSendRequestA and HttpSendRequestExA, on the same scenario, the
function should fail instead.
---
 dlls/wininet/http.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
index b646ddf5b69..ac3a1955270 100644
--- a/dlls/wininet/http.c
+++ b/dlls/wininet/http.c
@@ -1274,7 +1274,7 @@ BOOL WINAPI HttpAddRequestHeadersW(HINTERNET hHttpRequest,

TRACE("%p, %s, %lu, %08lx\n", hHttpRequest, debugstr_wn(lpszHeader, dwHeaderLength), dwHeaderLength, dwModifier);

-    if (!lpszHeader) 
+    if (!lpszHeader)
       return TRUE;

request = (http_request_t*) get_handle_object( hHttpRequest );
@@ -4063,13 +4063,13 @@ BOOL WINAPI HttpQueryInfoW(HINTERNET hHttpRequest, DWORD dwInfoLevel,
    	info_mod &= ~ modifier_flags[i].val;
        }
    }
-	
+
    if (info_mod) {
        TRACE(" Unknown (%08lx)", info_mod);
    }
    TRACE("\n");
     }
-    
+
     request = (http_request_t*) get_handle_object( hHttpRequest );
     if (NULL == request ||  request->hdr.htype != WH_HHTTPREQ)
     {
@@ -5043,8 +5043,13 @@ static DWORD HTTP_HttpSendRequestW(http_request_t *request, LPCWSTR lpszHeaders,
     }

/* add the headers the caller supplied */
-    if( lpszHeaders && dwHeaderLength )
+    if (lpszHeaders)
+    {
+        if (dwHeaderLength == 0)
+            dwHeaderLength = lstrlenW(lpszHeaders);
+
         HTTP_HttpAddRequestHeadersW(request, lpszHeaders, dwHeaderLength, HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE);
+    }

do
     {
@@ -5551,6 +5556,12 @@ BOOL WINAPI HttpSendRequestExA(HINTERNET hRequest,
         BuffersInW.dwStructSize = sizeof(LPINTERNET_BUFFERSW);
         if (lpBuffersIn->lpcszHeader)
         {
+            if (lpBuffersIn->dwHeadersLength == 0 && *lpBuffersIn->lpcszHeader != '\0')
+            {
+                SetLastError(ERROR_INVALID_PARAMETER);
+                return FALSE;
+            }
+
             headerlen = MultiByteToWideChar(CP_ACP,0,lpBuffersIn->lpcszHeader,
                     lpBuffersIn->dwHeadersLength,0,0);
             header = malloc(headerlen * sizeof(WCHAR));
@@ -5773,6 +5784,12 @@ BOOL WINAPI HttpSendRequestA(HINTERNET hHttpRequest, LPCSTR lpszHeaders,
     DWORD nLen=dwHeaderLength;
     if(lpszHeaders!=NULL)
     {
+        if (dwHeaderLength == 0 && *lpszHeaders != '\0')
+        {
+            SetLastError(ERROR_INVALID_PARAMETER);
+            return FALSE;
+        }
+
         nLen=MultiByteToWideChar(CP_ACP,0,lpszHeaders,dwHeaderLength,NULL,0);
         szHeaders = malloc(nLen * sizeof(WCHAR));
         MultiByteToWideChar(CP_ACP,0,lpszHeaders,dwHeaderLength,szHeaders,nLen);

[Jacek Caban (＠jacek)]

Tests need to pass after each individual commit. In this case, if we apply only the first commit, it will fail on Wine. Please reorder commits (or squash them or add todo_wine in the first patch and remove them in the second).

It would be nice to have `HttpSendRequestEx` test as well.