[PATCH] wininet: Return failure when adding HTTP header with empty value.

[Ziqing Hui]

Signed-off-by: Ziqing Hui <zhui(a)codeweavers.com>
---
 dlls/wininet/http.c       | 6 ++++++
 dlls/wininet/tests/http.c | 3 ---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
index 55698467cda..f6680790986 100644
--- a/dlls/wininet/http.c
+++ b/dlls/wininet/http.c
@@ -6139,6 +6139,12 @@ static DWORD HTTP_ProcessHeader(http_request_t *request, LPCWSTR field, LPCWSTR
     {
         HTTPHEADERW hdr;
 
+        if (!value[0])
+        {
+            LeaveCriticalSection(&request->headers_section);
+            return ERROR_INVALID_PARAMETER;
+        }
+
         hdr.lpszField = (LPWSTR)field;
         hdr.lpszValue = (LPWSTR)value;
         hdr.wFlags = hdr.wCount = 0;
diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
index ec576ae81e2..1cde6ea257c 100644
--- a/dlls/wininet/tests/http.c
+++ b/dlls/wininet/tests/http.c
@@ -2064,8 +2064,6 @@ static void HttpHeaders_test(void)
     ok(GetLastError() == ERROR_HTTP_HEADER_NOT_FOUND, "got %lu\n", GetLastError());
 
     /* Header with empty value should cause a failure */
-    todo_wine
-    {
     SetLastError(0xdeadbeef);
     ok(!HttpAddRequestHeadersA(hRequest, "EmptyTest1:", -1, HTTP_ADDREQ_FLAG_ADD), "Empty header should not be added.\n");
     ok(GetLastError() == ERROR_INVALID_PARAMETER, "Got unexpected error code %lu.\n", GetLastError());
@@ -2087,7 +2085,6 @@ static void HttpHeaders_test(void)
     ok(!HttpQueryInfoA(hRequest, HTTP_QUERY_CUSTOM | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &len, NULL),
        "Header with empty value is present.\n");
     ok(GetLastError() == ERROR_HTTP_HEADER_NOT_FOUND, "Got unexpected error code %lu.\n", GetLastError());
-    }
 
     ok(InternetCloseHandle(hRequest), "Close request handle failed\n");
 done:
-- 
2.25.1

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=114741

Your paranoid android.


=== w8adm (32 bit report) ===

wininet:
http.c:697: Test failed: req_error = 12007
http.c:708: Test failed: expected status 11 (INTERNET_STATUS_NAME_RESOLVED) 1 times, received 0 times
http.c:721: Test failed: expected status 30 (INTERNET_STATUS_SENDING_REQUEST) 1 times, received 0 times
http.c:722: Test failed: expected status 31 (INTERNET_STATUS_REQUEST_SENT) 1 times, received 0 times
http.c:723: Test failed: expected status 40 (INTERNET_STATUS_RECEIVING_RESPONSE) 1 times, received 0 times
http.c:724: Test failed: expected status 41 (INTERNET_STATUS_RESPONSE_RECEIVED) 1 times, received 0 times
http.c:733: Test failed: flags = 8, expected 0
http.c:747: Test failed: Expected any header character, got 0x00
http.c:774: Test failed: Expected 0x0000, got 7777
http.c:937: Test failed: Returned zero size in response to request complete
http.c:376: Test failed: unexpected status 10 (INTERNET_STATUS_RESOLVING_NAME)
http.c:376: Test failed: unexpected status 11 (INTERNET_STATUS_NAME_RESOLVED)
http.c:718: Test failed: expected status 10 (INTERNET_STATUS_RESOLVING_NAME) 0 times, received 1 times
http.c:719: Test failed: expected status 11 (INTERNET_STATUS_NAME_RESOLVED) 0 times, received 1 times

=== w1064v1507 (64 bit report) ===

wininet:
http.c:644: Test failed: expected status 10 (INTERNET_STATUS_RESOLVING_NAME) 0 times, received 1 times
http.c:645: Test failed: expected status 11 (INTERNET_STATUS_NAME_RESOLVED) 0 times, received 1 times
http.c:376: Test failed: unexpected status 10 (INTERNET_STATUS_RESOLVING_NAME)
http.c:376: Test failed: unexpected status 11 (INTERNET_STATUS_NAME_RESOLVED)

[Jacek Caban]

Hi Ziqing,

On 5/16/22 06:27, Ziqing Hui wrote:
> Signed-off-by: Ziqing Hui <zhui(a)codeweavers.com>
> ---
>   dlls/wininet/http.c       | 6 ++++++
>   dlls/wininet/tests/http.c | 3 ---
>   2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
> index 55698467cda..f6680790986 100644
> --- a/dlls/wininet/http.c
> +++ b/dlls/wininet/http.c
> @@ -6139,6 +6139,12 @@ static DWORD HTTP_ProcessHeader(http_request_t *request, LPCWSTR field, LPCWSTR
>       {
>           HTTPHEADERW hdr;
>
> +        if (!value[0])
> +        {
> +            LeaveCriticalSection(&request->headers_section);
> +            return ERROR_INVALID_PARAMETER;
> +        }

I think we could reject such headers even earlier, like in 
HTTP_InterpretHttpHeader.


Thanks,

Jacek