From b3b5c8a670191367a217bb5d4fe22af1afdad612 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Jun 2022 14:54:50 +0100 Subject: [PATCH] Fix wxWebResponse::GetHeader() and GetURL() in wxMSW These functions returned strings of wrong size, with some junk after the end of the actual string, due to a confusion between the size of the buffer in bytes used by the WinHTTP functions and the length of the buffer in (wide) characters used by wxWCharBuffer. Fix this and add a unit test checking that the expected header is returned. See #22549. Closes #22181. --- src/msw/webrequest_winhttp.cpp | 19 +++++++++++++++++-- tests/net/webrequest.cpp | 23 +++++++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/msw/webrequest_winhttp.cpp b/src/msw/webrequest_winhttp.cpp index d31c12ef05..3608b1483b 100644 --- a/src/msw/webrequest_winhttp.cpp +++ b/src/msw/webrequest_winhttp.cpp @@ -152,7 +152,15 @@ static wxString wxWinHTTPQueryHeaderString(HINTERNET hRequest, DWORD dwInfoLevel WINHTTP_NO_HEADER_INDEX); if ( ::GetLastError() == ERROR_INSUFFICIENT_BUFFER ) { - wxWCharBuffer resBuf(bufferLen); + // Buffer length is in bytes, including the terminating (wide) NUL, but + // wxWCharBuffer needs the size in characters and adds NUL itself. + if ( !bufferLen || (bufferLen % sizeof(wchar_t)) ) + { + wxLogDebug("Unexpected size of header %s: %lu", pwszName, bufferLen); + return wxString(); + } + + wxWCharBuffer resBuf(bufferLen / sizeof(wchar_t) - 1); if ( wxWinHTTP::WinHttpQueryHeaders(hRequest, dwInfoLevel, pwszName, resBuf.data(), &bufferLen, WINHTTP_NO_HEADER_INDEX) ) @@ -171,7 +179,14 @@ static wxString wxWinHTTPQueryOptionString(HINTERNET hInternet, DWORD dwOption) wxWinHTTP::WinHttpQueryOption(hInternet, dwOption, NULL, &bufferLen); if ( ::GetLastError() == ERROR_INSUFFICIENT_BUFFER ) { - wxWCharBuffer resBuf(bufferLen); + // Same as above: convert length in bytes into size in characters. + if ( !bufferLen || (bufferLen % sizeof(wchar_t)) ) + { + wxLogDebug("Unexpected size of option %lu: %lu", dwOption, bufferLen); + return wxString(); + } + + wxWCharBuffer resBuf(bufferLen / sizeof(wchar_t) - 1); if ( wxWinHTTP::WinHttpQueryOption(hInternet, dwOption, resBuf.data(), &bufferLen) ) result.assign(resBuf); } diff --git a/tests/net/webrequest.cpp b/tests/net/webrequest.cpp index 14593422ee..6b6e4eef69 100644 --- a/tests/net/webrequest.cpp +++ b/tests/net/webrequest.cpp @@ -229,6 +229,17 @@ TEST_CASE_METHOD(RequestFixture, CHECK( request.GetResponse().AsString() == "The quick brown fox jumps over the lazy dog" ); } +TEST_CASE_METHOD(RequestFixture, + "WebRequest::Get::Header", "[net][webrequest][get]") +{ + if ( !InitBaseURL() ) + return; + + Create("/response-headers?freeform=wxWidgets%20works!"); + Run(); + CHECK( request.GetResponse().GetHeader("freeform") == "wxWidgets works!" ); +} + TEST_CASE_METHOD(RequestFixture, "WebRequest::Get::Param", "[net][webrequest][get]") { @@ -510,13 +521,21 @@ TEST_CASE_METHOD(RequestFixture, request.Start(); RunLoopWithTimeout(); - WARN("Request state " << request.GetState()); + CHECK( request.GetState() == wxWebRequest::State_Completed ); wxWebResponse response = request.GetResponse(); REQUIRE( response.IsOk() ); - WARN("Status: " << response.GetStatus() + WARN("URL: " << response.GetURL() << "\n" << + "Status: " << response.GetStatus() << " (" << response.GetStatusText() << ")\n" << "Body length: " << response.GetContentLength() << "\n" << "Body: " << response.AsString() << "\n"); + + // Also show the value of the given header if requested. + wxString header; + if ( wxGetEnv("WX_TEST_WEBREQUEST_HEADER", &header) ) + { + WARN("Header " << header << ": " << response.GetHeader(header)); + } } WX_DECLARE_STRING_HASH_MAP(wxString, wxWebRequestHeaderMap);