From 13effc62f85510876a9a6a3adce312683ae7c111 Mon Sep 17 00:00:00 2001 From: Sebastian Lackner Date: Sat, 16 May 2015 03:43:53 +0200 Subject: [PATCH] wininet-Cleanup: Added various additional fixes for wininet header handling. --- debian/changelog | 1 + patches/patchinstall.sh | 2 + ...g-header-fields-should-fail-if-they-.patch | 263 ++++++++++++++++++ 3 files changed, 266 insertions(+) create mode 100644 patches/wininet-Cleanup/0013-wininet-Replacing-header-fields-should-fail-if-they-.patch diff --git a/debian/changelog b/debian/changelog index 00fe3192..d436b4d3 100644 --- a/debian/changelog +++ b/debian/changelog @@ -40,6 +40,7 @@ wine-staging (1.7.43) UNRELEASED; urgency=low handling, in preparation of various cleanup patches. * Added patch to add HTTP Host header in HttpSendRequest instead of HttpOpenRequest. + * Added various additional fixes for wininet header handling. * Removed patch to use lockfree implementation for FD cache (accepted upstream). * Removed patch to properly handle closing sockets during a select call diff --git a/patches/patchinstall.sh b/patches/patchinstall.sh index 7393ea7b..e836a473 100755 --- a/patches/patchinstall.sh +++ b/patches/patchinstall.sh @@ -5520,6 +5520,7 @@ if test "$enable_wininet_Cleanup" -eq 1; then patch_apply wininet-Cleanup/0010-rpcrt4-Fix-arguments-of-HttpAddRequestHeaders.patch patch_apply wininet-Cleanup/0011-wininet-Fix-arguments-of-HttpAddRequestHeaders.patch patch_apply wininet-Cleanup/0012-wininet-Strip-filename-if-no-path-is-set-in-cookie.patch + patch_apply wininet-Cleanup/0013-wininet-Replacing-header-fields-should-fail-if-they-.patch ( echo '+ { "Michael Müller", "wininet: Fix memory leak by not calling get_cookie_header twice.", 1 },'; echo '+ { "Michael Müller", "wininet/tests: Add more tests for cookies.", 1 },'; @@ -5533,6 +5534,7 @@ if test "$enable_wininet_Cleanup" -eq 1; then echo '+ { "Michael Müller", "rpcrt4: Fix arguments of HttpAddRequestHeaders.", 1 },'; echo '+ { "Michael Müller", "wininet: Fix arguments of HttpAddRequestHeaders.", 1 },'; echo '+ { "Michael Müller", "wininet: Strip filename if no path is set in cookie.", 1 },'; + echo '+ { "Michael Müller", "wininet: Replacing header fields should fail if they do not exist yet.", 1 },'; ) >> "$patchlist" fi diff --git a/patches/wininet-Cleanup/0013-wininet-Replacing-header-fields-should-fail-if-they-.patch b/patches/wininet-Cleanup/0013-wininet-Replacing-header-fields-should-fail-if-they-.patch new file mode 100644 index 00000000..0b0905de --- /dev/null +++ b/patches/wininet-Cleanup/0013-wininet-Replacing-header-fields-should-fail-if-they-.patch @@ -0,0 +1,263 @@ +From 5d96dbdcf3e2ade7aad2745f8c48a3e56ba264c2 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Michael=20M=C3=BCller?= +Date: Sat, 16 May 2015 03:16:15 +0200 +Subject: wininet: Replacing header fields should fail if they do not exist + yet. + +A lot of details are not properly covered by tests yet and were +marked with FIXME comments. The implementation was written in such +a way that it behaves identical to the old code in such situations. +--- + dlls/wininet/http.c | 185 +++++++++++++++++++++++----------------------- + dlls/wininet/tests/http.c | 6 +- + 2 files changed, 96 insertions(+), 95 deletions(-) + +diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c +index 0b0f0b1..b32f9a3 100644 +--- a/dlls/wininet/http.c ++++ b/dlls/wininet/http.c +@@ -6106,127 +6106,128 @@ static LPWSTR * HTTP_InterpretHttpHeader(LPCWSTR buffer) + + static DWORD HTTP_ProcessHeader(http_request_t *request, LPCWSTR field, LPCWSTR value, DWORD dwModifier) + { +- LPHTTPHEADERW lphttpHdr = NULL; ++ LPHTTPHEADERW lphttpHdr; + INT index; + BOOL request_only = !!(dwModifier & HTTP_ADDHDR_FLAG_REQ); +- DWORD res = ERROR_HTTP_INVALID_HEADER; ++ DWORD res = ERROR_SUCCESS; + + TRACE("--> %s: %s - 0x%08x\n", debugstr_w(field), debugstr_w(value), dwModifier); + + EnterCriticalSection( &request->headers_section ); + +- /* REPLACE wins out over ADD */ +- if (dwModifier & HTTP_ADDHDR_FLAG_REPLACE) +- dwModifier &= ~HTTP_ADDHDR_FLAG_ADD; +- +- if (dwModifier & HTTP_ADDHDR_FLAG_ADD) +- index = -1; +- else +- index = HTTP_GetCustomHeaderIndex(request, field, 0, request_only); +- ++ index = HTTP_GetCustomHeaderIndex(request, field, 0, request_only); + if (index >= 0) + { +- if (dwModifier & HTTP_ADDHDR_FLAG_ADD_IF_NEW) +- { +- LeaveCriticalSection( &request->headers_section ); +- return ERROR_HTTP_INVALID_HEADER; +- } + lphttpHdr = &request->custHeaders[index]; +- } +- else if (value) +- { +- HTTPHEADERW hdr; + +- hdr.lpszField = (LPWSTR)field; +- hdr.lpszValue = (LPWSTR)value; +- hdr.wFlags = hdr.wCount = 0; ++ /* replace existing header if FLAG_REPLACE is given */ ++ if (dwModifier & HTTP_ADDHDR_FLAG_REPLACE) ++ { ++ HTTP_DeleteCustomHeader( request, index ); + +- if (dwModifier & HTTP_ADDHDR_FLAG_REQ) +- hdr.wFlags |= HDR_ISREQUEST; ++ if (value && value[0]) ++ { ++ HTTPHEADERW hdr; + +- res = HTTP_InsertCustomHeader(request, &hdr); +- LeaveCriticalSection( &request->headers_section ); +- return res; +- } +- /* no value to delete */ +- else +- { +- LeaveCriticalSection( &request->headers_section ); +- return ERROR_SUCCESS; +- } ++ hdr.lpszField = (LPWSTR)field; ++ hdr.lpszValue = (LPWSTR)value; ++ hdr.wFlags = hdr.wCount = 0; + +- if (dwModifier & HTTP_ADDHDR_FLAG_REQ) +- lphttpHdr->wFlags |= HDR_ISREQUEST; +- else +- lphttpHdr->wFlags &= ~HDR_ISREQUEST; ++ if (dwModifier & HTTP_ADDHDR_FLAG_REQ) ++ hdr.wFlags |= HDR_ISREQUEST; + +- if (dwModifier & HTTP_ADDHDR_FLAG_REPLACE) +- { +- HTTP_DeleteCustomHeader( request, index ); ++ res = HTTP_InsertCustomHeader( request, &hdr ); ++ } + +- if (value && value[0]) ++ goto out; ++ } ++ ++ /* do not add new header if FLAG_ADD_IF_NEW is set */ ++ if (dwModifier & HTTP_ADDHDR_FLAG_ADD_IF_NEW) + { +- HTTPHEADERW hdr; ++ res = ERROR_HTTP_INVALID_HEADER; /* FIXME */ ++ goto out; ++ } + +- hdr.lpszField = (LPWSTR)field; +- hdr.lpszValue = (LPWSTR)value; +- hdr.wFlags = hdr.wCount = 0; ++ /* handle appending to existing header */ ++ if (dwModifier & COALESCEFLAGS) ++ { ++ LPWSTR lpsztmp; ++ WCHAR ch = 0; ++ INT len = 0; ++ INT origlen = strlenW(lphttpHdr->lpszValue); ++ INT valuelen = strlenW(value); + ++ /* FIXME: Should it really clear HDR_ISREQUEST? */ + if (dwModifier & HTTP_ADDHDR_FLAG_REQ) +- hdr.wFlags |= HDR_ISREQUEST; +- +- res = HTTP_InsertCustomHeader(request, &hdr); +- LeaveCriticalSection( &request->headers_section ); +- return res; +- } ++ lphttpHdr->wFlags |= HDR_ISREQUEST; ++ else ++ lphttpHdr->wFlags &= ~HDR_ISREQUEST; + +- LeaveCriticalSection( &request->headers_section ); +- return ERROR_SUCCESS; +- } +- else if (dwModifier & COALESCEFLAGS) +- { +- LPWSTR lpsztmp; +- WCHAR ch = 0; +- INT len = 0; +- INT origlen = strlenW(lphttpHdr->lpszValue); +- INT valuelen = strlenW(value); ++ if (dwModifier & HTTP_ADDHDR_FLAG_COALESCE_WITH_COMMA) ++ { ++ ch = ','; ++ lphttpHdr->wFlags |= HDR_COMMADELIMITED; ++ } ++ else if (dwModifier & HTTP_ADDHDR_FLAG_COALESCE_WITH_SEMICOLON) ++ { ++ ch = ';'; ++ lphttpHdr->wFlags |= HDR_COMMADELIMITED; ++ } + +- if (dwModifier & HTTP_ADDHDR_FLAG_COALESCE_WITH_COMMA) +- { +- ch = ','; +- lphttpHdr->wFlags |= HDR_COMMADELIMITED; +- } +- else if (dwModifier & HTTP_ADDHDR_FLAG_COALESCE_WITH_SEMICOLON) +- { +- ch = ';'; +- lphttpHdr->wFlags |= HDR_COMMADELIMITED; +- } ++ len = origlen + valuelen + ((ch > 0) ? 2 : 0); + +- len = origlen + valuelen + ((ch > 0) ? 2 : 0); ++ lpsztmp = heap_realloc(lphttpHdr->lpszValue, (len+1)*sizeof(WCHAR)); ++ if (lpsztmp) ++ { ++ lphttpHdr->lpszValue = lpsztmp; ++ /* FIXME: Increment lphttpHdr->wCount. Perhaps lpszValue should be an array */ ++ if (ch > 0) ++ { ++ lphttpHdr->lpszValue[origlen] = ch; ++ origlen++; ++ lphttpHdr->lpszValue[origlen] = ' '; ++ origlen++; ++ } + +- lpsztmp = heap_realloc(lphttpHdr->lpszValue, (len+1)*sizeof(WCHAR)); +- if (lpsztmp) +- { +- lphttpHdr->lpszValue = lpsztmp; +- /* FIXME: Increment lphttpHdr->wCount. Perhaps lpszValue should be an array */ +- if (ch > 0) ++ memcpy(&lphttpHdr->lpszValue[origlen], value, valuelen*sizeof(WCHAR)); ++ lphttpHdr->lpszValue[len] = '\0'; ++ } ++ else + { +- lphttpHdr->lpszValue[origlen] = ch; +- origlen++; +- lphttpHdr->lpszValue[origlen] = ' '; +- origlen++; ++ WARN("heap_realloc (%d bytes) failed\n",len+1); ++ res = ERROR_OUTOFMEMORY; + } + +- memcpy(&lphttpHdr->lpszValue[origlen], value, valuelen*sizeof(WCHAR)); +- lphttpHdr->lpszValue[len] = '\0'; +- res = ERROR_SUCCESS; +- } +- else +- { +- WARN("heap_realloc (%d bytes) failed\n",len+1); +- res = ERROR_OUTOFMEMORY; ++ goto out; + } + } ++ ++ /* FIXME: What about other combinations? */ ++ if ((dwModifier & ~HTTP_ADDHDR_FLAG_REQ) == HTTP_ADDHDR_FLAG_REPLACE) ++ { ++ res = ERROR_HTTP_HEADER_NOT_FOUND; ++ goto out; ++ } ++ ++ /* FIXME: What if value == ""? */ ++ if (value) ++ { ++ HTTPHEADERW hdr; ++ ++ hdr.lpszField = (LPWSTR)field; ++ hdr.lpszValue = (LPWSTR)value; ++ hdr.wFlags = hdr.wCount = 0; ++ ++ if (dwModifier & HTTP_ADDHDR_FLAG_REQ) ++ hdr.wFlags |= HDR_ISREQUEST; ++ ++ res = HTTP_InsertCustomHeader( request, &hdr ); ++ goto out; ++ } ++ ++ /* FIXME: What if value == NULL? */ ++out: + TRACE("<-- %d\n", res); + LeaveCriticalSection( &request->headers_section ); + return res; +diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c +index 87f0668..7444a67 100644 +--- a/dlls/wininet/tests/http.c ++++ b/dlls/wininet/tests/http.c +@@ -3100,13 +3100,13 @@ static void test_header_override(int port) + + ret = HttpAddRequestHeadersA(req, host_header_override, ~0u, HTTP_ADDREQ_FLAG_REPLACE); + err = GetLastError(); +- todo_wine ok(!ret, "HttpAddRequestHeaders succeeded\n"); +- todo_wine ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "Expected error ERROR_HTTP_HEADER_NOT_FOUND, got %d\n", err); ++ ok(!ret, "HttpAddRequestHeaders succeeded\n"); ++ ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "Expected error ERROR_HTTP_HEADER_NOT_FOUND, got %d\n", err); + + ret = HttpSendRequestA(req, NULL, 0, NULL, 0); + ok(ret, "HttpSendRequest failed\n"); + +- test_status_code_todo(req, 400); ++ test_status_code(req, 400); + InternetCloseHandle(req); + + InternetSetCookieA("http://localhost", "cookie", "biscuit"); +-- +2.4.0 +