diff --git a/.github/scripts/run-with-timeout.ps1 b/.github/scripts/run-with-timeout.ps1 new file mode 100644 index 000000000..cfbf13e33 --- /dev/null +++ b/.github/scripts/run-with-timeout.ps1 @@ -0,0 +1,253 @@ +[CmdletBinding()] +param( + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [string]$FilePath, + + [ValidateRange(1, 86400)] + [int]$TimeoutSeconds = 600, + + [ValidateRange(1, 16)] + [int]$ProcessCount = 1, + + [ValidateNotNullOrEmpty()] + [string]$DiagnosticsDirectory = "test-diagnostics", + + [ValidateNotNullOrEmpty()] + [string]$Label = [System.IO.Path]::GetFileNameWithoutExtension($FilePath), + + [string]$ProcessArguments = "" +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" + +if (-not ("RunWithTimeout.NativeMethods" -as [type])) { + Add-Type -TypeDefinition @" +namespace RunWithTimeout +{ + using System; + using System.Runtime.InteropServices; + + public static class NativeMethods + { + [DllImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + public static extern bool IsWow64Process( + IntPtr processHandle, + [MarshalAs(UnmanagedType.Bool)] out bool wow64Process); + } +} +"@ +} + +function Stop-RunningProcess { + param( + [Parameter(Mandatory = $true)] + [System.Diagnostics.Process]$Process + ) + + if (-not $Process.HasExited) { + try { + Stop-Process -Id $Process.Id + } + catch { + $Process.Refresh() + if (-not $Process.HasExited) { + throw + } + } + $Process.WaitForExit() + } +} + +function Get-DumpSystemDirectory { + param( + [Parameter(Mandatory = $true)] + [System.Diagnostics.Process]$Process + ) + + if (-not [Environment]::Is64BitOperatingSystem) { + return (Join-Path $env:WINDIR "System32") + } + + $isWow64 = $false + if (-not [RunWithTimeout.NativeMethods]::IsWow64Process($Process.Handle, [ref]$isWow64)) { + $errorCode = [Runtime.InteropServices.Marshal]::GetLastWin32Error() + throw "Unable to determine the architecture of process $($Process.Id) (Win32 error $errorCode)." + } + + if ($isWow64) { + return (Join-Path $env:WINDIR "SysWOW64") + } + + if (-not [Environment]::Is64BitProcess) { + return (Join-Path $env:WINDIR "Sysnative") + } + + return (Join-Path $env:WINDIR "System32") +} + +function Save-ProcessDump { + param( + [Parameter(Mandatory = $true)] + [System.Diagnostics.Process]$Process, + + [Parameter(Mandatory = $true)] + [string]$DumpPath + ) + + $dumpProcess = $null + $procdump = Get-Command procdump.exe -ErrorAction SilentlyContinue + if ($null -ne $procdump) { + # A minidump contains the thread stacks and module list needed for a + # deadlock diagnosis without copying arbitrary process memory into CI + # artifacts. + $arguments = "-accepteula -mm $($Process.Id) `"$DumpPath`"" + $dumpProcess = Start-Process -FilePath $procdump.Source -ArgumentList $arguments -PassThru -NoNewWindow + } + else { + # The dump writer must match the target process architecture. A + # 64-bit helper cannot reliably capture Win32 thread context, and a + # 32-bit helper cannot inspect a 64-bit target. + $systemDirectory = Get-DumpSystemDirectory -Process $Process + $powershell = Join-Path $systemDirectory "WindowsPowerShell\v1.0\powershell.exe" + $dumpScript = Join-Path $PSScriptRoot "write-minidump.ps1" + $arguments = "-NoLogo -NoProfile -ExecutionPolicy Bypass -File `"$dumpScript`" -ProcessId $($Process.Id) -DumpPath `"$DumpPath`"" + $dumpProcess = Start-Process -FilePath $powershell -ArgumentList $arguments -PassThru -NoNewWindow + } + + if (-not $dumpProcess.WaitForExit(30000)) { + Stop-RunningProcess -Process $dumpProcess + throw "Timed out while capturing dump for process $($Process.Id)." + } + $dumpProcess.WaitForExit() + $dumpProcess.Refresh() + + if ($dumpProcess.ExitCode -ne 0) { + throw "Dump capture for process $($Process.Id) exited with code $($dumpProcess.ExitCode)." + } + + if (-not (Test-Path -LiteralPath $DumpPath -PathType Leaf)) { + throw "Dump capture for process $($Process.Id) did not create $DumpPath." + } + + $dumpFile = Get-Item -LiteralPath $DumpPath -ErrorAction SilentlyContinue + if ($null -eq $dumpFile -or $dumpFile.Length -eq 0) { + throw "Dump capture for process $($Process.Id) created an empty dump." + } +} + +$resolvedFilePath = (Resolve-Path -LiteralPath $FilePath).Path +$resolvedDiagnosticsDirectory = [System.IO.Path]::GetFullPath($DiagnosticsDirectory) +New-Item -ItemType Directory -Path $resolvedDiagnosticsDirectory -Force | Out-Null + +$safeLabel = $Label -replace '[^A-Za-z0-9_.-]', '_' +$statusPath = Join-Path $resolvedDiagnosticsDirectory "$safeLabel-status.txt" +$startedAt = Get-Date +@( + "Command: $resolvedFilePath" + "Arguments: $ProcessArguments" + "Process count: $ProcessCount" + "Timeout seconds: $TimeoutSeconds" + "Started: $($startedAt.ToString('o'))" +) | Set-Content -LiteralPath $statusPath + +$processes = @() +try { + for ($index = 0; $index -lt $ProcessCount; $index++) { + $startInfo = New-Object System.Diagnostics.ProcessStartInfo + $startInfo.FileName = $resolvedFilePath + $startInfo.Arguments = $ProcessArguments + $startInfo.UseShellExecute = $false + + $process = New-Object System.Diagnostics.Process + $process.StartInfo = $startInfo + if (-not $process.Start()) { + throw "Failed to start $resolvedFilePath." + } + $processes += $process + } +} +catch { + foreach ($process in $processes) { + Stop-RunningProcess -Process $process + } + throw +} + +$deadline = $startedAt.AddSeconds($TimeoutSeconds) +while ($true) { + $failedProcess = $null + $failedExitCode = 0 + $running = @() + foreach ($process in $processes) { + if ($process.HasExited) { + # WaitForExit() populates ExitCode reliably for processes that can + # finish before the first polling iteration. + $process.WaitForExit() + $process.Refresh() + if ($process.ExitCode -ne 0 -and $null -eq $failedProcess) { + $failedProcess = $process + $failedExitCode = $process.ExitCode + } + } + else { + $running += $process + } + } + + if ($null -ne $failedProcess) { + foreach ($process in $processes) { + Stop-RunningProcess -Process $process + } + Add-Content -LiteralPath $statusPath -Value @( + "Completed: $((Get-Date).ToString('o'))" + "Result: failed" + "Exit code: $failedExitCode" + ) + exit $failedExitCode + } + + if ($running.Count -eq 0) { + Add-Content -LiteralPath $statusPath -Value @( + "Completed: $((Get-Date).ToString('o'))" + "Result: passed" + "Exit code: 0" + ) + exit 0 + } + + if ((Get-Date) -ge $deadline) { + Write-Host "::error::$Label exceeded its $TimeoutSeconds-second timeout." + Add-Content -LiteralPath $statusPath -Value @( + "Completed: $((Get-Date).ToString('o'))" + "Result: timed out" + "Exit code: 124" + ) + + foreach ($process in $running) { + try { + if (-not $process.HasExited) { + $detailsPath = Join-Path $resolvedDiagnosticsDirectory "$safeLabel-$($process.Id).txt" + Get-Process -Id $process.Id | + Format-List Id, ProcessName, StartTime, TotalProcessorTime, Threads, HandleCount | + Out-File -LiteralPath $detailsPath + + $dumpPath = Join-Path $resolvedDiagnosticsDirectory "$safeLabel-$($process.Id).dmp" + Save-ProcessDump -Process $process -DumpPath $dumpPath + Write-Host "Captured $dumpPath" + } + } + catch { + Write-Warning $_ + } + finally { + Stop-RunningProcess -Process $process + } + } + exit 124 + } + + Start-Sleep -Milliseconds 200 +} diff --git a/.github/scripts/write-minidump.ps1 b/.github/scripts/write-minidump.ps1 new file mode 100644 index 000000000..9ff613062 --- /dev/null +++ b/.github/scripts/write-minidump.ps1 @@ -0,0 +1,63 @@ +[CmdletBinding()] +param( + [Parameter(Mandatory = $true)] + [ValidateRange(1, [int]::MaxValue)] + [int]$ProcessId, + + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [string]$DumpPath +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" + +Add-Type -TypeDefinition @" +namespace WriteMiniDump +{ + using System; + using System.Runtime.InteropServices; + using Microsoft.Win32.SafeHandles; + + public static class NativeMethods + { + [DllImport("dbghelp.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + public static extern bool MiniDumpWriteDump( + IntPtr processHandle, + uint processId, + SafeFileHandle fileHandle, + uint dumpType, + IntPtr exceptionParameters, + IntPtr userStreamParameters, + IntPtr callbackParameters); + } +} +"@ + +$process = Get-Process -Id $ProcessId +$resolvedDumpPath = [System.IO.Path]::GetFullPath($DumpPath) +$dumpStream = [System.IO.File]::Open( + $resolvedDumpPath, + [System.IO.FileMode]::Create, + [System.IO.FileAccess]::Write, + [System.IO.FileShare]::None) + +try { + $created = [WriteMiniDump.NativeMethods]::MiniDumpWriteDump( + $process.Handle, + [uint32]$process.Id, + $dumpStream.SafeFileHandle, + 0, + [IntPtr]::Zero, + [IntPtr]::Zero, + [IntPtr]::Zero) + if (-not $created) { + $errorCode = [Runtime.InteropServices.Marshal]::GetLastWin32Error() + throw "MiniDumpWriteDump failed for process $ProcessId (Win32 error $errorCode)." + } +} +finally { + $dumpStream.Dispose() + $process.Dispose() +} diff --git a/.github/workflows/test-vcpkg.yml b/.github/workflows/test-vcpkg.yml index 59961ce53..98ef86429 100644 --- a/.github/workflows/test-vcpkg.yml +++ b/.github/workflows/test-vcpkg.yml @@ -24,7 +24,11 @@ concurrency: jobs: windows: runs-on: windows-latest - name: Windows (x64-windows-static) + name: Windows (x64-windows-static, ${{ matrix.transport }}) + strategy: + fail-fast: false + matrix: + transport: [WinHTTP, WinInet] steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 @@ -35,7 +39,12 @@ jobs: shell: pwsh - name: Run vcpkg port test - run: .\tests\vcpkg\test-vcpkg-windows.ps1 -VcpkgRoot "${{ runner.temp }}\vcpkg" + run: | + $arguments = @{ VcpkgRoot = "${{ runner.temp }}\vcpkg" } + if ("${{ matrix.transport }}" -eq "WinInet") { + $arguments.WinInet = $true + } + .\tests\vcpkg\test-vcpkg-windows.ps1 @arguments shell: pwsh linux: diff --git a/.github/workflows/test-win-latest.yml b/.github/workflows/test-win-latest.yml index 2a77d5e2a..66261d1e6 100644 --- a/.github/workflows/test-win-latest.yml +++ b/.github/workflows/test-win-latest.yml @@ -32,28 +32,47 @@ concurrency: jobs: test: - name: Test on Windows ${{ matrix.arch }}-${{ matrix.build }} + name: Test on Windows ${{ matrix.arch }}-${{ matrix.build }}${{ matrix.transport == 'WinInet' && ' (WinInet)' || '' }} runs-on: ${{ matrix.os }} + timeout-minutes: 30 strategy: + fail-fast: false matrix: arch: [Win32, x64] build: [Release, Debug] + transport: [WinHTTP, WinInet] os: [windows-2022] + exclude: + - build: Debug + transport: WinInet steps: - name: Checkout uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - continue-on-error: true - name: setup-msbuild uses: microsoft/setup-msbuild@6fb02220983dee41ce7ae257b6f4d8f9bf5ed4ce # v2.0.0 with: vs-version: '[17,)' - - name: Test ${{ matrix.arch }} ${{ matrix.build }} + - name: Test ${{ matrix.transport }} ${{ matrix.arch }} ${{ matrix.build }} shell: cmd - run: build-tests.cmd ${{ matrix.arch }} ${{ matrix.build }} + run: build-tests.cmd ${{ matrix.arch }} ${{ matrix.build }} "" ${{ matrix.transport }} + + - name: Upload test failure diagnostics + if: failure() || cancelled() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: windows-test-failure-${{ matrix.transport }}-${{ matrix.arch }}-${{ matrix.build }} + path: | + test-diagnostics + Solutions/out/${{ matrix.build }}/${{ matrix.arch }}/UnitTests/*.pdb + Solutions/out/${{ matrix.build }}/${{ matrix.arch }}/UnitTests/*.map + Solutions/out/${{ matrix.build }}/${{ matrix.arch }}/FuncTests/*.pdb + Solutions/out/${{ matrix.build }}/${{ matrix.arch }}/FuncTests/*.map + if-no-files-found: ignore + retention-days: 7 public-headers: name: Public header gate (MSVC) diff --git a/Solutions/win32-dll/win32-dll.vcxproj b/Solutions/win32-dll/win32-dll.vcxproj index b01b9e690..a7cae0a5e 100644 --- a/Solutions/win32-dll/win32-dll.vcxproj +++ b/Solutions/win32-dll/win32-dll.vcxproj @@ -211,7 +211,7 @@ Windows true - uuid.lib;wininet.lib;crypt32.lib;version.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;version.lib;%(AdditionalDependencies) runtimeobject.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll @@ -233,7 +233,7 @@ true - wininet.lib;user32.lib;shell32.lib;Advapi32.lib;Ole32.lib + wininet.lib;winhttp.lib;user32.lib;shell32.lib;Advapi32.lib;Ole32.lib %(AdditionalLibraryDirectories) @@ -297,7 +297,7 @@ true true true - uuid.lib;wininet.lib;crypt32.lib;version.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;version.lib;%(AdditionalDependencies) runtimeobject.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll @@ -322,7 +322,7 @@ true - wininet.lib;user32.lib;shell32.lib;Advapi32.lib;Ole32.lib + wininet.lib;winhttp.lib;user32.lib;shell32.lib;Advapi32.lib;Ole32.lib %(AdditionalLibraryDirectories) diff --git a/Solutions/win32-lib/win32-lib.vcxproj b/Solutions/win32-lib/win32-lib.vcxproj index 1b9fb6a7c..dd1a24cb3 100644 --- a/Solutions/win32-lib/win32-lib.vcxproj +++ b/Solutions/win32-lib/win32-lib.vcxproj @@ -279,7 +279,7 @@ Windows true - uuid.lib;wininet.lib;crypt32.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll false @@ -347,7 +347,7 @@ Windows true - uuid.lib;wininet.lib;crypt32.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll false @@ -425,7 +425,7 @@ true true true - uuid.lib;wininet.lib;crypt32.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll @@ -501,7 +501,7 @@ true true true - uuid.lib;wininet.lib;crypt32.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll diff --git a/Solutions/win32-mini-dll/win32-mini-dll.vcxproj b/Solutions/win32-mini-dll/win32-mini-dll.vcxproj index fe923aee2..2b8c67fef 100644 --- a/Solutions/win32-mini-dll/win32-mini-dll.vcxproj +++ b/Solutions/win32-mini-dll/win32-mini-dll.vcxproj @@ -240,7 +240,7 @@ Windows true - uuid.lib;wininet.lib;crypt32.lib;version.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;version.lib;%(AdditionalDependencies) runtimeobject.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll @@ -268,7 +268,7 @@ true - wininet.lib;user32.lib;shell32.lib;Advapi32.lib;Ole32.lib + wininet.lib;winhttp.lib;user32.lib;shell32.lib;Advapi32.lib;Ole32.lib %(AdditionalLibraryDirectories) @@ -357,7 +357,7 @@ true true true - uuid.lib;wininet.lib;crypt32.lib;version.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;version.lib;%(AdditionalDependencies) runtimeobject.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll @@ -385,7 +385,7 @@ true - wininet.lib;user32.lib;shell32.lib;Advapi32.lib;Ole32.lib + wininet.lib;winhttp.lib;user32.lib;shell32.lib;Advapi32.lib;Ole32.lib %(AdditionalLibraryDirectories) diff --git a/Solutions/win32-mini-lib/win32-mini-lib.vcxproj b/Solutions/win32-mini-lib/win32-mini-lib.vcxproj index 700623d89..18ab5abb0 100644 --- a/Solutions/win32-mini-lib/win32-mini-lib.vcxproj +++ b/Solutions/win32-mini-lib/win32-mini-lib.vcxproj @@ -321,7 +321,7 @@ Windows true - uuid.lib;wininet.lib;crypt32.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll false @@ -427,7 +427,7 @@ Windows true - uuid.lib;wininet.lib;crypt32.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll false @@ -534,7 +534,7 @@ true true true - uuid.lib;wininet.lib;crypt32.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll @@ -642,7 +642,7 @@ true true true - uuid.lib;wininet.lib;crypt32.lib;%(AdditionalDependencies) + uuid.lib;wininet.lib;winhttp.lib;crypt32.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) api-ms-win-core-winrt-l1-1-0.dll;api-ms-win-core-winrt-string-l1-1-0.dll diff --git a/build-tests.cmd b/build-tests.cmd index 7f3d0a0ba..12b74e59a 100644 --- a/build-tests.cmd +++ b/build-tests.cmd @@ -2,6 +2,18 @@ cd %~dp0 @setlocal ENABLEEXTENSIONS +set TRANSPORT=%~4 +if not defined TRANSPORT set TRANSPORT=WinHTTP +if /I "%TRANSPORT%"=="WinInet" ( + set TRANSPORT_PROPERTY=/p:MATSDK_USE_WININET=true +) else if /I "%TRANSPORT%"=="WinHTTP" ( + set TRANSPORT_PROPERTY=/p:MATSDK_USE_WININET=false +) else ( + echo ERROR: Unknown HTTP transport "%TRANSPORT%". Expected WinHTTP or WinInet. + exit /b 2 +) +echo HTTP transport: %TRANSPORT% + set CUSTOM_PROPS= if not "%~3"=="" ( if not exist "%~f3" ( @@ -52,11 +64,11 @@ set CONFIGURATION=%2 set MAXCPUCOUNT=%NUMBER_OF_PROCESSORS% set SOLUTION=Solutions\MSTelemetrySDK.sln -msbuild %SOLUTION% /target:sqlite:Rebuild,zlib:Rebuild,Tests\gmock:Rebuild,Tests\gtest:Rebuild,Tests\UnitTests:Rebuild,Tests\FuncTests:Rebuild /p:BuildProjectReferences=true /maxcpucount:%MAXCPUCOUNT% /detailedsummary /p:Configuration=%CONFIGURATION% /p:Platform=%PLAT% %CUSTOM_PROPS% +msbuild %SOLUTION% /target:sqlite:Rebuild,zlib:Rebuild,Tests\gmock:Rebuild,Tests\gtest:Rebuild,Tests\UnitTests:Rebuild,Tests\FuncTests:Rebuild /p:BuildProjectReferences=true /maxcpucount:%MAXCPUCOUNT% /detailedsummary /p:Configuration=%CONFIGURATION% /p:Platform=%PLAT% %TRANSPORT_PROPERTY% %CUSTOM_PROPS% if not "%ERRORLEVEL%"=="0" exit /b %ERRORLEVEL% -Solutions\out\%CONFIGURATION%\%PLAT%\UnitTests\UnitTests.exe +powershell.exe -NoLogo -NoProfile -ExecutionPolicy Bypass -File .github\scripts\run-with-timeout.ps1 -FilePath Solutions\out\%CONFIGURATION%\%PLAT%\UnitTests\UnitTests.exe -TimeoutSeconds 600 -Label UnitTests-%CONFIGURATION%-%PLAT%-%TRANSPORT% if not "%ERRORLEVEL%"=="0" exit /b %ERRORLEVEL% -Solutions\out\%CONFIGURATION%\%PLAT%\FuncTests\FuncTests.exe +powershell.exe -NoLogo -NoProfile -ExecutionPolicy Bypass -File .github\scripts\run-with-timeout.ps1 -FilePath Solutions\out\%CONFIGURATION%\%PLAT%\FuncTests\FuncTests.exe -TimeoutSeconds 600 -Label FuncTests-%CONFIGURATION%-%PLAT%-%TRANSPORT% if not "%ERRORLEVEL%"=="0" exit /b %ERRORLEVEL% -powershell -NoProfile -ExecutionPolicy Bypass -Command "$path = Join-Path (Get-Location) 'Solutions\out\%CONFIGURATION%\%PLAT%\FuncTests\FuncTests.exe'; $args = '--gtest_filter=MultipleLogManagersTests.MultiProcessesLogManager'; $p1 = Start-Process -FilePath $path -ArgumentList $args -PassThru; $p2 = Start-Process -FilePath $path -ArgumentList $args -PassThru; $p1.WaitForExit(); $p2.WaitForExit(); if ($p1.ExitCode -ne 0 -or $p2.ExitCode -ne 0) { exit 1 }" +powershell.exe -NoLogo -NoProfile -ExecutionPolicy Bypass -File .github\scripts\run-with-timeout.ps1 -FilePath Solutions\out\%CONFIGURATION%\%PLAT%\FuncTests\FuncTests.exe -ProcessArguments "--gtest_filter=MultipleLogManagersTests.MultiProcessesLogManager" -ProcessCount 2 -TimeoutSeconds 600 -Label FuncTests-concurrent-%CONFIGURATION%-%PLAT%-%TRANSPORT% if not "%ERRORLEVEL%"=="0" exit /b %ERRORLEVEL% diff --git a/cmake/MatsdkOptions.cmake b/cmake/MatsdkOptions.cmake index a56e468c6..3cb213ef8 100644 --- a/cmake/MatsdkOptions.cmake +++ b/cmake/MatsdkOptions.cmake @@ -42,6 +42,8 @@ option(MATSDK_BUILD_AZMON "Build Azure Monitor / Application Insights support" ON) option(MATSDK_BUILD_APPLE_HTTP "Build the Apple-native HTTP client" "${APPLE}") +option(MATSDK_USE_WININET + "Use WinInet instead of WinHTTP as the Win32 desktop HTTP client" OFF) set(_matsdk_android_http_client_predefined OFF) if(DEFINED MATSDK_ANDROID_HTTP_CLIENT) diff --git a/docs/building-with-vcpkg.md b/docs/building-with-vcpkg.md index 8cdb27c69..a4aa85a3c 100644 --- a/docs/building-with-vcpkg.md +++ b/docs/building-with-vcpkg.md @@ -220,18 +220,27 @@ On Linux, libcurl is provided by the default `curl-openssl` feature; `curl-mbedtls` swaps in the mbedTLS backend — see [Choose the Linux HTTP client / TLS backend](#choose-the-linux-http-client--tls-backend-largest-lever-on-linux). -Windows and macOS/iOS use platform-native HTTP clients (WinInet and +Windows and macOS/iOS use platform-native HTTP clients (WinHTTP and NSURLSession respectively). Android defaults to the platform Java/JNI HTTP bridge; native curl is available only through explicit `android-curl-*` features. > **Note (Windows):** The port targets the MSVC/`WIN32` PAL on Windows, which -> uses WinInet, so the default `curl` dependency is declared for Linux only +> uses WinHTTP, so the default `curl` dependency is declared for Linux only > (Android has separate explicit `android-curl-*` features). A MinGW / > non-MSVC Windows triplet — or forcing `-DPAL_IMPLEMENTATION=CPP11` on Windows — > selects the curl HTTP client, which the port does not provision on Windows > (broadening `curl` to `windows` would pull an unused curl into every MSVC > build, since vcpkg platform expressions can't key off the PAL). Use a standard > MSVC triplet such as `x64-windows-static` for Windows vcpkg builds. +> +> Consumers that require WinInet's IE-integrated proxy or cookie behavior can +> opt in with the `wininet` feature, for example +> `"features": ["wininet", "system-sqlite"]`. +> WinHTTP uses automatic or machine-level proxy configuration rather than the +> logged-on user's Internet Explorer settings, does not answer authentication +> challenges with ambient user credentials, and reports WinHTTP error codes. +> Consumers that depend on the prior WinInet behavior should select the feature +> explicitly before updating. ## Optional: SIMD-Optimized zlib with zlib-ng @@ -303,7 +312,7 @@ export table pins its symbols and defeats `/OPT:REF`. ### Choose the Linux HTTP client / TLS backend (largest lever on Linux) On Linux the built-in HTTP client is libcurl, and curl's TLS backend dominates -the SDK's footprint. (Windows uses WinInet, Apple uses NSURLSession, and Android +the SDK's footprint. (Windows uses WinHTTP by default, Apple uses NSURLSession, and Android uses the Java/JNI bridge by default, so this section does not apply there.) The port exposes the Linux TLS backend as two mutually-exclusive features; pick the one that matches what your application already has: diff --git a/examples/c/SampleC/SampleC.vcxproj b/examples/c/SampleC/SampleC.vcxproj index d307939cf..8dc8948f1 100644 --- a/examples/c/SampleC/SampleC.vcxproj +++ b/examples/c/SampleC/SampleC.vcxproj @@ -1,4 +1,4 @@ - + @@ -43,7 +43,7 @@ false - $(MSBuildProjectDirectory)\lib\$(Configuration)\$(Platform);$(VCInstallDir)lib;$(VCInstallDir)atlmfc\lib;$(WindowsSdkDir)lib;$(FrameworkSDKDir)\lib + $(LibraryPath) $(VCInstallDir)include;$(VCInstallDir)atlmfc\include;$(WindowsSdkDir)include;$(FrameworkSDKDir)\include;$(MSBuildProjectDirectory)\include @@ -53,7 +53,7 @@ Level3 Disabled HAVE_DYNAMIC_C_LIB;WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions) - $(SolutionDir)\..\lib\include\public + $(ProjectDir)\..\..\..\lib\include\public Console @@ -80,7 +80,7 @@ - $(SolutionDir)\..\lib\include\public + $(ProjectDir)\..\..\..\lib\include\public Console @@ -102,10 +102,10 @@ - - - - + + + + diff --git a/examples/c/SampleC/SampleC.vcxproj.filters b/examples/c/SampleC/SampleC.vcxproj.filters index ec99270d2..bc6cb1d50 100644 --- a/examples/c/SampleC/SampleC.vcxproj.filters +++ b/examples/c/SampleC/SampleC.vcxproj.filters @@ -1,4 +1,4 @@ - + @@ -20,16 +20,16 @@ - + Header Files - + Header Files - + Header Files - + Header Files diff --git a/examples/cpp/SampleCpp/SampleCpp.vcxproj b/examples/cpp/SampleCpp/SampleCpp.vcxproj index a8548808f..6f340fec9 100644 --- a/examples/cpp/SampleCpp/SampleCpp.vcxproj +++ b/examples/cpp/SampleCpp/SampleCpp.vcxproj @@ -244,7 +244,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public @@ -252,37 +252,37 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public @@ -290,19 +290,19 @@ true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public @@ -310,37 +310,37 @@ true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public @@ -348,13 +348,13 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public @@ -461,7 +461,7 @@ true true true - wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) @@ -546,7 +546,7 @@ true true true - wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) @@ -666,7 +666,7 @@ Console true - wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) @@ -818,7 +818,7 @@ Console true - wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) @@ -894,7 +894,7 @@ Console true - wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) @@ -1013,7 +1013,7 @@ true true true - wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) diff --git a/examples/cpp/SampleCppLogManagers/SampleCppLogManagers.vcxproj b/examples/cpp/SampleCppLogManagers/SampleCppLogManagers.vcxproj index 7f6b47434..def42ce22 100644 --- a/examples/cpp/SampleCppLogManagers/SampleCppLogManagers.vcxproj +++ b/examples/cpp/SampleCppLogManagers/SampleCppLogManagers.vcxproj @@ -67,7 +67,7 @@ true - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public;$(SolutionDir)\..\lib\pal\ + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public;$(SolutionDir)\..\lib\pal\ $(ProjectDir) $(Configuration)\ $(LibraryPath) @@ -75,16 +75,16 @@ true $(ProjectDir) - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public;$(SolutionDir)\..\lib\pal\ + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public;$(SolutionDir)\..\lib\pal\ $(LibraryPath) false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)\..\..\..\lib\include\public diff --git a/examples/cpp/SampleCppMini/SampleCppMini.vcxproj b/examples/cpp/SampleCppMini/SampleCppMini.vcxproj index 424394f7e..cdcc13ea4 100644 --- a/examples/cpp/SampleCppMini/SampleCppMini.vcxproj +++ b/examples/cpp/SampleCppMini/SampleCppMini.vcxproj @@ -262,7 +262,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public @@ -272,7 +272,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -280,7 +280,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -288,7 +288,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -296,7 +296,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -304,7 +304,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -312,7 +312,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public @@ -322,7 +322,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -330,7 +330,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -338,7 +338,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public @@ -348,7 +348,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -356,7 +356,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -364,7 +364,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -372,7 +372,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -380,7 +380,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -388,7 +388,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public @@ -398,7 +398,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -406,7 +406,7 @@ false - $(VC_IncludePath);$(WindowsSDK_IncludePath);$(SolutionDir)\..\lib\include\public + $(VC_IncludePath);$(WindowsSDK_IncludePath);$(ProjectDir)..\..\..\lib\include\public true @@ -430,7 +430,7 @@ false false false - false + Sync false Disabled Size @@ -452,13 +452,8 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - wininet.lib;Crypt32.lib; + wininet.lib;winhttp.lib;Crypt32.lib; - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - Copy DLL to target dir - @@ -486,7 +481,7 @@ false false false - false + Sync false Disabled Size @@ -508,15 +503,8 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - wininet.lib;Crypt32.lib; + wininet.lib;winhttp.lib;Crypt32.lib; - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -549,7 +537,7 @@ false false false - false + Sync false Disabled Size @@ -563,7 +551,7 @@ true true true - wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) false true true @@ -571,14 +559,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -611,7 +592,7 @@ false false false - false + Sync false Disabled Size @@ -626,7 +607,7 @@ true true true - wininet.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) true false true @@ -635,14 +616,7 @@ /merge:.rdata=.text false false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -675,7 +649,7 @@ false false false - false + Sync false Disabled Size @@ -689,7 +663,7 @@ true true true - wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) false true true @@ -697,14 +671,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -737,7 +704,7 @@ false false false - false + Sync false Disabled Size @@ -752,7 +719,7 @@ true true true - wininet.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) true false true @@ -761,14 +728,7 @@ /merge:.rdata=.text false false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -802,7 +762,7 @@ Default false false - false + Sync Disabled Size false @@ -823,15 +783,8 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - wininet.lib;Crypt32.lib; + wininet.lib;winhttp.lib;Crypt32.lib; - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -866,7 +819,7 @@ Default false false - false + Sync Disabled Size false @@ -877,7 +830,7 @@ Console true - wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) false true true @@ -887,14 +840,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -930,7 +876,7 @@ Default false false - false + Sync Disabled Size false @@ -941,7 +887,7 @@ Console true - wininet.lib;Crypt32.lib;libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;libucrtd.lib;Version.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) false true true @@ -951,14 +897,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -990,7 +929,7 @@ Default false false - false + Sync false Disabled Size @@ -1013,15 +952,8 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - wininet.lib;Crypt32.lib; + wininet.lib;winhttp.lib;Crypt32.lib; - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1054,7 +986,7 @@ Default false false - false + Sync false Disabled Size @@ -1077,15 +1009,8 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - wininet.lib;Crypt32.lib; + wininet.lib;winhttp.lib;Crypt32.lib; - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1119,7 +1044,7 @@ Default false false - false + Sync false Disabled Size @@ -1132,7 +1057,7 @@ Console true - wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) false true true @@ -1142,14 +1067,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1183,7 +1101,7 @@ Default false false - false + Sync false Disabled Size @@ -1196,7 +1114,7 @@ Console true - wininet.lib;Crypt32.lib;libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;libucrtd.lib;Version.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) false true true @@ -1206,14 +1124,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1246,7 +1157,7 @@ Default false false - false + Sync false Disabled Size @@ -1259,7 +1170,7 @@ Console true - wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) false true true @@ -1269,14 +1180,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1310,7 +1214,7 @@ Default false false - false + Sync false Disabled Size @@ -1323,7 +1227,7 @@ Console true - wininet.lib;Crypt32.lib;libucrtd.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;libucrtd.lib;Version.lib;kernel32.lib;user32.lib;%(AdditionalDependencies) false true true @@ -1333,14 +1237,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1370,7 +1267,7 @@ false false false - false + Sync false Disabled Size @@ -1393,15 +1290,8 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - wininet.lib;Crypt32.lib; + wininet.lib;winhttp.lib;Crypt32.lib; - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1433,7 +1323,7 @@ false false false - false + Sync false Disabled Size @@ -1448,7 +1338,7 @@ true true true - wininet.lib;Crypt32.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) false true true @@ -1456,14 +1346,7 @@ false /merge:.rdata=.text false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1496,7 +1379,7 @@ false false false - false + Sync false Disabled Size @@ -1512,7 +1395,7 @@ true true true - wininet.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;wininet.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) + wininet.lib;winhttp.lib;Crypt32.lib;libcmt.lib;libvcruntime.lib;libucrt.lib;Version.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies) true false true @@ -1521,14 +1404,7 @@ /merge:.rdata=.text false false - API-MS-WIN-CORE-WINRT-STRING-L1-1-0;API-MS-WIN-CORE-WINRT-L1-1-0 - - $(ProjectDir)\deploy-dll.cmd $(PlatformTarget) $(Configuration) $(OutDir) - - - Copy DLL to target dir - @@ -1548,13 +1424,12 @@ - + - - + {1dc6b38a-b390-34ce-907f-4958807a3d43} diff --git a/examples/cpp/SampleCppMini/SampleCppMini.vcxproj.filters b/examples/cpp/SampleCppMini/SampleCppMini.vcxproj.filters index 2df19ab39..ebc2bf270 100644 --- a/examples/cpp/SampleCppMini/SampleCppMini.vcxproj.filters +++ b/examples/cpp/SampleCppMini/SampleCppMini.vcxproj.filters @@ -22,7 +22,4 @@ Source Files - - - \ No newline at end of file diff --git a/examples/cpp/SampleCppMini/deploy-dll.cmd b/examples/cpp/SampleCppMini/deploy-dll.cmd deleted file mode 100644 index bd98ed454..000000000 --- a/examples/cpp/SampleCppMini/deploy-dll.cmd +++ /dev/null @@ -1,3 +0,0 @@ -copy %3\..\win32-mini-dll\*.dll %3 -copy %3\..\win32-mini-dll\*.pdb %3 -exit /b 0 diff --git a/examples/cpp/SampleCppUWP/SampleCppUWP.vcxproj b/examples/cpp/SampleCppUWP/SampleCppUWP.vcxproj index a55fbf088..39c8649fe 100644 --- a/examples/cpp/SampleCppUWP/SampleCppUWP.vcxproj +++ b/examples/cpp/SampleCppUWP/SampleCppUWP.vcxproj @@ -134,7 +134,7 @@ /bigobj %(AdditionalOptions) 4453;28204 - $(SolutionDir)\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) + $(ProjectDir)\..\..\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) _ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE=1;%(ClCompile.PreprocessorDefinitions) Cdecl true @@ -146,7 +146,7 @@ /bigobj %(AdditionalOptions) 4453;28204 - $(SolutionDir)\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) + $(ProjectDir)\..\..\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) _ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE=1;%(ClCompile.PreprocessorDefinitions) Cdecl true @@ -159,7 +159,7 @@ /bigobj %(AdditionalOptions) 4453;28204 - $(SolutionDir)\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) + $(ProjectDir)\..\..\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) _ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE=1;%(ClCompile.PreprocessorDefinitions) MinSpace Size @@ -173,7 +173,7 @@ /bigobj %(AdditionalOptions) 4453;28204 - $(SolutionDir)\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) + $(ProjectDir)\..\..\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) _ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE=1;%(ClCompile.PreprocessorDefinitions) MinSpace Size @@ -188,7 +188,7 @@ /bigobj %(AdditionalOptions) 4453;28204 - $(SolutionDir)\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) + $(ProjectDir)\..\..\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) _UNICODE;UNICODE;%(PreprocessorDefinitions) ProgramDatabase Cdecl @@ -204,7 +204,7 @@ /bigobj %(AdditionalOptions) 4453;28204 - $(SolutionDir)\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) + $(ProjectDir)\..\..\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) _UNICODE;UNICODE;%(PreprocessorDefinitions) MinSpace Size @@ -217,7 +217,7 @@ /bigobj %(AdditionalOptions) 4453;28204 - $(SolutionDir)\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) + $(ProjectDir)\..\..\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) _UNICODE;UNICODE;%(PreprocessorDefinitions) ProgramDatabase Cdecl @@ -229,7 +229,7 @@ /bigobj %(AdditionalOptions) 4453;28204 - $(SolutionDir)\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) + $(ProjectDir)\..\..\..\lib\include\public;$(ProjectDir);$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories) _UNICODE;UNICODE;%(PreprocessorDefinitions) MinSpace Size diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index b08fd4537..e1bd6d253 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -299,9 +299,24 @@ target_compile_definitions(matsdk_internal_config INTERFACE _USRDLL WINVER=_WIN32_WINNT_WIN7) target_compile_options(matsdk_internal_config INTERFACE /U_MBCS) +if(MATSDK_USE_WININET) + target_compile_definitions(matsdk_internal_config INTERFACE HAVE_MAT_WININET_HTTP_CLIENT) +else() + target_compile_definitions(matsdk_internal_config INTERFACE HAVE_MAT_WINHTTP_HTTP_CLIENT) +endif() +if(MATSDK_USE_WININET) list(APPEND SRCS http/HttpClient_WinInet.cpp http/HttpClient_WinInet.hpp + ) +else() + list(APPEND SRCS + http/HttpClient_WinHttp.cpp + http/HttpClient_WinHttp.hpp + http/IBoundedHttpClientCancel.hpp + ) +endif() + list(APPEND SRCS pal/desktop/WindowsDesktopDeviceInformationImpl.cpp pal/desktop/WindowsDesktopNetworkInformationImpl.cpp pal/desktop/WindowsDesktopSystemInformationImpl.cpp @@ -666,7 +681,12 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux" OR CMAKE_SYSTEM_NAME STREQUAL "Android") target_link_libraries(mat PUBLIC log) endif() elseif(PAL_IMPLEMENTATION STREQUAL "WIN32") - target_link_libraries(mat PUBLIC wininet crypt32 ws2_32) + if(MATSDK_USE_WININET) + target_link_libraries(mat PRIVATE wininet) + else() + target_link_libraries(mat PRIVATE winhttp) + endif() + target_link_libraries(mat PRIVATE crypt32) elseif(APPLE) target_link_libraries(mat PUBLIC "-framework CoreFoundation" diff --git a/lib/http/HttpClientFactory.cpp b/lib/http/HttpClientFactory.cpp index 5419f161d..b58175e1a 100644 --- a/lib/http/HttpClientFactory.cpp +++ b/lib/http/HttpClientFactory.cpp @@ -18,6 +18,8 @@ #include "http/HttpClient_WinRt.hpp" #elif defined(HAVE_MAT_WININET_HTTP_CLIENT) #include "http/HttpClient_WinInet.hpp" + #elif defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) + #include "http/HttpClient_WinHttp.hpp" #endif #elif defined(MATSDK_PAL_CPP11) #if TARGET_OS_IPHONE || (defined(__APPLE__) && defined(APPLE_HTTP)) @@ -49,6 +51,13 @@ namespace MAT_NS_BEGIN { return std::make_shared(); } +#elif defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) + /* Win32 WinHTTP client (default) */ + std::shared_ptr HttpClientFactory::Create() { + LOG_TRACE("Creating HttpClient_WinHttp"); + return std::make_shared(); + } + #endif #elif defined(HAVE_MAT_CURL_HTTP_CLIENT) std::shared_ptr HttpClientFactory::Create() { diff --git a/lib/http/HttpClientFactory.hpp b/lib/http/HttpClientFactory.hpp index c96bc2ab0..ae1fb9681 100644 --- a/lib/http/HttpClientFactory.hpp +++ b/lib/http/HttpClientFactory.hpp @@ -25,8 +25,22 @@ class HttpClientFactory // TODO: [maxgolov] - remove this once there is a better way to pass HTTP client configuration #if defined(MATSDK_PAL_WIN32) && !defined(_WINRT_DLL) -#define HAVE_MAT_WININET_HTTP_CLIENT -#include "http/HttpClient_WinInet.hpp" + #if defined(HAVE_MAT_WININET_HTTP_CLIENT) && defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) + #error WinInet and WinHTTP cannot both be selected. + #endif + #if defined(HAVE_MAT_WININET_HTTP_CLIENT) + #include "http/HttpClient_WinInet.hpp" + #else + // WinHTTP is the default Win32 desktop transport: unlike WinInet, it does + // not depend on a logged-on interactive user or that user's Internet + // Explorer settings, so it works in services and other non-interactive + // processes without extra configuration. Define HAVE_MAT_WININET_HTTP_CLIENT + // to opt back into WinInet (e.g. for IE-integrated proxy/cookie behavior). + #ifndef HAVE_MAT_WINHTTP_HTTP_CLIENT + #define HAVE_MAT_WINHTTP_HTTP_CLIENT + #endif + #include "http/HttpClient_WinHttp.hpp" + #endif #endif #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT diff --git a/lib/http/HttpClientManager.cpp b/lib/http/HttpClientManager.cpp index 3c7d1f809..730f7b341 100644 --- a/lib/http/HttpClientManager.cpp +++ b/lib/http/HttpClientManager.cpp @@ -11,6 +11,8 @@ #include #include #include +#include +#include #include #include @@ -86,11 +88,33 @@ namespace MAT_NS_BEGIN { m_httpClient(httpClient), m_taskDispatcher(taskDispatcher) { + int64_t configuredSeconds = + logManager.GetLogConfiguration()[CFG_INT_MAX_TEARDOWN_TIME]; + if (configuredSeconds > 0) + { + int64_t const maxSeconds = + std::chrono::milliseconds::max().count() / 1000; + m_cancelDrainTimeout = std::chrono::seconds( + std::min(configuredSeconds, maxSeconds)); + } } HttpClientManager::~HttpClientManager() noexcept { - cancelAllRequestsAsync(); + // HttpCallback and scheduled response tasks retain a reference to this + // manager, so non-reentrant destruction must be a full callback lifetime + // barrier. Reentrant destruction is unsupported because the active + // callback itself must still unwind through this object. +#ifndef NDEBUG + { + std::lock_guard lock(m_httpCallbacksMtx); + for (auto const& active : m_activeHttpCallbacks) + { + assert(active.second != std::this_thread::get_id()); + } + } +#endif + cancelAllRequests(); } void HttpClientManager::handleSendRequest(EventsUploadContextPtr const& ctx) @@ -116,30 +140,54 @@ namespace MAT_NS_BEGIN { /* This method may get executed synchronously on Windows from handleSendRequest in case of connection failure */ void HttpClientManager::onHttpResponse(HttpCallback* callback) { - EventsUploadContextPtr &ctx = callback->m_ctx; { - LOCKGUARD(m_httpCallbacksMtx); + std::lock_guard lock(m_httpCallbacksMtx); auto z = std::find(m_httpCallbacks.cbegin(), m_httpCallbacks.cend(), callback); if (z == m_httpCallbacks.end()) { - assert(false); + LOG_ERROR("Ignoring untracked HTTP callback=%p", callback); + return; } + m_activeHttpCallbacks[callback] = std::this_thread::get_id(); + m_httpCallbacksCV.notify_all(); + } + + EventsUploadContextPtr &ctx = callback->m_ctx; #if !defined(NDEBUG) && defined(HAVE_MAT_LOGGING) - // Response may be null if request got aborted - if (ctx->httpResponse != nullptr) - { - IHttpResponse const& response = (*ctx->httpResponse); - LOG_TRACE("HTTP response %s: result=%u, status=%u, body=%u bytes", - response.GetId().c_str(), response.GetResult(), response.GetStatusCode(), static_cast(response.GetBody().size())); - } + // Response may be null if request got aborted + if (ctx->httpResponse != nullptr) + { + IHttpResponse const& response = (*ctx->httpResponse); + LOG_TRACE("HTTP response %s: result=%u, status=%u, body=%u bytes", + response.GetId().c_str(), response.GetResult(), response.GetStatusCode(), static_cast(response.GetBody().size())); + } #endif + // Never hold m_httpCallbacksMtx while calling the transport or + // dispatching requestDone(): either path may synchronously re-enter this + // manager. Reentrant cancellation recognizes this callback as active + // and does not wait for its own stack to unwind. + try + { requestDone(ctx); - // request done should be handled by now + } + catch (const std::exception& ex) + { + LOG_ERROR("Unhandled exception in HTTP response callback: %s", ex.what()); + } + catch (...) + { + LOG_ERROR("Unhandled non-standard exception in HTTP response callback"); + } + // request done should be handled by now + { + std::lock_guard lock(m_httpCallbacksMtx); LOG_TRACE("HTTP remove callback=%p", callback); m_httpCallbacks.remove(callback); - // Wake cancelAllRequests() waiting for the list to drain. + m_activeHttpCallbacks.erase(callback); + // Wake cancelAllRequests() waiting for the list to drain while the + // condition variable is still guaranteed to be alive. m_httpCallbacksCV.notify_all(); } @@ -198,22 +246,45 @@ namespace MAT_NS_BEGIN { void HttpClientManager::cancelAllRequests(bool bestEffort) { - // Use the transport-specific bounded path when available; older clients - // fall back to cancelling tracked requests individually. + if (bestEffort && + m_cancelDrainTimeout <= std::chrono::milliseconds::zero()) + { + return; + } + // Quiesce the transport before taking m_httpCallbacksMtx. Moving this + // call under the mutex deadlocks when a synchronous transport completion + // re-enters onHttpResponse(). const auto cancelStart = std::chrono::steady_clock::now(); cancelAllRequestsAsync(bestEffort ? m_cancelDrainTimeout : std::chrono::milliseconds::zero()); // Drain callbacks through the condition variable signaled by onHttpResponse. - std::unique_lock lock(m_httpCallbacksMtx); + std::unique_lock lock(m_httpCallbacksMtx); + std::thread::id const callerThread = std::this_thread::get_id(); + auto callbacksDrainedForCaller = [this, callerThread] { + for (auto const& active : m_activeHttpCallbacks) + { + if (active.second == callerThread) + { + // A completion running on a single-thread dispatcher cannot + // wait for peer completions queued behind itself. Returning + // from reentrant cancellation lets this callback unwind and + // the dispatcher drain the remaining work. + return true; + } + } + return m_httpCallbacks.empty(); + }; if (bestEffort) { - // Keep pause bounded, including time spent in the transport cancel. + // Keep pause within the configured soft cap, including time spent + // in transport cancellation. A synchronous native handle close + // already in progress can finish after the deadline. const auto elapsed = std::chrono::duration_cast( std::chrono::steady_clock::now() - cancelStart); const auto remaining = (elapsed < m_cancelDrainTimeout) ? (m_cancelDrainTimeout - elapsed) : std::chrono::milliseconds::zero(); - if (!m_httpCallbacksCV.wait_for(lock, remaining, - [this] { return m_httpCallbacks.empty(); })) + if (!m_httpCallbacksCV.wait_for( + lock, remaining, callbacksDrainedForCaller)) { LOG_WARN("cancelAllRequests: %zu callback(s) still draining after %lld ms (best-effort)", m_httpCallbacks.size(), static_cast(m_cancelDrainTimeout.count())); @@ -221,8 +292,10 @@ namespace MAT_NS_BEGIN { } else { - // Shutdown/cleanup is the lifetime barrier for callback state, so drain fully. - m_httpCallbacksCV.wait(lock, [this] { return m_httpCallbacks.empty(); }); + // Non-reentrant shutdown/cleanup is the lifetime barrier for callback + // state. A callback re-entering cancellation must return so its own + // stack can unwind; destroying the manager from that stack is unsupported. + m_httpCallbacksCV.wait(lock, callbacksDrainedForCaller); } } diff --git a/lib/http/HttpClientManager.hpp b/lib/http/HttpClientManager.hpp index 4f350e37f..9877c65eb 100644 --- a/lib/http/HttpClientManager.hpp +++ b/lib/http/HttpClientManager.hpp @@ -14,6 +14,8 @@ #include #include #include +#include +#include namespace MAT_NS_BEGIN { @@ -65,16 +67,17 @@ class HttpClientManager ILogManager& m_logManager; IHttpClient& m_httpClient; ITaskDispatcher& m_taskDispatcher; - mutable std::recursive_mutex m_httpCallbacksMtx; + mutable std::mutex m_httpCallbacksMtx; std::list m_httpCallbacks; + std::map m_activeHttpCallbacks; // Signaled from onHttpResponse when a callback is removed, so cancelAllRequests // can drain via a condition variable instead of a poll loop. - std::condition_variable_any m_httpCallbacksCV; - // Upper bound on how long cancelAllRequests waits for callbacks to drain. A - // last-resort safety valve so a stalled dispatcher/HTTP stack can never make - // the drain spin or block forever. Adjustable so tests can - // exercise the timeout path without a long wait. - std::chrono::milliseconds m_cancelDrainTimeout{std::chrono::seconds(30)}; + std::condition_variable m_httpCallbacksCV; + // Configured soft cap on the best-effort pause drain. One native handle + // close already in progress may finish after it. Non-reentrant full + // shutdown remains a lifetime barrier and waits for every accepted + // request's terminal callback. + std::chrono::milliseconds m_cancelDrainTimeout{std::chrono::milliseconds::zero()}; }; } MAT_NS_END diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index 1a047f5d6..85a653e81 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/lib/http/HttpClient_Apple.mm @@ -128,12 +128,6 @@ - (void)URLSession:(NSURLSession*)session return std::string("REQ-") + std::to_string(seq.fetch_add(1)); } -static std::string NextRespId() -{ - static std::atomic seq; - return std::string("RESP-") + std::to_string(seq.fetch_add(1)); -} - static dispatch_once_t once; static NSURLSession* session; static MATStreamingSessionDelegate* sessionDelegate; @@ -205,7 +199,7 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error) @autoreleasepool { NSHTTPURLResponse *httpResp = static_cast(response); - auto simpleResponse = new SimpleHttpResponse { NextRespId() }; + auto simpleResponse = new SimpleHttpResponse { GetId() }; simpleResponse->m_statusCode = static_cast(httpResp.statusCode); @@ -220,10 +214,17 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error) NSString* errorDomain = [error domain]; long errorCode = [error code]; - if ([errorDomain isEqualToString:@"NSURLErrorDomain"] && (errorCode == NSURLErrorCancelled)) + if ([errorDomain isEqualToString:@"NSURLErrorDomain"] && + errorCode == NSURLErrorCancelled) { simpleResponse->m_result = HttpResult_Aborted; } + else if ([errorDomain isEqualToString:@"NSURLErrorDomain"] && + (errorCode == NSURLErrorBadURL || + errorCode == NSURLErrorUnsupportedURL)) + { + simpleResponse->m_result = HttpResult_LocalFailure; + } else { LOG_TRACE("HTTP response error code: %li", errorCode); diff --git a/lib/http/HttpClient_Curl.cpp b/lib/http/HttpClient_Curl.cpp index b910cdf28..d78941bbd 100644 --- a/lib/http/HttpClient_Curl.cpp +++ b/lib/http/HttpClient_Curl.cpp @@ -11,6 +11,7 @@ #include "ctmacros.hpp" #include +#include #include "utils/Utils.hpp" #include "HttpClient_Curl.hpp" @@ -18,6 +19,13 @@ namespace MAT_NS_BEGIN { + static bool IsLocalRequestError(CURLcode error) noexcept + { + return error == CURLE_UNSUPPORTED_PROTOCOL || + error == CURLE_URL_MALFORMAT || + error == CURLE_NOT_BUILT_IN; + } + static std::string NextReqId() { static std::atomic seq(0); return std::string("REQ-") + std::to_string(seq.fetch_add(1)); @@ -66,7 +74,6 @@ namespace MAT_NS_BEGIN { void HttpClient_Curl::SendRequestAsync(IHttpRequest* request, IHttpResponseCallback* callback) { // Note: 'request' is never owned by IHttpClient and gets deleted in EventsUploadContext.clear() - AddRequest(request); auto curlRequest = static_cast(request); std::string requestId = curlRequest->GetId(); @@ -81,28 +88,42 @@ namespace MAT_NS_BEGIN { sslCaInfo = m_sslCaInfo; } - auto curlOperation = std::make_shared(curlRequest->m_method, curlRequest->m_url, callback, requestHeaders, curlRequest->m_body, false, HTTP_CONN_TIMEOUT, m_sslVerify, sslCaInfo); + std::shared_ptr curlOperation; + try + { + curlOperation = std::make_shared( + curlRequest->m_method, curlRequest->m_url, callback, requestHeaders, + curlRequest->m_body, false, HTTP_CONN_TIMEOUT, m_sslVerify, sslCaInfo); + } + catch (const std::exception&) + { + auto response = std::unique_ptr( + new SimpleHttpResponse(requestId)); + response->m_result = HttpResult_LocalFailure; + callback->OnHttpResponse(response.get()); + response.release(); + return; + } curlRequest->SetOperation(curlOperation); - - // The lifetime of curlOperation is guarnteed by the call to result.wait() in the d'tor. - curlOperation->SendAsync([this, callback, requestId](CurlHttpOperation& operation) { - this->EraseRequest(requestId); + AddRequest(request); + curlOperation->SendAsync([this, callback, requestId](CurlHttpOperation& operation) { + EraseRequest(requestId); auto response = std::unique_ptr(new SimpleHttpResponse(requestId)); response->m_result = HttpResult_OK; - response->m_statusCode = operation.GetResponseCode(); - if (response->m_statusCode == CURLE_FAILED_INIT) { - // There was an error in CURL stack while trying to create request + response->m_statusCode = operation.GetHttpStatusCode(); + if (operation.WasAborted()) { + // Cancellation wins even when libcurl finishes the transfer + // successfully after the caller has requested an abort. + response->m_result = HttpResult_Aborted; + } else if (operation.GetSetupError() != CURLE_OK || + IsLocalRequestError(operation.GetTransportError())) { + // There was an error configuring the CURL request. response->m_result = HttpResult_LocalFailure; - } else if ((CURLE_OK < response->m_statusCode) && (response->m_statusCode <= CURL_LAST)) { - if (operation.WasAborted()) { - // Operation was manually aborted - response->m_result = HttpResult_Aborted; - } else { - // There was an error in CURL stack while trying to connect - response->m_result = HttpResult_NetworkFailure; - } + } else if (operation.GetTransportError() != CURLE_OK) { + // There was an error in CURL stack while trying to connect. + response->m_result = HttpResult_NetworkFailure; } auto responseHeaders = operation.GetResponseHeaders(); @@ -161,4 +182,3 @@ namespace MAT_NS_BEGIN { } MAT_NS_END #endif - diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index 7d599dec9..c41a8710c 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -17,11 +17,15 @@ #include #include #include +#include #include #include -#include +#include #include +#include +#include +#include #include #include @@ -71,13 +75,6 @@ class HttpClient_Curl : public IHttpClient { class CurlHttpOperation { public: - static long GetPreferredHttpVersion() - { - const curl_version_info_data* versionInfo = curl_version_info(CURLVERSION_NOW); - return (versionInfo != nullptr && (versionInfo->features & CURL_VERSION_HTTP2) != 0) - ? CURL_HTTP_VERSION_2_0 - : CURL_HTTP_VERSION_1_1; - } void DispatchEvent(HttpStateEvent type) { @@ -88,7 +85,6 @@ class CurlHttpOperation { } std::atomic isAborted { false }; // Set to 'true' when async callback is aborted - /** * Create local CURL instance for url and body * @@ -97,13 +93,27 @@ class CurlHttpOperation { * @param httpConnTimeout HTTP connection timeout in seconds * @param httpReadTimeout HTTP read timeout in seconds */ + // Selects HTTP/2 only when the libcurl we are actually linked against was + // built with HTTP/2 support. Setting CURLOPT_HTTP_VERSION to + // CURL_HTTP_VERSION_2_0 against a libcurl without HTTP/2 does not silently + // downgrade -- it fails the transfer with CURLE_UNSUPPORTED_PROTOCOL -- so + // the version has to be probed at runtime rather than assumed. + static long GetPreferredHttpVersion() noexcept + { + const curl_version_info_data* versionInfo = curl_version_info(CURLVERSION_NOW); + if (versionInfo != nullptr && (versionInfo->features & CURL_VERSION_HTTP2) != 0) + { + return CURL_HTTP_VERSION_2_0; + } + return CURL_HTTP_VERSION_1_1; + } + CurlHttpOperation( std::string method, std::string url, IHttpResponseCallback* callback, - // requestHeaders is copied into the curl_slist during construction - // and need not outlive this operation. requestBody is stored by - // reference and read by Send(), so it must outlive this operation. + // requestHeaders and requestBody are copied into operation-owned storage + // so the worker does not depend on the caller retaining the request. const std::map& requestHeaders, const std::vector& requestBody, // Default connectivity and response size options @@ -123,7 +133,7 @@ class CurlHttpOperation { m_sslCaInfo(sslCaInfo), // Local vars - requestBody(requestBody) + m_requestBody(requestBody) { TRACE("--------------------------------------------------------------------------------------------------\n"); response.memory = nullptr; @@ -134,38 +144,19 @@ class CurlHttpOperation { if(!curl) { TRACE("libcurl failed to init!\n"); - res = CURLE_FAILED_INIT; - DispatchEvent(OnCreateFailed); - return; - } - -#if 0 - // Be verbose - if (!SetOption(CURLOPT_VERBOSE, 1L)) -#else - if (!SetOption(CURLOPT_VERBOSE, 0L)) -#endif - { - DispatchEvent(OnCreateFailed); - return; - } - - // Specify target URL - if (!SetOption(CURLOPT_URL, m_url.c_str()) - || !SetOption(CURLOPT_SSL_VERIFYPEER, sslVerify ? 1L : 0L) - || !SetOption(CURLOPT_SSL_VERIFYHOST, sslVerify ? 2L : 0L)) - { - DispatchEvent(OnCreateFailed); - return; - } - - if (!m_sslCaInfo.empty() && !SetOption(CURLOPT_CAINFO, m_sslCaInfo.c_str())) - { + m_transportError = CURLE_FAILED_INIT; + m_setupError = CURLE_FAILED_INIT; DispatchEvent(OnCreateFailed); return; } - if (!SetOption(CURLOPT_HTTP_VERSION, GetPreferredHttpVersion())) + if (!SetOption(CURLOPT_VERBOSE, 0L) || + !SetOption(CURLOPT_URL, m_url.c_str()) || + !SetOption(CURLOPT_SSL_VERIFYPEER, sslVerify ? 1L : 0L) || + !SetOption(CURLOPT_SSL_VERIFYHOST, sslVerify ? 2L : 0L) || + (!m_sslCaInfo.empty() && !SetOption(CURLOPT_CAINFO, m_sslCaInfo.c_str())) || + // HTTP/2 when the linked libcurl supports it, otherwise HTTP/1.1 + !SetOption(CURLOPT_HTTP_VERSION, GetPreferredHttpVersion())) { DispatchEvent(OnCreateFailed); return; @@ -177,24 +168,24 @@ class CurlHttpOperation { for (const auto& kv : requestHeaders) { std::string header = kv.first + ": " + kv.second; - curl_slist* appended = curl_slist_append(m_headersChunk, header.c_str()); - if (appended == nullptr) + curl_slist* appendedHeaders = curl_slist_append(m_headersChunk, header.c_str()); + if (appendedHeaders == nullptr) { - res = CURLE_OUT_OF_MEMORY; + m_transportError = CURLE_OUT_OF_MEMORY; + m_setupError = CURLE_OUT_OF_MEMORY; DispatchEvent(OnCreateFailed); return; } - m_headersChunk = appended; + m_headersChunk = appendedHeaders; } - if(m_headersChunk != nullptr && !SetOption(CURLOPT_HTTPHEADER, m_headersChunk)) + if (m_headersChunk != nullptr && !SetOption(CURLOPT_HTTPHEADER, m_headersChunk)) { DispatchEvent(OnCreateFailed); return; } TRACE("method=%s, url=%s\n", this->m_method.c_str(), this->m_url.c_str()); - m_isConfigured = true; DispatchEvent(OnCreated); } @@ -203,41 +194,56 @@ class CurlHttpOperation { */ virtual ~CurlHttpOperation() { - // Given the request has not been aborted we should wait for completion here - // This guarantees the lifetime of this request. - if (result.valid()) + if (m_worker.joinable()) { - result.wait(); + if (m_worker.get_id() == std::this_thread::get_id()) + { + // The completion callback can release the owning request on this + // worker. Detach rather than joining the current thread; Send() has + // finished and the worker does not touch this operation afterward. + m_worker.detach(); + } + else + { + m_worker.join(); + } } - DispatchEvent(OnDestroy); - res = CURLE_OK; + + DispatchDestroyEvent(); + m_transportError = CURLE_OK; if (curl != nullptr) { curl_easy_cleanup(curl); } - curl_slist_free_all(m_headersChunk); + if (m_headersChunk != nullptr) + { + curl_slist_free_all(m_headersChunk); + } ReleaseResponse(); } /** * Send request synchronously */ - long Send() + void Send() { TRACE("method=%s\n", this->m_method.c_str()); ReleaseResponse(); // Request buffer - const void *request = requestBody.empty() ? nullptr : requestBody.data(); - const size_t reqSize = requestBody.size(); - int socketWaitResult = 0; + const void *request = m_requestBody.empty() ? nullptr : m_requestBody.data(); + const size_t reqSize = m_requestBody.size(); + long httpStatusCode = 0; + CURLcode infoResult = CURLE_OK; - if(!curl || !m_isConfigured) + if(!curl) + { + m_transportError = CURLE_FAILED_INIT; + DispatchEvent(OnSendFailed); + goto cleanup; + } + if (m_setupError != CURLE_OK) { - if (res == CURLE_OK) - { - res = CURLE_FAILED_INIT; - } DispatchEvent(OnSendFailed); goto cleanup; } @@ -252,46 +258,52 @@ class CurlHttpOperation { goto cleanup; } DispatchEvent(OnConnecting); + m_transportError = curl_easy_perform(curl); + if(CURLE_OK != m_transportError) { - const CURLcode curlResult = curl_easy_perform(curl); - res = static_cast(curlResult); - if(CURLE_OK != curlResult) - { - DispatchEvent(OnConnectFailed); // couldn't connect - stage 1 - TRACE("Error #1: %s\n", curl_easy_strerror(curlResult)); - goto cleanup; - } + DispatchEvent(OnConnectFailed); // couldn't connect - stage 1 + TRACE("Error #1: %s\n", curl_easy_strerror(m_transportError)); + goto cleanup; } - { - CURLcode infoResult; + /* Extract the socket from the curl handle - we'll need it for waiting. + * Note that this API takes a pointer to a 'long' while we use + * curl_socket_t for sockets otherwise. + */ + #if LIBCURL_VERSION_NUM >= 0x072D00 // Version 7.45.00 - infoResult = curl_easy_getinfo(curl, CURLINFO_ACTIVESOCKET, &sockextr); + m_transportError = curl_easy_getinfo(curl, CURLINFO_ACTIVESOCKET, &sockextr); #else + { long lastSocket = -1; - infoResult = curl_easy_getinfo(curl, CURLINFO_LASTSOCKET, &lastSocket); - if (infoResult == CURLE_OK) + m_transportError = curl_easy_getinfo(curl, CURLINFO_LASTSOCKET, &lastSocket); + if (m_transportError == CURLE_OK) { sockextr = static_cast(lastSocket); } + } #endif - if(CURLE_OK != infoResult || sockextr == CURL_SOCKET_BAD) - { - res = static_cast( - infoResult != CURLE_OK ? infoResult : CURLE_COULDNT_CONNECT); - DispatchEvent(OnConnectFailed); // couldn't connect - stage 2 - TRACE("Error #2: %s\n", curl_easy_strerror(static_cast(res))); - goto cleanup; - } + + if(CURLE_OK != m_transportError) + { + DispatchEvent(OnConnectFailed); // couldn't connect - stage 2 + TRACE("Error #2: %s\n", curl_easy_strerror(m_transportError)); + goto cleanup; + } + if (sockextr == CURL_SOCKET_BAD) + { + m_transportError = CURLE_FAILED_INIT; + DispatchEvent(OnConnectFailed); // couldn't connect - no socket + TRACE("Error #2: curl returned an invalid socket\n"); + goto cleanup; } /* wait for the socket to become ready for sending */ sockfd = sockextr; - socketWaitResult = WaitOnSocket(sockfd, 0, HTTP_CONN_TIMEOUT * 1000L); - if(socketWaitResult <= 0 || isAborted) + if (WaitOnSocket(sockfd, 0, HTTP_CONN_TIMEOUT * 1000L) <= 0 || isAborted) { TRACE("Error #3: timeout, aborted=%u\n", isAborted.load() ); - res = CURLE_OPERATION_TIMEDOUT; + m_transportError = CURLE_OPERATION_TIMEDOUT; DispatchEvent(OnConnectFailed); // couldn't connect - stage 3 goto cleanup; } @@ -306,33 +318,31 @@ class CurlHttpOperation { // send all data to our callback function if (rawResponse) { - if (!SetOption(CURLOPT_HEADER, 1L) - || !SetOption(CURLOPT_WRITEFUNCTION, - static_cast(&WriteMemoryCallback)) - || !SetOption(CURLOPT_WRITEDATA, static_cast(&response))) + if (!SetOption(CURLOPT_HEADER, 1L) || + !SetOption(CURLOPT_WRITEFUNCTION, &WriteMemoryCallback) || + !SetOption(CURLOPT_WRITEDATA, static_cast(&response))) + { + DispatchEvent(OnSendFailed); + goto cleanup; + } + } else { + if (!SetOption(CURLOPT_HEADERFUNCTION, &WriteVectorCallback) || + !SetOption(CURLOPT_HEADERDATA, static_cast(&respHeaders)) || + !SetOption(CURLOPT_WRITEFUNCTION, &WriteVectorCallback) || + !SetOption(CURLOPT_WRITEDATA, static_cast(&respBody))) { DispatchEvent(OnSendFailed); goto cleanup; } - } - else if (!SetOption(CURLOPT_WRITEFUNCTION, - static_cast(&WriteVectorCallback)) - || !SetOption(CURLOPT_HEADERFUNCTION, - static_cast(&WriteVectorCallback)) - || !SetOption(CURLOPT_HEADERDATA, static_cast(&respHeaders)) - || !SetOption(CURLOPT_WRITEDATA, static_cast(&respBody))) - { - DispatchEvent(OnSendFailed); - goto cleanup; } // TODO: only two methods supported for now - POST and GET if (m_method.compare("POST") == 0) { // POST - if (!SetOption(CURLOPT_POST, 1L) - || !SetOption(CURLOPT_POSTFIELDS, static_cast(request)) - || !SetOption(CURLOPT_POSTFIELDSIZE_LARGE, static_cast(reqSize))) + if (!SetOption(CURLOPT_POST, 1L) || + !SetOption(CURLOPT_POSTFIELDS, static_cast(request)) || + !SetOption(CURLOPT_POSTFIELDSIZE_LARGE, static_cast(reqSize))) { DispatchEvent(OnSendFailed); goto cleanup; @@ -344,26 +354,23 @@ class CurlHttpOperation { } else { TRACE("Error #4: unsupported method %s\n", m_method.c_str()); - res = CURLE_UNSUPPORTED_PROTOCOL; + m_transportError = CURLE_UNSUPPORTED_PROTOCOL; goto cleanup; } - if (!SetOption(CURLOPT_LOW_SPEED_TIME, 30L) - || !SetOption(CURLOPT_LOW_SPEED_LIMIT, 4096L)) + if (!SetOption(CURLOPT_LOW_SPEED_TIME, 30L) || + !SetOption(CURLOPT_LOW_SPEED_LIMIT, 4096L)) { DispatchEvent(OnSendFailed); goto cleanup; } DispatchEvent(OnSending); + m_transportError = curl_easy_perform(curl); + if(CURLE_OK != m_transportError) { - const CURLcode curlResult = curl_easy_perform(curl); - res = static_cast(curlResult); - if(CURLE_OK != curlResult) - { - DispatchEvent(OnSendFailed); - TRACE("Error: %s\n", curl_easy_strerror(curlResult)); - goto cleanup; - } + DispatchEvent(OnSendFailed); + TRACE("Error: %s\n", curl_easy_strerror(m_transportError)); + goto cleanup; } /* Code snippet to parse raw HTTP response. This might come in handy @@ -378,46 +385,75 @@ class CurlHttpOperation { */ /* libcurl is nice enough to parse the response code itself: */ + infoResult = curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &httpStatusCode); + if (infoResult != CURLE_OK) { - long responseCode = 0; - const CURLcode infoResult = curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &responseCode); - if (infoResult != CURLE_OK) - { - res = static_cast(infoResult); - DispatchEvent(OnSendFailed); - goto cleanup; - } - res = responseCode; + m_transportError = infoResult; + DispatchEvent(OnSendFailed); + TRACE("Error getting HTTP response code: %s\n", curl_easy_strerror(m_transportError)); + goto cleanup; } + m_httpStatusCode = httpStatusCode; // We got some response from server. Dump the contents. - TRACE("HTTP response code %d\n", res); + TRACE("HTTP response code %ld\n", httpStatusCode); DispatchEvent(OnResponse); cleanup: + return; + } + + void SendAsync(std::function callback = nullptr) { + // A newly created std::thread may run before it is assigned to m_worker. + // Hold this gate until the assignment completes so a fast failure cannot + // destroy the operation from its callback while SendAsync still uses it. + { + std::lock_guard startGuard(m_workerStartMtx); + if (m_sendAttempted) + { + throw std::logic_error("CurlHttpOperation is single-use"); + } + m_sendAttempted = true; + + try + { + m_worker = std::thread([this, callback]() { + { + std::lock_guard startGuard(m_workerStartMtx); + } + try + { + Send(); + } + catch (...) + { + // std::async stored worker exceptions in its unobserved + // future. A raw thread must contain them. + m_transportError = CURLE_FAILED_INIT; + m_setupError = CURLE_FAILED_INIT; + } + Complete(callback); + }); + return; + } + catch (...) + { + // Callable allocation/copy or std::thread creation failed. + } + } - // This function returns: - // - on success: HTTP status code. - // - on failure: CURL error code. - // The two sets of enums (CURLE, HTTP codes) - do not intersect, so we collapse them in one set. - return res; + m_transportError = CURLE_FAILED_INIT; + m_setupError = CURLE_FAILED_INIT; + Complete(callback); } - std::future & SendAsync(std::function callback = nullptr) { - result = std::async(std::launch::async, [this, callback] { - long result = Send(); - if (callback!=nullptr) - callback(*this); - return result; - }); - return result; + CURLcode GetTransportError() const + { + return m_transportError; } - /** - * Get HTTP response code. This function returns CURL error code if HTTP response code is invalid. - */ - long GetResponseCode() + long GetHttpStatusCode() const { - return res; + return m_httpStatusCode; } /** @@ -428,6 +464,11 @@ class CurlHttpOperation { return isAborted.load(); } + CURLcode GetSetupError() const + { + return m_setupError; + } + /** * Return a copy of response headers * @@ -521,20 +562,18 @@ class CurlHttpOperation { const size_t httpConnTimeout; // Timeout for connect. Default: 5s CURL *curl; // Local curl instance - long res = CURLE_OK; // Curl result OR HTTP status code if successful - + CURLcode m_transportError = CURLE_OK; + CURLcode m_setupError = CURLE_OK; + long m_httpStatusCode = 0; + IHttpResponseCallback* m_callback = nullptr; // Request values std::string m_method; std::string m_url; std::string m_sslCaInfo; - bool m_isConfigured = false; - // The SDK upload path keeps the owning IHttpRequest alive through the - // callback context until Send() completes; copying this body would duplicate - // every upload payload. Unlike CURLOPT_CAINFO, the body pointer is set and - // consumed during Send(), not retained from construction. - const std::vector& requestBody; + // Own the payload so operation lifetime is independent of CurlHttpRequest. + std::vector m_requestBody; struct curl_slist *m_headersChunk = nullptr; // Processed response headers and body @@ -550,20 +589,65 @@ class CurlHttpOperation { size_t sendlen = 0; // # bytes sent by client size_t acklen = 0; // # bytes ack by server - std::future result; + std::mutex m_workerStartMtx; + bool m_sendAttempted = false; + std::thread m_worker; + std::atomic m_destroyEventDispatched { false }; - template - bool SetOption(CURLoption option, TValue value) + void DispatchDestroyEvent() noexcept { - const CURLcode optionResult = curl_easy_setopt(curl, option, value); - if (optionResult != CURLE_OK) + if (!m_destroyEventDispatched.exchange(true, std::memory_order_acq_rel)) + { + try + { + DispatchEvent(OnDestroy); + } + catch (...) + { + // State observers must not terminate the worker or destructor. + } + } + } + + void Complete(const std::function& callback) noexcept + { + // Preserve the documented state event while m_callback is still valid. + // The completion callback can release the last owner, so this must remain + // the worker's final access to the operation. + DispatchDestroyEvent(); + try { - res = static_cast(optionResult); - TRACE("curl_easy_setopt(%d) failed: %s\n", - static_cast(option), curl_easy_strerror(optionResult)); + if (callback != nullptr) + { + callback(*this); + } + } + catch (...) + { + // Match the old unobserved-future behavior at the thread boundary. + } + } + + template + bool SetOption(CURLoption option, T value) + { + if (curl == nullptr) + { + m_transportError = CURLE_FAILED_INIT; + m_setupError = CURLE_FAILED_INIT; return false; } - return true; + + const CURLcode optionResult = curl_easy_setopt(curl, option, value); + if (optionResult == CURLE_OK) + { + return true; + } + + LOG_WARN("curl_easy_setopt(%d) failed: %s", static_cast(option), curl_easy_strerror(optionResult)); + m_transportError = optionResult; + m_setupError = optionResult; + return false; } /** @@ -607,14 +691,14 @@ class CurlHttpOperation { * @param userp * @return */ - static size_t WriteMemoryCallback(char *contents, size_t size, size_t nmemb, void *userp) + static size_t WriteMemoryCallback(char* contents, size_t size, size_t nmemb, void* userp) { // Guard the size * nmemb product against size_t overflow before using it. if (nmemb != 0 && size > static_cast(-1) / nmemb) { return 0; } size_t realsize = size * nmemb; - struct MemoryStruct *mem = (struct MemoryStruct *)userp; + auto* mem = static_cast(userp); // SECURITY: bound the buffered response (see kMaxResponseBytes). Compare // overflow-safely (mem->size is always <= kMaxResponseBytes here). Returning a @@ -651,7 +735,7 @@ class CurlHttpOperation { * @param data * @return */ - static size_t WriteVectorCallback(char *ptr, size_t size, size_t nmemb, void* userp) + static size_t WriteVectorCallback(char* ptr, size_t size, size_t nmemb, void* userp) { // Guard the size * nmemb product against size_t overflow before using it. if (nmemb != 0 && size > static_cast(-1) / nmemb) { diff --git a/lib/http/HttpClient_WinHttp.cpp b/lib/http/HttpClient_WinHttp.cpp new file mode 100644 index 000000000..f3ad915c6 --- /dev/null +++ b/lib/http/HttpClient_WinHttp.cpp @@ -0,0 +1,1647 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#include "mat/config.h" + +#ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT +#include "HttpClient_WinHttp.hpp" +#include "utils/StringConversion.hpp" +#include "utils/StringUtils.hpp" + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#pragma comment(lib, "crypt32.lib") +#pragma comment(lib, "winhttp.lib") + +namespace MAT_NS_BEGIN { + +namespace { + +constexpr DWORD DEFAULT_MAX_CONNECTIONS_PER_SERVER = 4; + +void setConnectionLimits(HINTERNET session, DWORD maxConnections) noexcept +{ + if (session == nullptr) + { + return; + } + + if (!::WinHttpSetOption(session, WINHTTP_OPTION_MAX_CONNS_PER_SERVER, + &maxConnections, sizeof(maxConnections))) + { + LOG_WARN("WinHttpSetOption(MAX_CONNS_PER_SERVER) failed: %d", ::GetLastError()); + } + if (!::WinHttpSetOption(session, WINHTTP_OPTION_MAX_CONNS_PER_1_0_SERVER, + &maxConnections, sizeof(maxConnections))) + { + LOG_WARN("WinHttpSetOption(MAX_CONNS_PER_1_0_SERVER) failed: %d", ::GetLastError()); + } +} + +} // namespace + +class WinHttpRequestWrapper; + +struct WinHttpClientState +{ + explicit WinHttpClientState(HINTERNET sessionHandle); + ~WinHttpClientState(); + + bool registerRequest( + std::string const& id, + std::shared_ptr request); + void eraseRequest(std::string const& id); + void stopAcceptingRequests(); + void beginCallback(); + void beginCallbackLocked(); + void endCallback(); + + HINTERNET session; + std::mutex requestsMutex; + std::map> requests; + std::condition_variable requestsCv; + std::atomic msRootCheck {false}; + bool acceptingRequests {true}; + size_t cancelAllDepth {0}; + size_t registryGeneration {0}; + size_t callbackGeneration {0}; + size_t callbacksInFlight {0}; + std::map callbacksByThread; +}; + +struct WinHttpCallbackAlreadyStarted +{ +}; + +class WinHttpCallbackScope +{ + public: + explicit WinHttpCallbackScope(std::shared_ptr state) + : m_state(std::move(state)) + { + m_state->beginCallback(); + } + + WinHttpCallbackScope( + std::shared_ptr state, + WinHttpCallbackAlreadyStarted) + : m_state(std::move(state)) + { + } + + ~WinHttpCallbackScope() + { + m_state->endCallback(); + } + + WinHttpCallbackScope(WinHttpCallbackScope const&) = delete; + WinHttpCallbackScope& operator=(WinHttpCallbackScope const&) = delete; + + private: + std::shared_ptr m_state; +}; + +// Ownership of the WinHTTP status-callback context. +// +// WinHTTP keeps the context value associated with a request handle until that +// handle is torn down, and documents WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING as +// the final callback for the handle ("There will be no more callbacks for this +// handle"). The context therefore holds a *strong* reference to the wrapper: +// every buffer WinHTTP was handed lives inside (or is kept alive by) that +// wrapper, so it stays valid for exactly as long as WinHTTP can still touch it. +// The reference is released only from the HANDLE_CLOSING callback, which also +// deletes the context. +struct WinHttpCallbackContext +{ + explicit WinHttpCallbackContext(std::shared_ptr request) + : request(std::move(request)) + { + } + + std::shared_ptr request; +}; + +class WinHttpRequestWrapper : public std::enable_shared_from_this +{ + protected: + // The step the WinHTTP state machine should take next. Operations are never + // issued directly from a completion callback; see schedule()/runPump(). + enum class NextOperation + { + None, + WriteBody, + ReceiveResponse, + QueryDataAvailable, + ReadData, + Complete + }; + + std::shared_ptr m_clientState; + std::string m_id; + IHttpResponseCallback* m_appCallback {nullptr}; + HINTERNET m_hConnect {nullptr}; + HINTERNET m_hRequest {nullptr}; + SimpleHttpRequest* m_request; + std::vector m_bodyBuffer; + // Fixed response read buffer. WinHttpReadData keeps the pointer until the + // read completes, so the buffer must never move for the life of the + // request; sizing it once up front also keeps the number of read + // completions needed to drain a response low (see MAX_HTTP_RESPONSE_SIZE, + // which still bounds the total that is buffered). + uint8_t m_readBuffer[8192] {0}; + size_t m_bodyWritten {0}; + std::atomic isCallbackCalled {false}; + bool isAborted {false}; + bool m_isHttps {false}; + bool m_msRootCheckRequired {false}; + std::atomic m_msRootCheckCompleted {false}; + bool m_contextInstalled {false}; + bool m_sendIssued {false}; + bool m_handleCallInProgress {false}; + bool m_closeRequestAfterCall {false}; + unsigned m_stateCallbackDepth {0}; + std::map m_stateCallbacksByThread; + bool m_stateCompletionPending {false}; + DWORD m_stateCompletionError {ERROR_SUCCESS}; + // Reason recorded by an abort that must let WinHTTP report the terminal + // callback itself instead of completing inline. + std::atomic m_deferredError {ERROR_SUCCESS}; + + // requestsMutex may nest this mutex only while the initial send claims or + // releases the pump. Code holding m_pumpMutex must release it before any + // operation that acquires requestsMutex. + std::mutex m_pumpMutex; + bool m_pumpActive {false}; + NextOperation m_nextOperation {NextOperation::None}; + DWORD m_completionError {ERROR_SUCCESS}; + + public: + WinHttpRequestWrapper( + std::shared_ptr clientState, + SimpleHttpRequest* request) + : m_clientState(std::move(clientState)), + m_id(request->GetId()), + m_request(request) + { + LOG_TRACE("%p WinHttpRequestWrapper()", this); + } + + WinHttpRequestWrapper(WinHttpRequestWrapper const&) = delete; + WinHttpRequestWrapper& operator=(WinHttpRequestWrapper const&) = delete; + + // The caller must hold m_clientState->requestsMutex. + bool hasStateCallbackOnThreadLocked(std::thread::id threadId) const + { + return m_stateCallbacksByThread.find(threadId) != + m_stateCallbacksByThread.end(); + } + + // The caller must hold m_clientState->requestsMutex. + bool hasActiveStateCallbackLocked() const + { + return m_stateCallbackDepth != 0; + } + + ~WinHttpRequestWrapper() noexcept + { + LOG_TRACE("%p ~WinHttpRequestWrapper()", this); + // Both completion and cancellation close the request handle explicitly: + // while WinHTTP owns the callback context it also owns a strong + // reference to this object, so the destructor can never be what closes + // that handle. Anything still open here belongs to a request that + // failed before WinHTTP took ownership of the context. + if (m_hRequest != nullptr) + { + ::WinHttpCloseHandle(m_hRequest); + } + if (m_hConnect != nullptr) + { + ::WinHttpCloseHandle(m_hConnect); + } + } + + /// + /// Asynchronously cancel pending request. + /// + /// Unlike WinInet's InternetCloseHandle, WinHttpCloseHandle on a request + /// with a pending async operation blocks the calling thread until that + /// operation's completion callback has finished running -- and that + /// callback runs on a *different* WinHTTP-internal thread. Holding + /// m_clientState->requestsMutex across the call (WinInet's pattern, safe there + /// because its callback runs synchronously on the calling thread) would + /// deadlock here: this thread would block inside WinHttpCloseHandle holding + /// the lock, while the completion callback blocks on the same thread's + /// erase() needing that same lock. So the handle is captured and closed + /// without holding the lock. This wrapper is only reachable through a + /// shared_ptr (see WinHttpClientState::requests / CancelRequestAsync), so + /// releasing the lock here cannot race with the object being freed -- + /// the caller already holds its own shared_ptr keeping *this* alive. + /// + void cancel() + { + abortRequest(ERROR_WINHTTP_OPERATION_CANCELLED); + } + + /// + /// Tears the request down and records why, without delivering the terminal + /// response from this call. + /// + /// WinHttpSendRequest documents that buffers handed to WinHTTP must stay + /// valid until an aborted operation reports + /// WINHTTP_CALLBACK_STATUS_REQUEST_ERROR with ERROR_WINHTTP_OPERATION_CANCELLED, + /// and invoking OnHttpResponse() is precisely what lets the caller destroy + /// the request object those buffers live in. Synthesizing the response as + /// soon as WinHttpCloseHandle returns would assume a teardown ordering + /// WinHTTP does not guarantee, so instead the handle is closed and the + /// response is delivered from the resulting REQUEST_ERROR callback -- or + /// from HANDLE_CLOSING, which WinHTTP always delivers last. + /// + void abortRequest(DWORD dwError, bool calledFromWinHttpCallback = false) + { + HINTERNET hRequestToClose = nullptr; + bool completeHere = false; + { + std::lock_guard lock(m_clientState->requestsMutex); + if (isCallbackCalled) + { + return; + } + isAborted = true; + DWORD noError = ERROR_SUCCESS; + m_deferredError.compare_exchange_strong(noError, dwError); + if (m_handleCallInProgress && !calledFromWinHttpCallback) + { + // WinHTTP forbids another thread from closing an asynchronous + // handle while this thread is inside WinHttpSendRequest or + // WinHttpWriteData. Record the cancellation and let that API + // frame close the handle as soon as its call returns. + m_closeRequestAfterCall = true; + return; + } + hRequestToClose = m_hRequest; + m_hRequest = nullptr; + // Without an installed callback context WinHTTP has no way to + // report HANDLE_CLOSING back to this object, so nothing else would + // ever complete the request. And until WinHttpSendRequest has been + // issued WinHTTP holds none of this request's buffers, so there is + // nothing to wait for. Both states may be completed inline. + completeHere = !m_contextInstalled || !m_sendIssued; + } + if (hRequestToClose != nullptr) + { + ::WinHttpCloseHandle(hRequestToClose); + } + if (completeHere) + { + onRequestComplete(dwError); + } + } + + /// + /// Verify that the server end-point certificate is MS-Rooted. + /// Unlike WinInet's INTERNET_OPTION_SERVER_CERT_CHAIN_CONTEXT (which hands + /// back a ready-made chain), WinHttpQueryOption only returns the leaf server + /// certificate context, so the chain must be built explicitly before running + /// the same CERT_CHAIN_POLICY_MICROSOFT_ROOT policy check WinInet performs. + /// + bool isMsRootCert(HINTERNET hRequest) + { + PCCERT_CONTEXT pCertContext = nullptr; + DWORD dwSize = sizeof(pCertContext); + if (!::WinHttpQueryOption(hRequest, WINHTTP_OPTION_SERVER_CERT_CONTEXT, &pCertContext, &dwSize)) + { + LOG_WARN("WinHttpQueryOption(SERVER_CERT_CONTEXT) failed: %d", ::GetLastError()); + return false; + } + + bool result = true; + PCCERT_CHAIN_CONTEXT pChainCtx = nullptr; + CERT_CHAIN_PARA chainPara = { sizeof(chainPara) }; + if (::CertGetCertificateChain(NULL, pCertContext, NULL, pCertContext->hCertStore, &chainPara, 0, NULL, &pChainCtx)) + { + CERT_CHAIN_POLICY_STATUS pps = { 0, 0, 0, 0, nullptr }; + pps.cbSize = sizeof(pps); + // Verify that the cert chain roots up to the Microsoft application root at top level + CERT_CHAIN_POLICY_PARA policyPara = { 0, 0, nullptr }; + policyPara.cbSize = sizeof(policyPara); + policyPara.dwFlags = MICROSOFT_ROOT_CERT_CHAIN_POLICY_CHECK_APPLICATION_ROOT_FLAG; + policyPara.pvExtraPolicyPara = nullptr; + + BOOL policyChecked = ::CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_MICROSOFT_ROOT, pChainCtx, &policyPara, &pps); + if (!policyChecked) + { + LOG_WARN("CertVerifyCertificateChainPolicy() failed: unable to verify"); + result = false; + } + else if (pps.dwError != ERROR_SUCCESS) + { + LOG_WARN("CertVerifyCertificateChainPolicy() failed: invalid root CA - %d", pps.dwError); + result = false; + } + ::CertFreeCertificateChain(pChainCtx); + } + else + { + LOG_WARN("CertGetCertificateChain() failed: %d", ::GetLastError()); + result = false; + } + ::CertFreeCertificateContext(pCertContext); + return result; + } + + HINTERNET getRequestHandle() + { + std::lock_guard lock(m_clientState->requestsMutex); + return m_hRequest; + } + + // Keep each WinHTTP operation and the handle check under the same lock as + // cancellation. WinHttpCloseHandle remains outside the lock because it + // waits for callbacks that may need this mutex. + DWORD receiveResponse() + { + std::lock_guard lock(m_clientState->requestsMutex); + if (m_hRequest == nullptr) + { + return ERROR_WINHTTP_OPERATION_CANCELLED; + } + if (!::WinHttpReceiveResponse(m_hRequest, NULL)) + { + return ::GetLastError(); + } + return ERROR_SUCCESS; + } + + DWORD queryDataAvailable() + { + std::lock_guard lock(m_clientState->requestsMutex); + if (m_hRequest == nullptr) + { + return ERROR_WINHTTP_OPERATION_CANCELLED; + } + if (!::WinHttpQueryDataAvailable(m_hRequest, NULL)) + { + return ::GetLastError(); + } + return ERROR_SUCCESS; + } + + DWORD readData() + { + std::lock_guard lock(m_clientState->requestsMutex); + if (m_hRequest == nullptr) + { + return ERROR_WINHTTP_OPERATION_CANCELLED; + } + if (!::WinHttpReadData(m_hRequest, m_readBuffer, + static_cast(sizeof(m_readBuffer)), NULL)) + { + return ::GetLastError(); + } + return ERROR_SUCCESS; + } + + // Hands the remaining request body to WinHTTP. The body is deliberately not + // passed as WinHttpSendRequest's lpOptional: that buffer belongs to the + // caller's request object and WinHTTP may hold it until the request handle + // is closed, whereas WinHttpWriteData releases it at WRITE_COMPLETE. + DWORD writeBody() + { + HINTERNET request = nullptr; + const void* body = nullptr; + DWORD bodySize = 0; + { + std::lock_guard lock(m_clientState->requestsMutex); + if (m_hRequest == nullptr) + { + return ERROR_WINHTTP_OPERATION_CANCELLED; + } + size_t remaining = m_request->m_body.size() - m_bodyWritten; + request = m_hRequest; + body = m_request->m_body.data() + m_bodyWritten; + bodySize = static_cast(remaining); + m_handleCallInProgress = true; + } + + BOOL result = ::WinHttpWriteData(request, body, bodySize, NULL); + DWORD error = result ? ERROR_SUCCESS : ::GetLastError(); + + HINTERNET cancelledRequest = nullptr; + { + std::lock_guard lock(m_clientState->requestsMutex); + m_handleCallInProgress = false; + if (m_closeRequestAfterCall) + { + m_closeRequestAfterCall = false; + cancelledRequest = m_hRequest; + m_hRequest = nullptr; + } + } + if (cancelledRequest != nullptr) + { + ::WinHttpCloseHandle(cancelledRequest); + } + return error; + } + + DWORD validateCurrentRequestMsRootCert() + { + std::lock_guard lock(m_clientState->requestsMutex); + if (m_hRequest == nullptr) + { + return ERROR_WINHTTP_OPERATION_CANCELLED; + } + return isMsRootCert(m_hRequest) ? ERROR_SUCCESS : ERROR_WINHTTP_SECURE_INVALID_CERT; + } + + // Detaches and closes the request handle. WinHttpCloseHandle can block + // until an in-flight callback returns, and that callback may need + // m_clientState->requestsMutex, so the handle is detached under the lock and + // closed without it. + void closeRequestHandle() + { + HINTERNET hRequestToClose = nullptr; + { + std::lock_guard lock(m_clientState->requestsMutex); + hRequestToClose = m_hRequest; + m_hRequest = nullptr; + } + if (hRequestToClose != nullptr) + { + ::WinHttpCloseHandle(hRequestToClose); + } + } + + // Queues the next step of the WinHTTP state machine. + // + // WinHTTP is explicitly allowed to complete an operation synchronously and + // re-enter this object's status callback on the calling thread ("reentered + // on the same thread for the current request"). Issuing the next WinHTTP + // call straight from a completion would then nest a pair of stack frames + // per response chunk -- unbounded for a large response -- and would also + // re-enter m_clientState->requestsMutex, which is not recursive. So only the + // outermost frame ever issues operations: a nested completion records what + // should happen next and returns, and runPump() picks it up once the + // WinHTTP call it was nested inside has returned. + void schedule(NextOperation next, DWORD completionError = ERROR_SUCCESS) + { + { + std::lock_guard lock(m_pumpMutex); + if (m_nextOperation == NextOperation::Complete && next != NextOperation::Complete) + { + // A terminal result is already queued; nothing may displace it. + return; + } + m_nextOperation = next; + m_completionError = completionError; + if (m_pumpActive) + { + return; + } + m_pumpActive = true; + } + runPump(); + } + + // Issues queued operations until WinHTTP takes one asynchronously. The + // caller must already own the pump (m_pumpActive set) and must not hold + // m_clientState->requestsMutex. + void runPump() + { + for (;;) + { + NextOperation current = NextOperation::None; + DWORD completionError = ERROR_SUCCESS; + { + std::lock_guard lock(m_pumpMutex); + current = m_nextOperation; + completionError = m_completionError; + m_nextOperation = NextOperation::None; + if (current == NextOperation::None || isCallbackCalled) + { + m_pumpActive = false; + return; + } + if (current == NextOperation::Complete) + { + m_pumpActive = false; + } + } + + if (current == NextOperation::Complete) + { + onRequestComplete(completionError); + return; + } + + DWORD dwError = issueOperation(current); + if (dwError == ERROR_SUCCESS) + { + continue; + } + + { + std::lock_guard lock(m_pumpMutex); + m_nextOperation = NextOperation::None; + m_pumpActive = false; + } + if (current == NextOperation::WriteBody) + { + // A synchronous WinHttpWriteData failure leaves no documented + // way to prove WinHTTP has let go of the caller's body buffer, + // so let the handle's final callback deliver the response. + abortRequest(dwError); + } + else + { + onRequestComplete(dwError); + } + return; + } + } + + DWORD issueOperation(NextOperation operation) + { + switch (operation) + { + case NextOperation::WriteBody: + return writeBody(); + + case NextOperation::ReceiveResponse: + return receiveResponse(); + + case NextOperation::QueryDataAvailable: + return queryDataAvailable(); + + case NextOperation::ReadData: + return readData(); + + default: + return ERROR_SUCCESS; + } + } + + void DispatchEvent(std::unique_lock& lock, HttpStateEvent type) + { + if (m_appCallback != nullptr && !isCallbackCalled) + { + void* handle = static_cast(m_hRequest); + IHttpResponseCallback* callback = m_appCallback; + auto state = m_clientState; + ++m_stateCallbackDepth; + ++m_stateCallbacksByThread[std::this_thread::get_id()]; + state->beginCallbackLocked(); + lock.unlock(); + { + WinHttpCallbackScope callbackScope( + state, WinHttpCallbackAlreadyStarted {}); + callback->OnHttpStateEvent(type, handle, 0); + } + + bool complete = false; + DWORD completionError = ERROR_SUCCESS; + { + lock.lock(); + assert(m_stateCallbackDepth != 0); + --m_stateCallbackDepth; + auto stateCallback = m_stateCallbacksByThread.find( + std::this_thread::get_id()); + assert(stateCallback != m_stateCallbacksByThread.end()); + if (stateCallback != m_stateCallbacksByThread.end() && + --stateCallback->second == 0) + { + m_stateCallbacksByThread.erase(stateCallback); + } + if (m_stateCallbackDepth == 0 && m_stateCompletionPending) + { + complete = true; + completionError = m_stateCompletionError; + m_stateCompletionPending = false; + m_stateCompletionError = ERROR_SUCCESS; + } + } + if (complete) + { + // Terminal delivery may free the application callback. Leave the + // setup lock released, matching the existing DispatchEvent + // contract when a state callback synchronously completes. + lock.unlock(); + onRequestComplete(completionError); + } + } + } + + // Asynchronously send HTTP request and invoke response callback. + // Ownership semantics: send(...) method self-destroys *this* upon + // reaching the terminal WinHTTP callback. There must be absolutely no + // methods that attempt to use the object after triggering send on it. + // Send operation on request may be issued no more than once. + // + // Handle setup runs under m_clientState->requestsMutex. State callbacks are the + // deliberate exception: DispatchEvent releases the lock while invoking + // application code, then setup checks cancellation before continuing. + // + // DEADLOCK NOTE: the lock must NOT still be held when a synchronous + // failure completes the request. onRequestComplete() invokes the + // application callback, which is documented (below) to be able to tear the + // client down synchronously -- that reaches CancelAllRequests(), which + // waits on the shared state's condition variable. DispatchEvent releases this lock + // around application state callbacks. If a callback completes the request, + // it leaves the lock released and sendLocked() returns without touching the + // client again; otherwise it reacquires the lock before setup continues. + void send(IHttpResponseCallback* callback) + { + m_appCallback = callback; + std::shared_ptr keepAlive = shared_from_this(); + if (!m_clientState->registerRequest(m_id, keepAlive)) + { + onRequestComplete(ERROR_WINHTTP_OPERATION_CANCELLED); + return; + } + + bool failed = false; + DWORD dwError = ERROR_SUCCESS; + { + std::unique_lock lock(m_clientState->requestsMutex); + failed = !sendLocked(lock, dwError); + } + if (failed) + { + onRequestComplete(dwError); + return; + } + // sendLocked() claimed the pump before calling WinHttpSendRequest, so a + // completion WinHTTP delivered synchronously on this thread could only + // park the next step instead of issuing it while the setup lock was + // still held. Run whatever it parked now that the lock is gone. + runPump(); + } + + // Returns true if the request was handed off to WinHTTP asynchronously. + // Returns false on synchronous failure, setting dwError to the result the + // caller must complete the request with (once the lock has been dropped). + bool sendLocked(std::unique_lock& lock, DWORD& dwErrorOut) + { + if (isCallbackCalled || isAborted) + { + // Request force-aborted before creating a WinHTTP handle. + if (!isCallbackCalled) + { + DispatchEvent(lock, OnConnectFailed); + } + dwErrorOut = ERROR_WINHTTP_OPERATION_CANCELLED; + return false; + } + + DispatchEvent(lock, OnConnecting); + if (isCallbackCalled || isAborted) + { + dwErrorOut = ERROR_WINHTTP_OPERATION_CANCELLED; + return false; + } + + std::wstring wUrl = to_utf16_string(m_request->m_url); + URL_COMPONENTS urlc; + memset(&urlc, 0, sizeof(urlc)); + urlc.dwStructSize = sizeof(urlc); + wchar_t hostname[256] = { 0 }; + urlc.lpszHostName = hostname; + urlc.dwHostNameLength = ARRAYSIZE(hostname); + wchar_t path[1024] = { 0 }; + urlc.lpszUrlPath = path; + urlc.dwUrlPathLength = ARRAYSIZE(path); + if (!::WinHttpCrackUrl(wUrl.c_str(), static_cast(wUrl.size()), 0, &urlc)) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.c_str()); + // Invalid URL passed to WinHTTP API + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = dwError; + return false; + } + + if (m_clientState->session == nullptr) + { + LOG_WARN("WinHttpOpen() did not produce a usable session handle"); + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = ERROR_WINHTTP_CANNOT_CONNECT; + return false; + } + + // TODO: connect handle for the same target should be cached across + // requests to enable keep-alive (same pre-existing opportunity noted + // in HttpClient_WinInet.cpp; out of scope for this transport swap). + m_hConnect = ::WinHttpConnect(m_clientState->session, hostname, urlc.nPort, 0); + if (m_hConnect == nullptr) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpConnect() failed: %d", dwError); + // Cannot connect to host + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = dwError; + return false; + } + + std::wstring wMethod = to_utf16_string(m_request->m_method); + m_isHttps = (urlc.nScheme == INTERNET_SCHEME_HTTPS); + // Latch the policy for this request: the callbacks that enforce it run + // long after send() returns, and the setting can be changed at any time. + m_msRootCheckRequired = + m_clientState->msRootCheck.load(std::memory_order_acquire); + m_hRequest = ::WinHttpOpenRequest( + m_hConnect, wMethod.c_str(), path, NULL, WINHTTP_NO_REFERER, + WINHTTP_DEFAULT_ACCEPT_TYPES, + WINHTTP_FLAG_REFRESH | (m_isHttps ? WINHTTP_FLAG_SECURE : 0)); + if (m_hRequest == nullptr) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpOpenRequest() failed: %d", dwError); + // Request cannot be opened to given URL because of some connectivity issue + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = dwError; + return false; + } + + // Match the WinInet transport's INTERNET_FLAG_NO_AUTH behavior. + // Telemetry requests must not answer server or proxy authentication + // challenges with ambient process credentials. + DWORD disableFeatures = WINHTTP_DISABLE_AUTHENTICATION; + if (m_msRootCheckRequired) + { + // Automatic redirects would move the request to a new TLS peer + // after the original certificate check, potentially forwarding + // telemetry credentials to a non-Microsoft-root endpoint. + disableFeatures |= WINHTTP_DISABLE_REDIRECTS; + } + if (!::WinHttpSetOption( + m_hRequest, WINHTTP_OPTION_DISABLE_FEATURE, &disableFeatures, sizeof(disableFeatures))) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpSetOption(DISABLE_AUTHENTICATION) failed: %d", dwError); + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = dwError; + return false; + } + + // Unlike WinInet, WinHTTP has no automatic cookie jar to suppress (it + // never manages cookies on the caller's behalf) and never shows UI, so + // neither INTERNET_FLAG_NO_COOKIES nor INTERNET_FLAG_NO_UI has a WinHTTP + // equivalent to set here. + + // WinHttpSetStatusCallback returns the PREVIOUS callback function + // pointer (typically NULL here, since this is the first registration + // on a freshly opened request handle) -- not a BOOL -- and signals + // failure only via the distinct WINHTTP_INVALID_STATUS_CALLBACK + // sentinel. Treating a null "previous callback" as failure would + // reject every request immediately after this call. + if (::WinHttpSetStatusCallback(m_hRequest, &WinHttpRequestWrapper::winHttpCallback, + WINHTTP_CALLBACK_FLAG_ALL_COMPLETIONS | + WINHTTP_CALLBACK_FLAG_HANDLES | + WINHTTP_CALLBACK_FLAG_SEND_REQUEST, + 0) == WINHTTP_INVALID_STATUS_CALLBACK) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpSetStatusCallback() failed: %d", dwError); + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = dwError; + return false; + } + + // Install the callback context explicitly, before anything else can + // fail. Relying on WinHttpSendRequest's dwContext instead would strand + // the context (and the strong reference it holds) whenever the send + // fails before WinHTTP records it -- WinHTTP would then report + // HANDLE_CLOSING with a zero context and nothing would free it. Once + // the option is set, the handle owns the context and HANDLE_CLOSING is + // guaranteed to hand it back. Until then unique_ptr owns it, so no path + // out of this function can leak it. + std::unique_ptr context(new WinHttpCallbackContext(shared_from_this())); + DWORD_PTR contextValue = reinterpret_cast(context.get()); + if (!::WinHttpSetOption( + m_hRequest, WINHTTP_OPTION_CONTEXT_VALUE, &contextValue, sizeof(contextValue))) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpSetOption(CONTEXT_VALUE) failed: %d", dwError); + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = dwError; + return false; + } + context.release(); + m_contextInstalled = true; + + std::ostringstream os; + for (auto const& header : m_request->m_headers) { + os << header.first << ": " << header.second << "\r\n"; + } + std::wstring wHeaders = to_utf16_string(os.str()); + + if (!wHeaders.empty() && + wHeaders.size() > static_cast(std::numeric_limits::max())) + { + LOG_WARN("Request headers exceed WinHTTP's maximum size"); + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = ERROR_INVALID_PARAMETER; + return false; + } + if (!wHeaders.empty() && + !::WinHttpAddRequestHeaders(m_hRequest, wHeaders.c_str(), static_cast(wHeaders.size()), + WINHTTP_ADDREQ_FLAG_ADD | WINHTTP_ADDREQ_FLAG_REPLACE)) + { + DWORD dwError = ::GetLastError(); + LOG_WARN("WinHttpAddRequestHeaders() failed: %d", dwError); + // Unable to add request headers. There's no point in proceeding with upload because + // our server is expecting those custom request headers to always be there. + DispatchEvent(lock, OnConnectFailed); + dwErrorOut = dwError; + return false; + } + + // Try to send headers and request body to server + DispatchEvent(lock, OnSending); + if (isCallbackCalled || isAborted) + { + dwErrorOut = ERROR_WINHTTP_OPERATION_CANCELLED; + return false; + } + if (m_request->m_body.size() > static_cast(std::numeric_limits::max())) + { + LOG_WARN("Request body exceeds WinHTTP's maximum size"); + DispatchEvent(lock, OnSendFailed); + dwErrorOut = ERROR_INVALID_PARAMETER; + return false; + } + if (m_hRequest == nullptr) + { + dwErrorOut = ERROR_WINHTTP_OPERATION_CANCELLED; + return false; + } + // Send the headers only. dwTotalLength still declares Content-Length, so + // the server sees the same request; the body follows via + // WinHttpWriteData. The SENDING_REQUEST callback validates the negotiated + // certificate before WinHTTP commits these headers to the wire. + DWORD totalLength = static_cast(m_request->m_body.size()); + // Claim the pump so that a completion WinHTTP may deliver synchronously + // on this thread parks its next step instead of issuing a WinHTTP call + // (and re-entering the shared-state mutex) while setup still holds the lock. + // send() releases the pump once the lock is gone. + { + std::lock_guard pumpLock(m_pumpMutex); + m_pumpActive = true; + m_nextOperation = NextOperation::None; + } + m_sendIssued = true; + m_handleCallInProgress = true; + HINTERNET hRequest = m_hRequest; + // SENDING_REQUEST may run synchronously from WinHttpSendRequest and must + // acquire requestsMutex to enforce the certificate policy. Keep the + // wrapper alive, but release the registry lock across the WinHTTP call. + lock.unlock(); + BOOL bResult = ::WinHttpSendRequest( + hRequest, WINHTTP_NO_ADDITIONAL_HEADERS, 0, + WINHTTP_NO_REQUEST_DATA, 0, totalLength, contextValue); + DWORD dwSendError = bResult ? ERROR_SUCCESS : ::GetLastError(); + lock.lock(); + m_handleCallInProgress = false; + HINTERNET cancelledRequest = nullptr; + if (m_closeRequestAfterCall) + { + m_closeRequestAfterCall = false; + cancelledRequest = m_hRequest; + m_hRequest = nullptr; + } + if (cancelledRequest != nullptr) + { + // Closing the handle may synchronously invoke a terminal callback, + // which acquires requestsMutex through onRequestComplete(). + lock.unlock(); + ::WinHttpCloseHandle(cancelledRequest); + lock.lock(); + } + if (!bResult) + { + DWORD dwError = m_deferredError.load(std::memory_order_acquire); + if (dwError == ERROR_SUCCESS) + { + dwError = dwSendError; + } + { + std::lock_guard pumpLock(m_pumpMutex); + m_pumpActive = false; + m_nextOperation = NextOperation::None; + } + // The send never started, so WinHTTP holds none of this request's + // buffers and cancellation may still complete inline. It does keep + // the context on the request handle and delivers HANDLE_CLOSING once + // onRequestComplete() closes that handle, which is what frees it. + m_sendIssued = false; + LOG_WARN("WinHttpSendRequest() failed: %d", dwError); + // Unable to send request + DispatchEvent(lock, OnSendFailed); + dwErrorOut = dwError; + return false; + } + // Async request has been queued; completion arrives via winHttpCallback. + return true; + } + + // Drives the WinHTTP async state machine: SendRequest -> (certificate + // policy) -> WriteData -> ReceiveResponse -> (QueryDataAvailable -> + // ReadData)* -> onRequestComplete. Unlike WinInet (whose async completions + // all report through the single INTERNET_STATUS_REQUEST_COMPLETE code, and + // whose synchronous API calls signal a pending async op via a FALSE return + // + GetLastError()==ERROR_IO_PENDING), WinHTTP has one distinct callback + // status per stage, and a FALSE return from any of these calls on an async + // handle is always a genuine synchronous failure -- never "pending". + // + // No stage issues the next WinHTTP call directly: everything goes through + // schedule(), so a completion WinHTTP delivers synchronously on the calling + // thread cannot nest another operation inside the one it is reporting. + static void CALLBACK winHttpCallback(HINTERNET hInternet, DWORD_PTR dwContext, DWORD dwInternetStatus, LPVOID lpvStatusInformation, DWORD dwStatusInformationLength) + { + UNREFERENCED_PARAMETER(hInternet); + + WinHttpCallbackContext* context = reinterpret_cast(dwContext); + if (context == nullptr) + { + return; + } + + if (dwInternetStatus == WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING) + { + // Documented as the final callback for this handle, so WinHTTP no + // longer references anything this request handed it. Release the + // context -- and with it the strong reference that has been keeping + // the wrapper (and its read buffer) alive -- but only after using it + // as the backstop that guarantees every request produces exactly one + // terminal response, including the cancellation paths that + // deliberately do not complete inline. + std::shared_ptr self = context->request; + delete context; + if (self != nullptr && !self->isCallbackCalled) + { + self->onRequestComplete(self->m_deferredError.exchange(ERROR_SUCCESS)); + } + return; + } + + std::shared_ptr self = context->request; + if (self == nullptr || self->isCallbackCalled) + { + // The terminal response has already been delivered; the request is + // no longer tracked by the client, which may since have been torn + // down. Nothing here may touch it again. + return; + } + + LOG_TRACE("winHttpCallback: hInternet %p, self %p, dwInternetStatus %u", hInternet, self.get(), dwInternetStatus); + + switch (dwInternetStatus) + { + case WINHTTP_CALLBACK_STATUS_SENDING_REQUEST: + // TLS is negotiated, but the request headers have not left the + // process. Enforce the configured Microsoft-root policy here so + // API keys and auth tickets are never disclosed to a server that + // only passes the platform's broader certificate policy. + if (self->m_isHttps && self->m_msRootCheckRequired && + !self->m_msRootCheckCompleted.exchange(true)) + { + DWORD dwError = self->validateCurrentRequestMsRootCert(); + if (dwError != ERROR_SUCCESS) + { + // WinHTTP permits closing a handle from its own status + // callback even while WinHttpSendRequest is active. Do + // that here so rejected credentials never leave the + // process; external cancellation uses the deferred path. + self->abortRequest(dwError, true); + } + } + return; + + case WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE: + self->schedule(self->m_request->m_body.empty() + ? NextOperation::ReceiveResponse + : NextOperation::WriteBody); + return; + + case WINHTTP_CALLBACK_STATUS_WRITE_COMPLETE: + { + // WinHTTP has released the caller's body buffer for the bytes it + // reports here. Short writes are not expected, but honour them + // rather than truncating the payload. + DWORD written = (lpvStatusInformation != nullptr) + ? *static_cast(lpvStatusInformation) : 0; + self->m_bodyWritten += written; + if (self->m_bodyWritten < self->m_request->m_body.size()) + { + if (written == 0) + { + self->schedule(NextOperation::Complete, ERROR_WINHTTP_CONNECTION_ERROR); + return; + } + self->schedule(NextOperation::WriteBody); + return; + } + self->schedule(NextOperation::ReceiveResponse); + return; + } + + case WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE: + // The certificate policy was already enforced before the + // request headers were transmitted. + self->schedule(NextOperation::QueryDataAvailable); + return; + + case WINHTTP_CALLBACK_STATUS_DATA_AVAILABLE: + { + DWORD bytesAvailable = (lpvStatusInformation != nullptr) + ? *static_cast(lpvStatusInformation) : 0; + if (bytesAvailable == 0) + { + // No more data: response is complete. + self->schedule(NextOperation::Complete, ERROR_SUCCESS); + return; + } + // SECURITY: refuse an over-large response instead of buffering it + // (see MAX_HTTP_RESPONSE_SIZE) so a hostile/MITM'd collector cannot + // exhaust process memory. Checked before every read so the buffer + // never exceeds the cap; reported as an invalid server response -> + // NetworkFailure (retried). + if (self->m_bodyBuffer.size() > MAX_HTTP_RESPONSE_SIZE || + bytesAvailable > MAX_HTTP_RESPONSE_SIZE - self->m_bodyBuffer.size()) + { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + self->schedule(NextOperation::Complete, ERROR_WINHTTP_INVALID_SERVER_RESPONSE); + return; + } + // readData() takes whatever fits in the fixed buffer; anything + // beyond that is reported again by the next QueryDataAvailable. + self->schedule(NextOperation::ReadData); + return; + } + + case WINHTTP_CALLBACK_STATUS_READ_COMPLETE: + // dwStatusInformationLength is the number of bytes actually placed + // into the buffer passed to WinHttpReadData (may be less than the + // buffer size that was offered). + if (dwStatusInformationLength > sizeof(self->m_readBuffer) || + self->m_bodyBuffer.size() > MAX_HTTP_RESPONSE_SIZE || + dwStatusInformationLength > MAX_HTTP_RESPONSE_SIZE - self->m_bodyBuffer.size()) + { + self->schedule(NextOperation::Complete, ERROR_WINHTTP_INVALID_SERVER_RESPONSE); + return; + } + self->m_bodyBuffer.insert(self->m_bodyBuffer.end(), + self->m_readBuffer, self->m_readBuffer + dwStatusInformationLength); + self->schedule(NextOperation::QueryDataAvailable); + return; + + case WINHTTP_CALLBACK_STATUS_REQUEST_ERROR: + { + DWORD dwError = ERROR_WINHTTP_INTERNAL_ERROR; + if (lpvStatusInformation != nullptr && + dwStatusInformationLength >= sizeof(WINHTTP_ASYNC_RESULT)) + { + dwError = static_cast(lpvStatusInformation)->dwError; + } + // The operation that owned the buffers WinHTTP was given has + // finished failing, so the response may be handed back now. A + // locally recorded abort reason wins over WinHTTP's generic + // "operation cancelled". + DWORD deferred = self->m_deferredError.exchange(ERROR_SUCCESS); + self->schedule(NextOperation::Complete, (deferred != ERROR_SUCCESS) ? deferred : dwError); + return; + } + + default: + return; + } + } + + void onRequestComplete(DWORD dwError) + { + { + std::lock_guard lock(m_clientState->requestsMutex); + if (m_stateCallbackDepth != 0) + { + m_stateCompletionPending = true; + m_stateCompletionError = dwError; + return; + } + if (isCallbackCalled.exchange(true)) + { + return; + } + } + + std::unique_ptr response(new SimpleHttpResponse(m_id)); + // Closing the request handle below releases WinHTTP's callback context, + // and that context holds the strong reference that has been keeping + // this object alive. Hold one here so the rest of this method -- and + // the application callback it invokes -- cannot run on a freed object. + auto keepAlive = shared_from_this(); + HINTERNET request = getRequestHandle(); + if (dwError == ERROR_SUCCESS && request == nullptr) + { + dwError = ERROR_WINHTTP_OPERATION_CANCELLED; + } + bool const receivedResponse = dwError == ERROR_SUCCESS; + + if (dwError == ERROR_SUCCESS) { + response->m_body = m_bodyBuffer; + response->m_result = HttpResult_OK; + + DWORD statusCode = 0; + DWORD dwSize = sizeof(statusCode); + if (!::WinHttpQueryHeaders(request, WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER, + WINHTTP_HEADER_NAME_BY_INDEX, &statusCode, &dwSize, WINHTTP_NO_HEADER_INDEX)) + { + LOG_WARN("WinHttpQueryHeaders(STATUS_CODE) failed: %d", ::GetLastError()); + response->m_result = HttpResult_NetworkFailure; + } + response->m_statusCode = statusCode; + + // Raw headers, as "Name: Value\r\n..." pairs -- the same shape WinInet + // hands back via HTTP_QUERY_RAW_HEADERS_CRLF. + DWORD headerBytes = 0; + BOOL headersQueried = ::WinHttpQueryHeaders( + request, WINHTTP_QUERY_RAW_HEADERS_CRLF, + WINHTTP_HEADER_NAME_BY_INDEX, WINHTTP_NO_OUTPUT_BUFFER, &headerBytes, + WINHTTP_NO_HEADER_INDEX); + DWORD headerErr = headersQueried ? ERROR_SUCCESS : ::GetLastError(); + if (!headersQueried && headerErr == ERROR_INSUFFICIENT_BUFFER && headerBytes > 0) + { + if (headerBytes % sizeof(wchar_t) != 0) + { + LOG_WARN("WinHttpQueryHeaders(RAW_HEADERS_CRLF) returned an invalid byte count: %lu", headerBytes); + } + else + { + std::wstring wHeaders(headerBytes / sizeof(wchar_t), L'\0'); + DWORD bufferBytes = headerBytes; + if (::WinHttpQueryHeaders( + request, WINHTTP_QUERY_RAW_HEADERS_CRLF, + WINHTTP_HEADER_NAME_BY_INDEX, &wHeaders[0], &bufferBytes, + WINHTTP_NO_HEADER_INDEX)) + { + // WinHttpQueryHeaders includes the buffer's trailing NUL(s) in + // the byte count; trim at the first one before converting. + size_t nul = wHeaders.find(L'\0'); + if (nul != std::wstring::npos) + { + wHeaders.resize(nul); + } + parseHeaders(to_utf8_string(wHeaders), *response); + } + else + { + LOG_WARN("WinHttpQueryHeaders(RAW_HEADERS_CRLF) failed twice: %d", ::GetLastError()); + } + } + } + else if (!headersQueried) + { + LOG_WARN("WinHttpQueryHeaders(RAW_HEADERS_CRLF) failed: %d", headerErr); + } + } else { + switch (dwError) { + case ERROR_WINHTTP_OPERATION_CANCELLED: + response->m_result = HttpResult_Aborted; + break; + + case ERROR_WINHTTP_TIMEOUT: + case ERROR_WINHTTP_NAME_NOT_RESOLVED: + case ERROR_WINHTTP_CANNOT_CONNECT: + case ERROR_WINHTTP_CONNECTION_ERROR: + case ERROR_WINHTTP_RESEND_REQUEST: + case ERROR_WINHTTP_SECURE_CERT_DATE_INVALID: + case ERROR_WINHTTP_SECURE_CERT_CN_INVALID: + case ERROR_WINHTTP_CLIENT_AUTH_CERT_NEEDED: + case ERROR_WINHTTP_SECURE_INVALID_CA: + case ERROR_WINHTTP_SECURE_CERT_REV_FAILED: + case ERROR_WINHTTP_SECURE_CHANNEL_ERROR: + case ERROR_WINHTTP_SECURE_INVALID_CERT: + case ERROR_WINHTTP_SECURE_CERT_REVOKED: + case ERROR_WINHTTP_SECURE_CERT_WRONG_USAGE: + case ERROR_WINHTTP_SECURE_FAILURE: + case ERROR_WINHTTP_REDIRECT_FAILED: + case ERROR_WINHTTP_INVALID_SERVER_RESPONSE: + case ERROR_WINHTTP_RESPONSE_DRAIN_OVERFLOW: + response->m_result = HttpResult_NetworkFailure; + break; + + default: + response->m_result = HttpResult_LocalFailure; + break; + } + } + + { + auto state = m_clientState; + WinHttpCallbackScope callbackScope(state); + auto callback = m_appCallback; + auto requestId = m_id; + // Let go of the request handle before entering application code: + // OnHttpResponse() is what allows the caller to destroy the request + // object whose body buffer WinHTTP was given, so WinHTTP must be + // done with this request first. Closing it is also what triggers + // HANDLE_CLOSING, which releases the callback context. + closeRequestHandle(); + // Remove the request before entering application code. The callback + // can synchronously tear down the client and destroy this wrapper. + state->eraseRequest(requestId); + if (callback != nullptr) + { + // The implementation-specific handle is no longer valid once + // terminal delivery begins, so do not expose a stale handle. + if (receivedResponse) + { + callback->OnHttpStateEvent(OnResponse, nullptr, 0); + } + callback->OnHttpResponse(response.release()); + } + } + } + + private: + // Parses "Name: Value\r\n"-formatted raw headers (as returned by + // WINHTTP_QUERY_RAW_HEADERS_CRLF / HTTP_QUERY_RAW_HEADERS_CRLF) into an + // HttpHeaders map. Shared shape with HttpClient_WinInet's inline parser. + static void parseHeaders(std::string const& raw, SimpleHttpResponse& response) + { + size_t lineStart = 0; + while (lineStart < raw.size()) { + size_t lineEnd = raw.find("\r\n", lineStart); + if (lineEnd == std::string::npos) { + lineEnd = raw.size(); + } + + const std::string line = raw.substr(lineStart, lineEnd - lineStart); + const size_t colon = line.find(':'); + if (colon != std::string::npos) { + size_t valueStart = colon + 1; + while (valueStart < line.size() && line[valueStart] == ' ') { + ++valueStart; + } + response.m_headers.add(line.substr(0, colon), line.substr(valueStart)); + } + + if (lineEnd == raw.size()) { + break; + } + lineStart = lineEnd + 2; + } + } +}; + +//--- + +WinHttpClientState::WinHttpClientState(HINTERNET sessionHandle) : + session(sessionHandle) +{ +} + +WinHttpClientState::~WinHttpClientState() +{ + if (session != nullptr) + { + ::WinHttpCloseHandle(session); + } +} + +bool WinHttpClientState::registerRequest( + std::string const& id, + std::shared_ptr request) +{ + std::lock_guard lock(requestsMutex); + if (!acceptingRequests) + { + return false; + } + requests[id] = std::move(request); + ++registryGeneration; + bool const shouldSend = cancelAllDepth == 0; + requestsCv.notify_all(); + return shouldSend; +} + +void WinHttpClientState::eraseRequest(std::string const& id) +{ + std::lock_guard lock(requestsMutex); + requests.erase(id); + ++registryGeneration; + requestsCv.notify_all(); +} + +void WinHttpClientState::stopAcceptingRequests() +{ + std::lock_guard lock(requestsMutex); + acceptingRequests = false; +} + +void WinHttpClientState::beginCallback() +{ + std::lock_guard lock(requestsMutex); + beginCallbackLocked(); + requestsCv.notify_all(); +} + +void WinHttpClientState::beginCallbackLocked() +{ + ++callbacksInFlight; + ++callbacksByThread[std::this_thread::get_id()]; + ++callbackGeneration; +} + +void WinHttpClientState::endCallback() +{ + std::lock_guard lock(requestsMutex); + auto it = callbacksByThread.find(std::this_thread::get_id()); + if (callbacksInFlight == 0) + { + LOG_ERROR("WinHTTP callback accounting underflow"); + requestsCv.notify_all(); + return; + } + + --callbacksInFlight; + if (it == callbacksByThread.end() || it->second == 0) + { + LOG_ERROR("WinHTTP callback thread was not registered"); + } + else if (--it->second == 0) + { + callbacksByThread.erase(it); + } + ++callbackGeneration; + requestsCv.notify_all(); +} + +unsigned HttpClient_WinHttp::s_nextRequestId = 0; + +HttpClient_WinHttp::HttpClient_WinHttp() +{ + // WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY (Windows 8.1+) resolves the proxy + // without depending on a logged-on interactive user or that user's + // Internet Explorer settings -- unlike WinInet's + // INTERNET_OPEN_TYPE_PRECONFIG, which requires one. This is why WinHTTP, + // not WinInet, is Microsoft's documented recommendation for services and + // other non-interactive processes. On an older OS that rejects this access + // type, fall back to the machine-wide WinHTTP proxy configuration. This is + // the documented pre-Windows-8.1 behavior and avoids bypassing enterprise + // proxies entirely. Only fall back for the compatibility error; other + // failures should not be hidden by a second, unrelated WinHttpOpen call. + HINTERNET session = ::WinHttpOpen( + NULL, WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY, + WINHTTP_NO_PROXY_NAME, WINHTTP_NO_PROXY_BYPASS, WINHTTP_FLAG_ASYNC); + if (session == nullptr) + { + DWORD dwError = ::GetLastError(); + if (dwError == ERROR_INVALID_PARAMETER) + { + LOG_WARN("WinHttpOpen(AUTOMATIC_PROXY) is unsupported; retrying with default proxy"); + session = ::WinHttpOpen( + NULL, WINHTTP_ACCESS_TYPE_DEFAULT_PROXY, + WINHTTP_NO_PROXY_NAME, WINHTTP_NO_PROXY_BYPASS, WINHTTP_FLAG_ASYNC); + } + else + { + LOG_WARN("WinHttpOpen(AUTOMATIC_PROXY) failed: %lu", dwError); + } + } + // WinHTTP otherwise permits an unlimited number of connections per origin. + // Keep transport concurrency aligned with the SDK's default pending-upload + // limit until ApplySettings supplies the configured value. + setConnectionLimits(session, DEFAULT_MAX_CONNECTIONS_PER_SERVER); + m_state = std::make_shared(session); +} + +HttpClient_WinHttp::~HttpClient_WinHttp() +{ + m_state->stopAcceptingRequests(); + CancelAllRequests(); + m_state.reset(); +} + +IHttpRequest* HttpClient_WinHttp::CreateRequest() +{ + std::string id = "WH-" + toString(::InterlockedIncrement(&s_nextRequestId)); + return new SimpleHttpRequest(id); +} + +void HttpClient_WinHttp::SendRequestAsync(IHttpRequest* request, IHttpResponseCallback* callback) +{ + // Note: 'request' is never owned by IHttpClient and gets deleted in EventsUploadContext.clear() + auto state = m_state; + auto wrapper = std::make_shared( + std::move(state), static_cast(request)); + wrapper->send(callback); +} + +void HttpClient_WinHttp::CancelRequestAsync(std::string const& id) +{ + auto state = m_state; + // Copy the shared_ptr out of the map while holding the lock only for the + // lookup, then call cancel() without the lock held (cancel() blocks in + // WinHttpCloseHandle waiting for a completion callback on another thread + // that needs this same lock -- see cancel()'s comment). The local copy + // keeps the wrapper alive for the duration of this call even if erase() + // concurrently removes the map's own reference. + std::shared_ptr request; + { + std::lock_guard lock(state->requestsMutex); + auto it = state->requests.find(id); + if (it != state->requests.end()) { + request = it->second; + } + } + if (request) { + request->cancel(); + } +} + +void HttpClient_WinHttp::CancelAllRequests() +{ + CancelAllRequests(std::chrono::milliseconds::zero()); +} + +void HttpClient_WinHttp::CancelAllRequests(std::chrono::milliseconds bestEffortTimeout) +{ + auto state = m_state; + class CancelAllScope + { + public: + explicit CancelAllScope(std::shared_ptr state) + : m_state(std::move(state)) + { + std::lock_guard lock(m_state->requestsMutex); + ++m_state->cancelAllDepth; + } + + ~CancelAllScope() + { + if (m_active) + { + std::lock_guard lock(m_state->requestsMutex); + --m_state->cancelAllDepth; + } + } + + void finishLocked() + { + --m_state->cancelAllDepth; + m_active = false; + } + + private: + std::shared_ptr m_state; + bool m_active {true}; + } cancelAllScope(state); + + bool const hasTimeout = + bestEffortTimeout > std::chrono::milliseconds::zero(); + auto const deadline = + std::chrono::steady_clock::now() + bestEffortTimeout; + std::thread::id const callerThread = std::this_thread::get_id(); + auto callbacksDrainedForCaller = [&state, callerThread]() { + // Application callbacks cannot wait for peer callbacks: simultaneous + // callbacks doing so would wait on one another. Each callback scope + // retains the shared client state independently. + return state->callbacksByThread.find(callerThread) != + state->callbacksByThread.end() || + state->callbacksInFlight == 0; + }; + auto requestsDrainedForCaller = [&state, callerThread]() { + if (state->requests.empty()) + { + return true; + } + + bool callerIsInStateCallback = false; + for (auto const& item : state->requests) + { + if (item.second->hasStateCallbackOnThreadLocked(callerThread)) + { + callerIsInStateCallback = true; + break; + } + } + for (auto const& item : state->requests) + { + if (!callerIsInStateCallback || + !item.second->hasActiveStateCallbackLocked()) + { + return false; + } + } + return true; + }; + + for (;;) + { + std::vector> requests; + size_t registryGeneration; + size_t callbackGeneration; + { + std::lock_guard lock(state->requestsMutex); + if (state->requests.empty() && callbacksDrainedForCaller()) + { + // Holding the registry lock makes completion of this cancellation + // epoch the linearization point: later registrations are new work. + cancelAllScope.finishLocked(); + return; + } + + registryGeneration = state->registryGeneration; + callbackGeneration = state->callbackGeneration; + for (auto const& item : state->requests) + { + requests.push_back(item.second); + } + } + + for (auto const& request : requests) + { + if (hasTimeout && std::chrono::steady_clock::now() >= deadline) + { + break; + } + request->cancel(); + } + + std::unique_lock lock(state->requestsMutex); + if (requestsDrainedForCaller() && callbacksDrainedForCaller()) + { + cancelAllScope.finishLocked(); + return; + } + auto stateChangedOrDrained = [&]() { + return state->registryGeneration != registryGeneration || + state->callbackGeneration != callbackGeneration || + (requestsDrainedForCaller() && callbacksDrainedForCaller()); + }; + if (hasTimeout) + { + if (!state->requestsCv.wait_until( + lock, deadline, stateChangedOrDrained)) + { + return; + } + } + else + { + state->requestsCv.wait(lock, stateChangedOrDrained); + } + } +} + +/// +/// Enforces MS-root server certificate check. +/// +/// if set to true [enforce verification that server cert is MS-Rooted]. +void HttpClient_WinHttp::ApplySettings(ILogConfiguration& config) +{ + int64_t configuredMaxConnections = config[CFG_INT_MAX_PENDING_REQ]; + DWORD maxConnections = DEFAULT_MAX_CONNECTIONS_PER_SERVER; + if (configuredMaxConnections > 0) + { + auto const largestFiniteLimit = + static_cast(std::numeric_limits::max() - 1); + maxConnections = static_cast( + configuredMaxConnections > largestFiniteLimit + ? largestFiniteLimit + : configuredMaxConnections); + } + setConnectionLimits(m_state->session, maxConnections); + SetMsRootCheck(config[CFG_MAP_HTTP][CFG_BOOL_HTTP_MS_ROOT_CHECK]); +} + +void HttpClient_WinHttp::SetMsRootCheck(bool enforceMsRoot) +{ + m_state->msRootCheck.store(enforceMsRoot, std::memory_order_release); +} + +/// +/// Determines whether MS-Rooted server cert check required. +/// +/// +/// true if [MS-Rooted server cert check required]; otherwise, false. +/// +bool HttpClient_WinHttp::IsMsRootCheckRequired() +{ + return m_state->msRootCheck.load(std::memory_order_acquire); +} + +} MAT_NS_END +#endif // HAVE_MAT_DEFAULT_HTTP_CLIENT +// clang-format on diff --git a/lib/http/HttpClient_WinHttp.hpp b/lib/http/HttpClient_WinHttp.hpp new file mode 100644 index 000000000..b95cdfcbb --- /dev/null +++ b/lib/http/HttpClient_WinHttp.hpp @@ -0,0 +1,65 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#ifndef HTTPCLIENT_WINHTTP_HPP +#define HTTPCLIENT_WINHTTP_HPP + +#ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT + +#include "IHttpClient.hpp" +#include "IBoundedHttpClientCancel.hpp" +#include "pal/PAL.hpp" + +#include "ILogManager.hpp" + +#include +#include +#include +#include + +namespace MAT_NS_BEGIN { + +#ifndef _WINHTTPX_ +typedef void* HINTERNET; +#endif + +class WinHttpRequestWrapper; +struct WinHttpClientState; + +// WinHTTP-based HTTP client. Unlike WinInet, WinHTTP does not depend on a +// logged-on interactive user or that user's Internet Explorer settings, so +// it is Microsoft's recommended transport for services and other +// non-interactive processes (see +// https://learn.microsoft.com/windows/win32/winhttp/porting-wininet-applications-to-winhttp). +// This is the default Win32 desktop transport; HttpClient_WinInet remains +// available as an explicit opt-in for callers that need IE-integrated proxy +// or cookie behavior. +class HttpClient_WinHttp : public IHttpClient, public IBoundedHttpClientCancel { + public: + // Common IHttpClient methods + HttpClient_WinHttp(); + virtual ~HttpClient_WinHttp(); + virtual IHttpRequest* CreateRequest() final; + virtual void SendRequestAsync(IHttpRequest* request, IHttpResponseCallback* callback) final; + virtual void CancelRequestAsync(std::string const& id) final; + virtual void CancelAllRequests() final; + virtual void CancelAllRequests(std::chrono::milliseconds bestEffortTimeout) final; + + virtual void ApplySettings(ILogConfiguration& config) override; + + // Methods unique to WinHttp implementation. + void SetMsRootCheck(bool enforceMsRoot); + bool IsMsRootCheckRequired(); + + protected: + std::shared_ptr m_state; + static unsigned s_nextRequestId; + friend class WinHttpRequestWrapper; +}; + +} MAT_NS_END + +#endif // HAVE_MAT_DEFAULT_HTTP_CLIENT + +#endif // HTTPCLIENT_WINHTTP_HPP diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index 2ec8be9b0..1a584ac4d 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -6,28 +6,98 @@ #include "mat/config.h" #ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT -#pragma warning(push) -#pragma warning(disable:4189) /* Turn off Level 4: local variable is initialized but not referenced. dwError unused in Release without printing it. */ #include "HttpClient_WinInet.hpp" #include "utils/StringUtils.hpp" #include #include -#include +#include +#include +#include #include #include +#include +#include #include #include +#pragma comment(lib, "crypt32.lib") +#pragma comment(lib, "wininet.lib") + namespace MAT_NS_BEGIN { -class WinInetRequestWrapper +class WinInetRequestWrapper; + +struct WinInetCallbackContext +{ + explicit WinInetCallbackContext(std::shared_ptr request) + : request(std::move(request)) + { + } + + std::shared_ptr request; +}; + +struct WinInetClientState +{ + explicit WinInetClientState(HINTERNET internetHandle); + ~WinInetClientState(); + + bool registerRequest( + std::string const& id, + std::shared_ptr request); + void eraseRequest(std::string const& id); + void stopAcceptingRequests(); + void beginCallback(); + void endCallback(); + + HINTERNET internet; + std::mutex requestsMutex; + std::map> requests; + std::condition_variable requestsCv; + std::atomic msRootCheck {false}; + bool acceptingRequests {true}; + size_t cancelAllDepth {0}; + size_t registryGeneration {0}; + size_t callbackGeneration {0}; + size_t callbacksInFlight {0}; + std::map callbacksByThread; +}; + +class WinInetCallbackScope +{ + public: + explicit WinInetCallbackScope( + std::shared_ptr state) + : m_state(std::move(state)) + { + m_state->beginCallback(); + } + + ~WinInetCallbackScope() + { + m_state->endCallback(); + } + + WinInetCallbackScope(WinInetCallbackScope const&) = delete; + WinInetCallbackScope& operator=(WinInetCallbackScope const&) = delete; + + private: + std::shared_ptr m_state; +}; + +class WinInetRequestWrapper : public std::enable_shared_from_this { protected: - HttpClient_WinInet& m_parent; + std::shared_ptr m_clientState; std::string m_id; IHttpResponseCallback* m_appCallback {nullptr}; + // WinInet may deliver completion callbacks synchronously from an async API. + // This per-request recursive mutex permits only that narrow re-entry. It is + // never nested with the parent request-map mutex; cancellation snapshots + // the registry before touching request handles or invoking application code. + std::recursive_mutex m_handleMutex; HINTERNET m_hWinInetSession {nullptr}; HINTERNET m_hWinInetRequest {nullptr}; SimpleHttpRequest* m_request; @@ -35,11 +105,108 @@ class WinInetRequestWrapper DWORD m_bufferUsed {0}; std::vector m_bodyBuffer; bool m_readingData {false}; - bool isCallbackCalled {false}; - bool isAborted {false}; + std::atomic m_terminalCallbackStarted {false}; + std::atomic m_isAborted {false}; + std::atomic m_deferredError {ERROR_SUCCESS}; + bool m_msRootCheckRequired {false}; + bool m_contextInstalled {false}; + bool m_sendIssued {false}; + bool m_setupActive {false}; + unsigned m_stateCallbackDepth {0}; + std::map m_stateCallbacksByThread; + bool m_setupCompletionPending {false}; + DWORD m_setupCompletionError {ERROR_SUCCESS}; + unsigned m_asyncApiDepth {0}; + bool m_apiCompletionPending {false}; + DWORD m_apiCompletionError {ERROR_SUCCESS}; + + class SetupGuard + { + public: + explicit SetupGuard(WinInetRequestWrapper& owner) noexcept + : m_owner(owner) + { + std::lock_guard lock(m_owner.m_handleMutex); + m_owner.m_setupActive = true; + } + + ~SetupGuard() noexcept(false) + { + m_owner.finishSetup(); + } + + SetupGuard(SetupGuard const&) = delete; + SetupGuard& operator=(SetupGuard const&) = delete; + + private: + WinInetRequestWrapper& m_owner; + }; + + void finishSetup() + { + bool complete = false; + DWORD completionError = ERROR_SUCCESS; + { + std::lock_guard lock(m_handleMutex); + m_setupActive = false; + complete = m_setupCompletionPending; + completionError = m_setupCompletionError; + m_setupCompletionPending = false; + m_setupCompletionError = ERROR_SUCCESS; + } + if (complete) + { + onRequestComplete(completionError); + } + } + + HINTERNET detachRequestHandle() + { + std::lock_guard lock(m_handleMutex); + HINTERNET request = m_hWinInetRequest; + m_hWinInetRequest = nullptr; + return request; + } + + HINTERNET detachSessionHandle() + { + std::lock_guard lock(m_handleMutex); + HINTERNET session = m_hWinInetSession; + m_hWinInetSession = nullptr; + return session; + } + + void closeRequestHandle() + { + HINTERNET request = detachRequestHandle(); + if (request != nullptr) + { + // InternetCloseHandle may synchronously deliver HANDLE_CLOSING. + // Never hold either mutex while closing. + ::InternetCloseHandle(request); + } + } + + void closeSessionHandle() + { + HINTERNET session = detachSessionHandle(); + if (session != nullptr) + { + ::InternetCloseHandle(session); + } + } + + bool shouldStopSetup() const noexcept + { + return m_isAborted.load(std::memory_order_acquire) || + m_terminalCallbackStarted.load(std::memory_order_acquire); + } + public: - WinInetRequestWrapper(HttpClient_WinInet& parent, SimpleHttpRequest* request) - : m_parent(parent), + WinInetRequestWrapper( + std::shared_ptr clientState, + SimpleHttpRequest* request) + : m_clientState(std::move(clientState)), m_id(request->GetId()), m_request(request) { @@ -49,14 +216,24 @@ class WinInetRequestWrapper WinInetRequestWrapper(WinInetRequestWrapper const&) = delete; WinInetRequestWrapper& operator=(WinInetRequestWrapper const&) = delete; + bool hasStateCallbackOnThread(std::thread::id threadId) + { + std::lock_guard lock(m_handleMutex); + return m_stateCallbacksByThread.find(threadId) != + m_stateCallbacksByThread.end(); + } + + bool hasActiveStateCallback() + { + std::lock_guard lock(m_handleMutex); + return m_stateCallbackDepth != 0; + } + ~WinInetRequestWrapper() noexcept { LOG_TRACE("%p ~WinInetRequestWrapper()", this); - if (m_hWinInetRequest != nullptr) - { - ::InternetCloseHandle(m_hWinInetRequest); - ::InternetCloseHandle(m_hWinInetSession); - } + closeRequestHandle(); + closeSessionHandle(); } /// @@ -64,14 +241,10 @@ class WinInetRequestWrapper /// the object destructor, but rather hints the implementation to speed-up the /// destruction. /// - /// Two possible outcomes:. - //// - /// - set isAborted to true: cancel request without sending to WinInet stack, - /// in case if request has not been sent to WinInet stack yet. - //// - /// - close m_hWinInetRequest handle: WinInet fails all subsequent attempts to - /// use invalidated handle and aborts all pending WinInet worker threads on it. - /// In that case we complete with ERROR_INTERNET_OPERATION_CANCELLED. + /// Cancellation marks setup as aborted and closes an existing request handle. + /// Before the asynchronous send starts, completion can be delivered directly. + /// After it starts, completion is deferred until REQUEST_COMPLETE or + /// HANDLE_CLOSING proves that WinInet has released the caller's body buffer. /// /// It may happen that we get some feedback from WinInet, i.e. we are canceling /// at that same moment when the request is complete. In that case we process @@ -79,12 +252,36 @@ class WinInetRequestWrapper /// void cancel() { - LOCKGUARD(m_parent.m_requestsMutex); - isAborted = true; - if (m_hWinInetRequest != nullptr) + HINTERNET request = nullptr; + bool completeHere = false; + { + std::lock_guard lock(m_handleMutex); + if (m_terminalCallbackStarted.load(std::memory_order_acquire)) + { + return; + } + m_isAborted.store(true, std::memory_order_release); + DWORD noError = ERROR_SUCCESS; + m_deferredError.compare_exchange_strong( + noError, ERROR_INTERNET_OPERATION_CANCELLED, std::memory_order_acq_rel); + request = m_hWinInetRequest; + m_hWinInetRequest = nullptr; + // Before an async send is issued, WinInet owns none of the request + // body's storage and no REQUEST_COMPLETE callback is guaranteed. + completeHere = + m_stateCallbackDepth == 0 && + !m_setupActive && + (!m_contextInstalled || !m_sendIssued); + } + if (request != nullptr) { - ::InternetCloseHandle(m_hWinInetRequest); - // async request callback destroys the object + // WinInet may invoke callbacks here. The callback context retains + // this wrapper until HANDLE_CLOSING. + ::InternetCloseHandle(request); + } + if (completeHere) + { + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); } } @@ -93,6 +290,11 @@ class WinInetRequestWrapper */ bool isMsRootCert() { + std::lock_guard lock(m_handleMutex); + if (m_hWinInetRequest == nullptr) + { + return false; + } // Pointer to certificate chain obtained via InternetQueryOption : // Ref. https://blogs.msdn.microsoft.com/alejacma/2012/01/18/how-to-use-internet_option_server_cert_chain_context-with-internetqueryoption-in-c/ PCCERT_CHAIN_CONTEXT pCertCtx = nullptr; @@ -137,44 +339,43 @@ class WinInetRequestWrapper } // Asynchronously send HTTP request and invoke response callback. - // Ownership semantics: send(...) method self-destroys *this* upon - // receiving WinInet callback. There must be absolutely no methods - // that attempt to use the object after triggering send on it. - // Send operation on request may be issued no more than once. - // - // Implementation details: - // - // lockguard around m_requestsMutex covers the following stages: - // - request added to map - // - URL parsed - // - DNS lookup performed, socket opened, SSL handshake - // - MS-Root SSL cert validation (if requested) - // - populating HTTP request headers - // - scheduling async(!) upload of HTTP post body - // - // Note that if any of the stages above fails, we invoke onRequestComplete(...). - // That method destroys "this" request object and in order to avoid - // any corruption we immediately return after invoking onRequestComplete(...). - // + // The request map owns the wrapper during setup, and the callback context + // retains it after a WinInet request handle is created. Send may be issued + // only once. void send(IHttpResponseCallback* callback) { - LOCKGUARD(m_parent.m_requestsMutex); - // Register app callback and request in HttpClient map + SetupGuard setupGuard(*this); m_appCallback = callback; - m_parent.m_requests[m_id] = this; + m_msRootCheckRequired = + m_clientState->msRootCheck.load(std::memory_order_acquire); + if (!m_clientState->registerRequest(m_id, shared_from_this())) + { + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } - // If outside code asked us to abort that request before we could proceed with - // creating a WinInet handle, then clean it right away before proceeding with - // any async WinInet API calls. - if (isAborted) + if (shouldStopSetup()) { - // Request force-aborted before creating a WinInet handle. DispatchEvent(OnConnectFailed); onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } DispatchEvent(OnConnecting); + if (shouldStopSetup()) + { + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } + + if (m_request->m_url.size() > static_cast(std::numeric_limits::max())) + { + LOG_WARN("Request URL exceeds WinInet's maximum size"); + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_INVALID_PARAMETER); + return; + } + URL_COMPONENTSA urlc; memset(&urlc, 0, sizeof(urlc)); urlc.dwStructSize = sizeof(urlc); @@ -184,123 +385,262 @@ class WinInetRequestWrapper char path[1024] = { 0 }; urlc.lpszUrlPath = path; urlc.dwUrlPathLength = sizeof(path); - if (!::InternetCrackUrlA(m_request->m_url.data(), (DWORD)m_request->m_url.size(), 0, &urlc)) + if (!::InternetCrackUrlA( + m_request->m_url.c_str(), static_cast(m_request->m_url.size()), 0, &urlc)) { DWORD dwError = ::GetLastError(); - LOG_WARN("InternetCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.data()); - // Invalid URL passed to WinInet API + LOG_WARN("InternetCrackUrl() failed: dwError=%d url=%s", dwError, m_request->m_url.c_str()); DispatchEvent(OnConnectFailed); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + onRequestComplete(dwError); return; } - m_hWinInetSession = ::InternetConnectA(m_parent.m_hInternet, hostname, urlc.nPort, - NULL, NULL, INTERNET_SERVICE_HTTP, 0, reinterpret_cast(this)); - if (m_hWinInetSession == NULL) { - DWORD dwError = ::GetLastError(); + DWORD dwError = ERROR_SUCCESS; + { + std::lock_guard lock(m_handleMutex); + if (shouldStopSetup()) + { + dwError = ERROR_INTERNET_OPERATION_CANCELLED; + } + else + { + m_hWinInetSession = ::InternetConnectA( + m_clientState->internet, hostname, urlc.nPort, + NULL, NULL, INTERNET_SERVICE_HTTP, 0, 0); + if (m_hWinInetSession == nullptr) + { + dwError = ::GetLastError(); + } + } + } + if (dwError != ERROR_SUCCESS) + { LOG_WARN("InternetConnect() failed: %d", dwError); - // Cannot connect to host DispatchEvent(OnConnectFailed); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + onRequestComplete(dwError); return; } // TODO: Session handle for the same target should be cached across requests to enable keep-alive. PCSTR szAcceptTypes[] = {"*/*", NULL}; - m_hWinInetRequest = ::HttpOpenRequestA( - m_hWinInetSession, m_request->m_method.c_str(), path, NULL, NULL, szAcceptTypes, - INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_NO_AUTH | INTERNET_FLAG_NO_CACHE_WRITE | - INTERNET_FLAG_NO_COOKIES | INTERNET_FLAG_NO_UI | INTERNET_FLAG_PRAGMA_NOCACHE | - INTERNET_FLAG_RELOAD | (urlc.nScheme == INTERNET_SCHEME_HTTPS ? INTERNET_FLAG_SECURE : 0), - reinterpret_cast(this)); - if (m_hWinInetRequest == NULL) { - DWORD dwError = ::GetLastError(); + { + std::unique_ptr context( + new WinInetCallbackContext(shared_from_this())); + std::lock_guard lock(m_handleMutex); + if (shouldStopSetup()) + { + dwError = ERROR_INTERNET_OPERATION_CANCELLED; + } + else + { + m_hWinInetRequest = ::HttpOpenRequestA( + m_hWinInetSession, m_request->m_method.c_str(), path, NULL, NULL, szAcceptTypes, + INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_NO_AUTH | INTERNET_FLAG_NO_CACHE_WRITE | + INTERNET_FLAG_NO_COOKIES | INTERNET_FLAG_NO_UI | INTERNET_FLAG_PRAGMA_NOCACHE | + INTERNET_FLAG_RELOAD | + (m_msRootCheckRequired ? INTERNET_FLAG_NO_AUTO_REDIRECT : 0) | + (urlc.nScheme == INTERNET_SCHEME_HTTPS ? INTERNET_FLAG_SECURE : 0), + reinterpret_cast(context.get())); + if (m_hWinInetRequest == nullptr) + { + dwError = ::GetLastError(); + } + else if (::InternetSetStatusCallback( + m_hWinInetRequest, &WinInetRequestWrapper::winInetCallback) == + INTERNET_INVALID_STATUS_CALLBACK) + { + dwError = ::GetLastError(); + } + else + { + context.release(); + m_contextInstalled = true; + } + } + } + if (dwError != ERROR_SUCCESS) + { LOG_WARN("HttpOpenRequest() failed: %d", dwError); - // Request cannot be opened to given URL because of some connectivity issue DispatchEvent(OnConnectFailed); + onRequestComplete(dwError); + return; + } + if (shouldStopSetup()) + { onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } /* Perform optional MS Root certificate check for certain end-point URLs */ - if (m_parent.IsMsRootCheckRequired()) + if (m_msRootCheckRequired) { if (!isMsRootCert()) { + if (shouldStopSetup()) + { + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } // Request cannot be completed: end-point certificate is not MS-Rooted DispatchEvent(OnConnectFailed); onRequestComplete(ERROR_INTERNET_SEC_INVALID_CERT); return; } } - - ::InternetSetStatusCallback(m_hWinInetRequest, &WinInetRequestWrapper::winInetCallback); + if (shouldStopSetup()) + { + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } std::ostringstream os; for (auto const& header : m_request->m_headers) { os << header.first << ": " << header.second << "\r\n"; } + std::string headers = os.str(); - if (!::HttpAddRequestHeadersA(m_hWinInetRequest, os.str().data(), static_cast(os.tellp()), HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE)) + if (headers.size() > static_cast(std::numeric_limits::max())) + { + LOG_WARN("Request headers exceed WinInet's maximum size"); + DispatchEvent(OnConnectFailed); + onRequestComplete(ERROR_INVALID_PARAMETER); + return; + } + + if (!headers.empty()) + { + std::lock_guard lock(m_handleMutex); + if (m_hWinInetRequest == nullptr || shouldStopSetup()) + { + dwError = ERROR_INTERNET_OPERATION_CANCELLED; + } + else if (!::HttpAddRequestHeadersA( + m_hWinInetRequest, headers.c_str(), static_cast(headers.size()), + HTTP_ADDREQ_FLAG_ADD | HTTP_ADDREQ_FLAG_REPLACE)) + { + dwError = ::GetLastError(); + } + } + if (dwError != ERROR_SUCCESS) { - DWORD dwError = ::GetLastError(); LOG_WARN("HttpAddRequestHeadersA() failed: %d", dwError); - // Unable to add request headers. There's no point in proceeding with upload because - // our server is expecting those custom request headers to always be there. DispatchEvent(OnConnectFailed); + onRequestComplete(dwError); + return; + } + if (shouldStopSetup()) + { onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); return; } - // Try to send headers and request body to server DispatchEvent(OnSending); - void *data = static_cast(m_request->m_body.data()); - DWORD size = static_cast(m_request->m_body.size()); - BOOL bResult = ::HttpSendRequest(m_hWinInetRequest, NULL, 0, data, (DWORD)size); - DWORD dwError = GetLastError(); + if (shouldStopSetup()) + { + onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + return; + } + if (m_request->m_body.size() > static_cast(std::numeric_limits::max())) + { + LOG_WARN("Request body exceeds WinInet's maximum size"); + DispatchEvent(OnSendFailed); + onRequestComplete(ERROR_INVALID_PARAMETER); + return; + } - if (bResult == TRUE && dwError != ERROR_IO_PENDING) { - dwError = ::GetLastError(); + BOOL sendResult = FALSE; + bool completionPending = false; + DWORD completionError = ERROR_SUCCESS; + { + std::lock_guard lock(m_handleMutex); + if (m_hWinInetRequest == nullptr || shouldStopSetup()) + { + dwError = ERROR_INTERNET_OPERATION_CANCELLED; + } + else + { + void* data = m_request->m_body.empty() + ? nullptr + : static_cast(m_request->m_body.data()); + m_sendIssued = true; + ++m_asyncApiDepth; + sendResult = ::HttpSendRequestA( + m_hWinInetRequest, nullptr, 0, data, + static_cast(m_request->m_body.size())); + dwError = sendResult ? ERROR_SUCCESS : ::GetLastError(); + --m_asyncApiDepth; + completionPending = m_apiCompletionPending; + completionError = m_apiCompletionError; + m_apiCompletionPending = false; + m_apiCompletionError = ERROR_SUCCESS; + } + } + + if (completionPending) + { + onRequestComplete(completionError); + return; + } + if (sendResult) + { + // WinInet is permitted to finish an asynchronous-session request + // synchronously. A TRUE return is success, not an error. + onRequestComplete(ERROR_SUCCESS); + return; + } + if (dwError != ERROR_IO_PENDING) + { LOG_WARN("HttpSendRequest() failed: %d", dwError); - // Unable to send requerst DispatchEvent(OnSendFailed); - onRequestComplete(ERROR_INTERNET_OPERATION_CANCELLED); + onRequestComplete(dwError); return; } - // Async request has been queued in WinInet thread pool } static void CALLBACK winInetCallback(HINTERNET hInternet, DWORD_PTR dwContext, DWORD dwInternetStatus, LPVOID lpvStatusInformation, DWORD dwStatusInformationLength) { - UNREFERENCED_PARAMETER(dwStatusInformationLength); // Only used inside an assertion - UNREFERENCED_PARAMETER(hInternet); // Only used in debug printout OACR_USE_PTR(hInternet); - WinInetRequestWrapper* self = reinterpret_cast(dwContext); + WinInetCallbackContext* context = reinterpret_cast(dwContext); + if (context == nullptr) + { + return; + } LOG_TRACE("winInetCallback: hInternet %p, dwContext %p, dwInternetStatus %u", hInternet, dwContext, dwInternetStatus); // Are you looking at logs and need to decode dwInternetStatus values? // Go To Definition (F12) on INTERNET_STATUS_REQUEST_COMPLETE below to get to the right place of WinInet.h. switch (dwInternetStatus) { - case INTERNET_STATUS_REQUEST_SENT: { - assert(hInternet == self->m_hWinInetRequest); + case INTERNET_STATUS_REQUEST_SENT: return; - } - case INTERNET_STATUS_HANDLE_CLOSING: - // HANDLE_CLOSING should always come after REQUEST_COMPLETE. When (and if) - // it (ever) happens, WinInetRequestWrapper* self pointer may point to object - // that has been already destroyed. We do not perform any actions on it. + case INTERNET_STATUS_HANDLE_CLOSING: { + // The request handle owns the callback context after callback + // registration. HANDLE_CLOSING is its final notification. + std::unique_ptr contextOwner(context); + auto self = contextOwner->request; + DWORD deferredError = self->m_deferredError.load(std::memory_order_acquire); + if (deferredError != ERROR_SUCCESS && + !self->m_terminalCallbackStarted.load(std::memory_order_acquire)) + { + self->onRequestComplete(deferredError); + } return; + } case INTERNET_STATUS_REQUEST_COMPLETE: { - assert(dwStatusInformationLength >= sizeof(INTERNET_ASYNC_RESULT)); - INTERNET_ASYNC_RESULT& result = *static_cast(lpvStatusInformation); - assert(hInternet == self->m_hWinInetRequest); - if ((self != nullptr) && (self->m_hWinInetRequest != nullptr)) { - self->onRequestComplete(result.dwError); + auto self = context->request; + if (lpvStatusInformation == nullptr || + dwStatusInformationLength < sizeof(INTERNET_ASYNC_RESULT)) + { + LOG_WARN("WinInet REQUEST_COMPLETE callback returned invalid status data"); + self->onRequestComplete(ERROR_INTERNET_INTERNAL_ERROR); + return; } + INTERNET_ASYNC_RESULT const& result = + *static_cast(lpvStatusInformation); + self->onRequestComplete(result.dwError); return; } @@ -311,118 +651,231 @@ class WinInetRequestWrapper void DispatchEvent(HttpStateEvent type) { - if (m_appCallback != nullptr) + IHttpResponseCallback* callback = nullptr; + HINTERNET request = nullptr; + std::thread::id const callbackThread = std::this_thread::get_id(); { - m_appCallback->OnHttpStateEvent(type, static_cast(m_hWinInetRequest), 0); + std::lock_guard lock(m_handleMutex); + if (m_appCallback == nullptr || + m_terminalCallbackStarted.load(std::memory_order_acquire)) + { + return; + } + callback = m_appCallback; + request = m_hWinInetRequest; + ++m_stateCallbackDepth; + ++m_stateCallbacksByThread[callbackThread]; + } + callback->OnHttpStateEvent(type, static_cast(request), 0); + { + std::lock_guard lock(m_handleMutex); + --m_stateCallbackDepth; + auto it = m_stateCallbacksByThread.find(callbackThread); + if (it != m_stateCallbacksByThread.end() && --it->second == 0) + { + m_stateCallbacksByThread.erase(it); + } } } void onRequestComplete(DWORD dwError) { - if (dwError == ERROR_SUCCESS) { - // If looking good so far, try to fetch the response body first. - // It might potentially be another async operation which will - // trigger INTERNET_STATUS_REQUEST_COMPLETE again. - - // SECURITY: refuse an over-large response instead of buffering it (see - // MAX_HTTP_RESPONSE_SIZE) so a hostile/MITM'd collector cannot exhaust - // process memory. Checked before every append so the buffer never exceeds - // the cap; reported as an invalid server response -> NetworkFailure (retried). - if (m_bodyBuffer.size() + m_bufferUsed > MAX_HTTP_RESPONSE_SIZE) { - LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); - dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE; - } else { - m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed); - while (!m_readingData || m_bufferUsed != 0) { - BOOL bResult = ::InternetReadFile(m_hWinInetRequest, m_buffer, sizeof(m_buffer), &m_bufferUsed); + { + std::lock_guard lock(m_handleMutex); + if (m_stateCallbackDepth != 0 || m_setupActive) + { + m_setupCompletionPending = true; + m_setupCompletionError = dwError; + return; + } + if (m_asyncApiDepth != 0) + { + // WinInet can invoke REQUEST_COMPLETE before an asynchronous + // API returns. Let the issuing frame consume that completion + // after it has restored its local state. + m_apiCompletionPending = true; + m_apiCompletionError = dwError; + return; + } + if (m_terminalCallbackStarted.load(std::memory_order_acquire)) + { + return; + } + DWORD deferredError = m_deferredError.load(std::memory_order_acquire); + if (deferredError != ERROR_SUCCESS) + { + dwError = deferredError; + } + } + + if (dwError == ERROR_SUCCESS) + { + std::lock_guard lock(m_handleMutex); + DWORD deferredError = m_deferredError.load(std::memory_order_acquire); + if (deferredError != ERROR_SUCCESS) + { + dwError = deferredError; + } + else if (m_hWinInetRequest == nullptr) + { + dwError = ERROR_INTERNET_OPERATION_CANCELLED; + } + else + { + auto appendReadBuffer = [this]() -> bool { + if (m_bodyBuffer.size() > MAX_HTTP_RESPONSE_SIZE || + m_bufferUsed > MAX_HTTP_RESPONSE_SIZE - m_bodyBuffer.size()) + { + return false; + } + m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed); + return true; + }; + + bool shouldRead = !m_readingData || m_bufferUsed != 0; + if (m_readingData && !appendReadBuffer()) + { + dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE; + } + + while (dwError == ERROR_SUCCESS && shouldRead) + { + ++m_asyncApiDepth; + BOOL readResult = ::InternetReadFile( + m_hWinInetRequest, m_buffer, sizeof(m_buffer), &m_bufferUsed); + DWORD readError = readResult ? ERROR_SUCCESS : ::GetLastError(); + --m_asyncApiDepth; m_readingData = true; - if (!bResult) { - dwError = GetLastError(); - if (dwError == ERROR_IO_PENDING) { - // Do not touch anything from this thread anymore. - // The buffer passed to InternetReadFile() and the - // read count will be filled asynchronously, so they - // must stay valid and writable until the next - // INTERNET_STATUS_REQUEST_COMPLETE callback comes - // (that's why those are member variables). - LOG_TRACE("InternetReadFile() failed: ERROR_IO_PENDING. Waiting for INTERNET_STATUS_REQUEST_COMPLETE to be called again"); + + bool completionPending = m_apiCompletionPending; + DWORD completionError = m_apiCompletionError; + m_apiCompletionPending = false; + m_apiCompletionError = ERROR_SUCCESS; + + if (completionPending) + { + if (completionError != ERROR_SUCCESS) + { + dwError = completionError; + break; + } + } + else if (!readResult) + { + if (readError == ERROR_IO_PENDING) + { + LOG_TRACE("InternetReadFile() is pending; waiting for REQUEST_COMPLETE"); return; } - LOG_WARN("InternetReadFile() failed: %d", dwError); + dwError = readError; break; } - if (m_bodyBuffer.size() + m_bufferUsed > MAX_HTTP_RESPONSE_SIZE) { - LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + if (!appendReadBuffer()) + { dwError = ERROR_HTTP_INVALID_SERVER_RESPONSE; break; } - m_bodyBuffer.insert(m_bodyBuffer.end(), m_buffer, m_buffer + m_bufferUsed); + shouldRead = m_bufferUsed != 0; } } } - std::unique_ptr response(new SimpleHttpResponse(m_id)); + if (dwError == ERROR_HTTP_INVALID_SERVER_RESPONSE) + { + LOG_WARN("HTTP response exceeds max buffered size (%zu bytes); aborting", MAX_HTTP_RESPONSE_SIZE); + } + else if (dwError != ERROR_SUCCESS && + dwError != ERROR_INTERNET_OPERATION_CANCELLED) + { + LOG_WARN("WinInet request failed: %d", dwError); + } + + HINTERNET request = nullptr; + { + std::lock_guard lock(m_handleMutex); + DWORD deferredError = m_deferredError.load(std::memory_order_acquire); + if (deferredError != ERROR_SUCCESS) + { + dwError = deferredError; + } + if (m_terminalCallbackStarted.exchange(true, std::memory_order_acq_rel)) + { + return; + } + request = m_hWinInetRequest; + if (dwError == ERROR_SUCCESS && request == nullptr) + { + dwError = ERROR_INTERNET_OPERATION_CANCELLED; + } + } - // SUCCESS with no IO_PENDING means we're done with the response body: try to parse the response headers. - if (dwError == ERROR_SUCCESS) { + std::unique_ptr response(new SimpleHttpResponse(m_id)); + if (dwError == ERROR_SUCCESS) + { response->m_body = m_bodyBuffer; - response->m_result = HttpResult_OK; - - uint32_t value = 0; - DWORD dwSize = sizeof(value); - BOOL bResult = ::HttpQueryInfoA(m_hWinInetRequest, HTTP_QUERY_STATUS_CODE | HTTP_QUERY_FLAG_NUMBER, &value, &dwSize, NULL); - if (!bResult) { - LOG_WARN("HttpQueryInfo(STATUS_CODE) failed: %d", GetLastError()); - } - response->m_statusCode = value; - - char* pBuffer = reinterpret_cast(m_buffer); - dwSize = sizeof(m_buffer) - 1; - if (!HttpQueryInfoA(m_hWinInetRequest, HTTP_QUERY_RAW_HEADERS_CRLF, pBuffer, &dwSize, NULL)) { - dwError = GetLastError(); - if (dwError != ERROR_INSUFFICIENT_BUFFER) { - LOG_WARN("HttpQueryInfo(RAW_HEADERS) failed: %d", dwError); - dwSize = 0; - } else { - m_bodyBuffer.resize(dwSize + 1); - pBuffer = reinterpret_cast(m_bodyBuffer.data()); - if (!HttpQueryInfoA(m_hWinInetRequest, HTTP_QUERY_RAW_HEADERS_CRLF, pBuffer, &dwSize, NULL)) { - LOG_WARN("HttpQueryInfo(RAW_HEADERS) failed twice: %d", dwError); - dwSize = 0; - } + + uint32_t statusCode = 0; + DWORD statusBytes = sizeof(statusCode); + { + std::lock_guard lock(m_handleMutex); + if (!::HttpQueryInfoA( + request, HTTP_QUERY_STATUS_CODE | HTTP_QUERY_FLAG_NUMBER, + &statusCode, &statusBytes, nullptr)) + { + dwError = ::GetLastError(); + LOG_WARN("HttpQueryInfo(STATUS_CODE) failed: %d", dwError); } } - pBuffer[dwSize] = '\0'; + response->m_statusCode = statusCode; - char const* ptr = pBuffer; - while (*ptr) { - char const* colon = strchr(ptr, ':'); - if (!colon) { - break; - } - std::string name(ptr, colon); + if (dwError == ERROR_SUCCESS) + { + response->m_result = HttpResult_OK; - ptr = colon + 1; - while (*ptr == ' ') { - ptr++; + DWORD headerBytes = 0; + BOOL headersQueried = FALSE; + DWORD headerError = ERROR_SUCCESS; + { + std::lock_guard lock(m_handleMutex); + headersQueried = ::HttpQueryInfoA( + request, HTTP_QUERY_RAW_HEADERS_CRLF, nullptr, + &headerBytes, nullptr); + headerError = headersQueried ? ERROR_SUCCESS : ::GetLastError(); } - - char const* eol = strstr(ptr, "\r\n"); - if (!eol) { - break; + if (!headersQueried && + headerError == ERROR_INSUFFICIENT_BUFFER && + headerBytes > 0 && + headerBytes < std::numeric_limits::max()) + { + std::vector headers(static_cast(headerBytes) + 1, '\0'); + DWORD bufferBytes = headerBytes; + { + std::lock_guard lock(m_handleMutex); + headersQueried = ::HttpQueryInfoA( + request, HTTP_QUERY_RAW_HEADERS_CRLF, headers.data(), + &bufferBytes, nullptr); + headerError = headersQueried ? ERROR_SUCCESS : ::GetLastError(); + } + if (headersQueried) + { + headers.back() = '\0'; + parseHeaders(std::string(headers.data()), *response); + } + else + { + LOG_WARN("HttpQueryInfo(RAW_HEADERS) failed twice: %d", headerError); + } + } + else if (!headersQueried && headerError != ERROR_SUCCESS) + { + LOG_WARN("HttpQueryInfo(RAW_HEADERS) failed: %d", headerError); } - std::string value1(ptr, eol); - - response->m_headers.add(name, value1); - ptr = eol + 2; } - // This event handler covers the only positive case when we actually got some server response. - // We may still invoke OnHttpResponse(...) below for this positive as well as other negative - // cases where there was a short-read, connection failuire or timeout on reading the response. - DispatchEvent(OnResponse); + } - } else { + if (dwError != ERROR_SUCCESS) + { switch (dwError) { case ERROR_INTERNET_OPERATION_CANCELLED: response->m_result = HttpResult_Aborted; @@ -461,53 +914,164 @@ class WinInetRequestWrapper } } - assert(isCallbackCalled == false); - if (!isCallbackCalled) + auto keepAlive = shared_from_this(); + auto callback = m_appCallback; + auto requestId = m_id; + + // Closing first guarantees WinInet no longer owns the caller's request + // body before OnHttpResponse allows that request to be destroyed. + closeRequestHandle(); + closeSessionHandle(); + WinInetCallbackScope callbackScope(m_clientState); + // Remove the request before application code so a callback may safely + // cancel all requests or tear the client down synchronously. + m_clientState->eraseRequest(requestId); + + if (callback != nullptr) + { + if (dwError == ERROR_SUCCESS) + { + // The implementation-specific handle is no longer valid once + // terminal delivery begins, so do not expose a stale handle. + callback->OnHttpStateEvent(OnResponse, nullptr, 0); + } + callback->OnHttpResponse(response.release()); + } + } + + static void parseHeaders(std::string const& raw, SimpleHttpResponse& response) + { + size_t lineStart = 0; + while (lineStart < raw.size()) { - // Only one WinInet worker thread may invoke async callback for a given request at any given moment of time. - // That ensures that isCallbackCalled does not require a lock around it. We unregister the callback here - // to ensure that no more callbacks are coming for that m_hWinInetRequest. - ::InternetSetStatusCallback(m_hWinInetRequest, NULL); - isCallbackCalled = true; - m_appCallback->OnHttpResponse(response.release()); - // HttpClient parent is destroying this HttpRequest object by id - m_parent.erase(m_id); + size_t lineEnd = raw.find("\r\n", lineStart); + if (lineEnd == std::string::npos) + { + lineEnd = raw.size(); + } + + std::string const line = raw.substr(lineStart, lineEnd - lineStart); + size_t const colon = line.find(':'); + if (colon != std::string::npos) + { + size_t valueStart = colon + 1; + while (valueStart < line.size() && line[valueStart] == ' ') + { + ++valueStart; + } + response.m_headers.add( + line.substr(0, colon), line.substr(valueStart)); + } + + if (lineEnd == raw.size()) + { + break; + } + lineStart = lineEnd + 2; } } }; //--- -unsigned HttpClient_WinInet::s_nextRequestId = 0; +WinInetClientState::WinInetClientState(HINTERNET internetHandle) : + internet(internetHandle) +{ +} -HttpClient_WinInet::HttpClient_WinInet() : - m_msRootCheck(false) +WinInetClientState::~WinInetClientState() { - m_hInternet = ::InternetOpen(NULL, INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, INTERNET_FLAG_ASYNC); + if (internet != nullptr) + { + ::InternetCloseHandle(internet); + } } -HttpClient_WinInet::~HttpClient_WinInet() +bool WinInetClientState::registerRequest( + std::string const& id, + std::shared_ptr request) { - CancelAllRequests(); - ::InternetCloseHandle(m_hInternet); + bool shouldSend; + { + std::lock_guard lock(requestsMutex); + if (!acceptingRequests) + { + return false; + } + requests[id] = std::move(request); + ++registryGeneration; + shouldSend = cancelAllDepth == 0; + } + requestsCv.notify_all(); + return shouldSend; } -/** - * This method is called exclusively from onRequestComplete . - * No other code paths that lead to request destruction. - */ -void HttpClient_WinInet::erase(std::string const& id) +void WinInetClientState::eraseRequest(std::string const& id) { - LOCKGUARD(m_requestsMutex); - auto it = m_requests.find(id); - if (it != m_requests.end()) { - auto req = it->second; - m_requests.erase(it); - // Wake CancelAllRequests() waiting for the map to drain. - m_requestsCV.notify_all(); - // delete WinInetRequestWrapper - delete req; + { + std::lock_guard lock(requestsMutex); + requests.erase(id); + ++registryGeneration; } + requestsCv.notify_all(); +} + +void WinInetClientState::stopAcceptingRequests() +{ + std::lock_guard lock(requestsMutex); + acceptingRequests = false; +} + +void WinInetClientState::beginCallback() +{ + { + std::lock_guard lock(requestsMutex); + ++callbacksInFlight; + ++callbacksByThread[std::this_thread::get_id()]; + ++callbackGeneration; + } + requestsCv.notify_all(); +} + +void WinInetClientState::endCallback() +{ + { + std::lock_guard lock(requestsMutex); + if (callbacksInFlight == 0) + { + LOG_ERROR("WinInet callback accounting underflow"); + requestsCv.notify_all(); + return; + } + + --callbacksInFlight; + auto it = callbacksByThread.find(std::this_thread::get_id()); + if (it == callbacksByThread.end() || it->second == 0) + { + LOG_ERROR("WinInet callback thread was not registered"); + } + else if (--it->second == 0) + { + callbacksByThread.erase(it); + } + ++callbackGeneration; + } + requestsCv.notify_all(); +} + +unsigned HttpClient_WinInet::s_nextRequestId = 0; + +HttpClient_WinInet::HttpClient_WinInet() +{ + auto internet = ::InternetOpen( + NULL, INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, INTERNET_FLAG_ASYNC); + m_state = std::make_shared(internet); +} + +HttpClient_WinInet::~HttpClient_WinInet() +{ + m_state->stopAcceptingRequests(); + CancelAllRequests(); } IHttpRequest* HttpClient_WinInet::CreateRequest() @@ -519,20 +1083,24 @@ IHttpRequest* HttpClient_WinInet::CreateRequest() void HttpClient_WinInet::SendRequestAsync(IHttpRequest* request, IHttpResponseCallback* callback) { // Note: 'request' is never owned by IHttpClient and gets deleted in EventsUploadContext.clear() - WinInetRequestWrapper *wrapper = new WinInetRequestWrapper(*this, static_cast(request)); + auto wrapper = std::make_shared( + m_state, static_cast(request)); wrapper->send(callback); } void HttpClient_WinInet::CancelRequestAsync(std::string const& id) { - LOCKGUARD(m_requestsMutex); - auto it = m_requests.find(id); - if (it != m_requests.end()) { - auto request = it->second; - if (request) { - request->cancel(); + std::shared_ptr request; + { + std::lock_guard lock(m_state->requestsMutex); + auto it = m_state->requests.find(id); + if (it != m_state->requests.end()) { + request = it->second; } } + if (request) { + request->cancel(); + } } @@ -543,38 +1111,131 @@ void HttpClient_WinInet::CancelAllRequests() void HttpClient_WinInet::CancelAllRequests(std::chrono::milliseconds bestEffortTimeout) { - // vector of all request IDs - std::vector ids; + auto state = m_state; + class CancelAllScope { - LOCKGUARD(m_requestsMutex); - for (auto const& item : m_requests) { - ids.push_back(item.first); + public: + explicit CancelAllScope(std::shared_ptr state) + : m_state(std::move(state)) + { + std::lock_guard lock(m_state->requestsMutex); + ++m_state->cancelAllDepth; } - } - // cancel all requests one-by-one not holding the lock - for (const auto &id : ids) - CancelRequestAsync(id); - - // Wait for all request destructors to run (erase() removes them on the WinInet - // callback thread). Use a condition variable signaled from erase() rather than a - // poll loop so this never spins at 100% CPU while draining. WinInet delivers the - // cancellation callbacks on its own threads, so the wait completes without - // depending on the SDK task dispatcher. - std::unique_lock lock(m_requestsMutex); - if (bestEffortTimeout > std::chrono::milliseconds::zero()) - { - // Best-effort (e.g. pause): the caller must not block indefinitely. The client - // is NOT being destroyed here, so a late callback that arrives after this - // returns still runs erase() on a live client -- returning early is safe. - m_requestsCV.wait_for(lock, bestEffortTimeout, [this] { return m_requests.empty(); }); - } - else + + ~CancelAllScope() + { + if (m_active) + { + std::lock_guard lock(m_state->requestsMutex); + --m_state->cancelAllDepth; + } + } + + void finishLocked() + { + --m_state->cancelAllDepth; + m_active = false; + } + + private: + std::shared_ptr m_state; + bool m_active {true}; + } cancelAllScope(state); + + bool const hasTimeout = + bestEffortTimeout > std::chrono::milliseconds::zero(); + auto const deadline = + std::chrono::steady_clock::now() + bestEffortTimeout; + std::thread::id const callerThread = std::this_thread::get_id(); + auto requestsDrainedForCaller = [&state, callerThread]() { + if (state->requests.empty()) + { + return true; + } + + bool callerIsInStateCallback = false; + for (auto const& item : state->requests) + { + if (item.second->hasStateCallbackOnThread(callerThread)) + { + callerIsInStateCallback = true; + break; + } + } + for (auto const& item : state->requests) + { + if (!callerIsInStateCallback || + !item.second->hasActiveStateCallback()) + { + return false; + } + } + return true; + }; + auto callbacksDrainedForCaller = [&state, callerThread]() { + // A terminal callback cannot wait for peer callbacks: two callbacks + // doing so concurrently would wait on each other. Each callback scope + // retains the shared client state independently. + return state->callbacksByThread.find(callerThread) != + state->callbacksByThread.end() || + state->callbacksInFlight == 0; + }; + + for (;;) { - // Full drain barrier (the destructor calls this): returning early with - // requests still in flight would let a late WinInet callback invoke - // WinInetRequestWrapper::OnHttpResponse -> m_parent.erase() on a destroyed - // client, so wait for every request to drain. - m_requestsCV.wait(lock, [this] { return m_requests.empty(); }); + std::vector> requests; + size_t registryGeneration; + size_t callbackGeneration; + { + std::lock_guard lock(state->requestsMutex); + if (state->requests.empty() && callbacksDrainedForCaller()) + { + // Holding the registry lock makes completion of this cancellation + // epoch the linearization point: later registrations are new work. + cancelAllScope.finishLocked(); + return; + } + + registryGeneration = state->registryGeneration; + callbackGeneration = state->callbackGeneration; + for (auto const& item : state->requests) + { + requests.push_back(item.second); + } + } + + for (auto const& request : requests) + { + if (hasTimeout && std::chrono::steady_clock::now() >= deadline) + { + break; + } + request->cancel(); + } + + std::unique_lock lock(state->requestsMutex); + if (requestsDrainedForCaller() && callbacksDrainedForCaller()) + { + cancelAllScope.finishLocked(); + return; + } + auto stateChangedOrDrained = [&]() { + return state->registryGeneration != registryGeneration || + state->callbackGeneration != callbackGeneration || + (requestsDrainedForCaller() && callbacksDrainedForCaller()); + }; + if (hasTimeout) + { + if (!state->requestsCv.wait_until( + lock, deadline, stateChangedOrDrained)) + { + return; + } + } + else + { + state->requestsCv.wait(lock, stateChangedOrDrained); + } } } @@ -589,21 +1250,20 @@ void HttpClient_WinInet::ApplySettings(ILogConfiguration& config) void HttpClient_WinInet::SetMsRootCheck(bool enforceMsRoot) { - m_msRootCheck = enforceMsRoot; + m_state->msRootCheck.store(enforceMsRoot, std::memory_order_release); } /// -/// Determines whether MS-Roted server cert check required. +/// Determines whether an MS-Rooted server certificate check is required. /// /// /// true if [MS-Rooted server cert check required]; otherwise, false. /// bool HttpClient_WinInet::IsMsRootCheckRequired() { - return m_msRootCheck; + return m_state->msRootCheck.load(std::memory_order_acquire); } } MAT_NS_END -#pragma warning(pop) #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT // clang-format on diff --git a/lib/http/HttpClient_WinInet.hpp b/lib/http/HttpClient_WinInet.hpp index 42b256157..dde1b2538 100644 --- a/lib/http/HttpClient_WinInet.hpp +++ b/lib/http/HttpClient_WinInet.hpp @@ -14,6 +14,7 @@ #include "ILogManager.hpp" #include +#include #include namespace MAT_NS_BEGIN { @@ -23,6 +24,7 @@ typedef void* HINTERNET; #endif class WinInetRequestWrapper; +struct WinInetClientState; class HttpClient_WinInet : public IHttpClient, public IBoundedHttpClientCancel { public: @@ -42,17 +44,8 @@ class HttpClient_WinInet : public IHttpClient, public IBoundedHttpClientCancel { bool IsMsRootCheckRequired(); protected: - void erase(std::string const& id); - - protected: - HINTERNET m_hInternet; - std::recursive_mutex m_requestsMutex; - std::map m_requests; - // Signaled from erase() when a request is removed, so CancelAllRequests can drain - // via a condition variable instead of a poll loop (no 100% CPU spin). - std::condition_variable_any m_requestsCV; + std::shared_ptr m_state; static unsigned s_nextRequestId; - bool m_msRootCheck; friend class WinInetRequestWrapper; }; diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index 12ac6aa00..062c90318 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/lib/http/HttpClient_WinRt.cpp @@ -11,9 +11,7 @@ #include "http/HttpClient_WinRt.hpp" #include "utils/StringUtils.hpp" -#include #include -#include #include #include @@ -21,7 +19,6 @@ #include #include #include -#include using namespace Windows::Foundation; using namespace Windows::Foundation::Collections; diff --git a/lib/http/IBoundedHttpClientCancel.hpp b/lib/http/IBoundedHttpClientCancel.hpp index f832e4678..c0527d311 100644 --- a/lib/http/IBoundedHttpClientCancel.hpp +++ b/lib/http/IBoundedHttpClientCancel.hpp @@ -16,8 +16,10 @@ class IBoundedHttpClientCancel public: virtual ~IBoundedHttpClientCancel() noexcept = default; - // Positive timeout is a best-effort cap. Zero means the caller requires a - // full drain, matching IHttpClient::CancelAllRequests(). + // Positive timeout is a soft, best-effort cap. Implementations stop + // initiating additional cancellations at the deadline, but one synchronous + // native handle close already in progress may finish after it. Zero means + // the caller requires a full drain, matching IHttpClient::CancelAllRequests(). virtual void CancelAllRequests(std::chrono::milliseconds bestEffortTimeout) = 0; }; diff --git a/lib/include/public/DebugEvents.hpp b/lib/include/public/DebugEvents.hpp index 506611c04..65fde316e 100644 --- a/lib/include/public/DebugEvents.hpp +++ b/lib/include/public/DebugEvents.hpp @@ -167,8 +167,10 @@ namespace MAT_NS_BEGIN /// for debugging and unit testing (not recommended for use in a production environment). /// /// Customers can implement this abstract class to track when certain events - /// happen under the hood in the Microsoft Telemetry SDK. The callback is synchronously executed - /// within the context of the Microsoft Telemetry worker thread. + /// happen under the hood in the Microsoft Telemetry SDK. The callback is synchronously + /// executed within the context of an SDK-owned thread. A listener must not synchronously + /// destroy the LogManager or call FlushAndTeardown(); defer teardown to an + /// application-owned thread after the callback returns instead. /// class MATSDK_LIBABI DebugEventListener { @@ -247,4 +249,3 @@ namespace MAT_NS_BEGIN } MAT_NS_END #endif - diff --git a/lib/include/public/IHttpClient.hpp b/lib/include/public/IHttpClient.hpp index 0b2727803..66193e78f 100644 --- a/lib/include/public/IHttpClient.hpp +++ b/lib/include/public/IHttpClient.hpp @@ -196,9 +196,9 @@ namespace MAT_NS_BEGIN virtual ~IHttpResponse() noexcept = default; /// - /// Gets the response ID. + /// Gets the ID of the request that produced this response. /// - /// A string that contains the response ID. + /// The same ID returned by the originating IHttpRequest::GetId(). virtual const std::string& GetId() const = 0; /// diff --git a/lib/offline/OfflineStorageHandler.cpp b/lib/offline/OfflineStorageHandler.cpp index 52ce15515..132c661b8 100644 --- a/lib/offline/OfflineStorageHandler.cpp +++ b/lib/offline/OfflineStorageHandler.cpp @@ -10,14 +10,45 @@ #include "ILogManager.hpp" #include +#include #include #include +#include namespace MAT_NS_BEGIN { - MATSDK_LOG_INST_COMPONENT_CLASS(OfflineStorageHandler, "EventsSDK.StorageHandler", "Events telemetry client - OfflineStorageHandler class") + namespace + { + class ActivityGuard + { + public: + explicit ActivityGuard(ILogManager& logManager) : + m_logManager(logManager), + m_active(logManager.StartActivity()) + { + } + + ~ActivityGuard() noexcept + { + if (m_active) + { + m_logManager.EndActivity(); + } + } + + bool IsActive() const noexcept + { + return m_active; + } + + private: + ILogManager& m_logManager; + bool m_active; + }; + } + OfflineStorageHandler::OfflineStorageHandler(ILogManager& logManager, IRuntimeConfig& runtimeConfig, ITaskDispatcher& taskDispatcher) : m_observer(nullptr), m_logManager(logManager), @@ -59,12 +90,14 @@ namespace MAT_NS_BEGIN { void OfflineStorageHandler::WaitForFlush() { + MAT::Task* pendingTask = nullptr; { LOCKGUARD(m_flushLock); if (!m_flushPending) return; + pendingTask = m_flushHandle.GetTask(); } - LOG_INFO("Waiting for pending Flush (%p) to complete...", m_flushHandle.m_task); + LOG_INFO("Waiting for pending Flush (%p) to complete...", pendingTask); m_flushComplete.wait(); } @@ -113,7 +146,22 @@ namespace MAT_NS_BEGIN { if (nullptr != m_offlineStorageMemory) { m_offlineStorageMemory->ReleaseAllRecords(); - Flush(); + // Shutdown already owns the handler lifetime and runs after the + // LogManager has paused new activity. Persist the memory cache + // directly instead of routing through the asynchronous activity + // guard, which must reject work once pause begins. + try + { + FlushImpl(); + } + catch (const std::exception& ex) + { + LOG_ERROR("Offline storage shutdown flush failed: %s", ex.what()); + } + catch (...) + { + LOG_ERROR("Offline storage shutdown flush failed"); + } m_offlineStorageMemory->Shutdown(); } if (nullptr != m_offlineStorageDisk) @@ -161,11 +209,35 @@ namespace MAT_NS_BEGIN { return count; } + void OfflineStorageHandler::SignalFlushComplete() + { + LOCKGUARD(m_flushLock); + m_flushHandle = PAL::DeferredCallbackHandle(); + m_flushPending = false; + m_flushComplete.post(); + } + void OfflineStorageHandler::Flush() { - if (!m_logManager.StartActivity()) { - return; + try + { + ActivityGuard activity(m_logManager); + if (activity.IsActive()) + { + FlushImpl(); + } } + catch (...) + { + SignalFlushComplete(); + throw; + } + + SignalFlushComplete(); + } + + void OfflineStorageHandler::FlushImpl() + { // Flush could be executed from context of worker thread, as well as from TPM and // after HTTP callback. Make sure it is atomic / thread-safe. LOCKGUARD(m_flushLock); @@ -180,23 +252,50 @@ namespace MAT_NS_BEGIN { { // This will block on and then take a lock for the duration of this move, and // StoreRecord() will then block until the move completes. - auto records = m_offlineStorageMemory->GetRecords(false, EventLatency_Unspecified); - std::vector ids; + auto memoryRecords = + m_offlineStorageMemory->GetRecords(false, EventLatency_Unspecified); + std::vector persistentRecords; + persistentRecords.reserve(memoryRecords.size()); + for (auto& record : memoryRecords) + { + if (record.persistence != EventPersistence_DoNotStoreOnDisk) + { + persistentRecords.push_back(std::move(record)); + } + } // TODO: [MG] - consider running the batch in transaction // if (sqlite) // sqlite->Execute("BEGIN"); - size_t totalSaved = m_offlineStorageDisk->StoreRecords(records); + // IOfflineStorage::StoreRecords accepts a mutable vector, so an + // external storage module may consume or reorder its input. Keep an + // untouched batch for exception and partial-write recovery. + auto recordsForRetry = persistentRecords; + size_t const recordsToSave = recordsForRetry.size(); + size_t totalSaved = 0; + try + { + totalSaved = m_offlineStorageDisk->StoreRecords(persistentRecords); + } + catch (...) + { + // GetRecords() removes records from the RAM queue. Restore them + // before propagating so a transient disk failure cannot lose data. + m_offlineStorageMemory->StoreRecords(recordsForRetry); + throw; + } // TODO: [MG] - consider running the batch in transaction // if (sqlite) // sqlite->Execute("END"); - // Delete records from reserved on flush - HttpHeaders dummy; - bool fromMemory = true; - m_offlineStorageMemory->DeleteRecords(ids, dummy, fromMemory); + if (totalSaved != recordsToSave) + { + // StoreRecords reports only a count, not the failed record IDs. + // Restore the complete batch to preserve at-least-once delivery. + m_offlineStorageMemory->StoreRecords(recordsForRetry); + } // Notify event listener about the records cached OnStorageRecordsSaved(totalSaved); @@ -211,17 +310,15 @@ namespace MAT_NS_BEGIN { } // Checkpoint DB - if (m_config.HasConfig(CFG_BOOL_CHECKPOINT_DB_ON_FLUSH) && m_config[CFG_BOOL_CHECKPOINT_DB_ON_FLUSH]) + if (m_offlineStorageDisk != nullptr && + m_config.HasConfig(CFG_BOOL_CHECKPOINT_DB_ON_FLUSH) && + m_config[CFG_BOOL_CHECKPOINT_DB_ON_FLUSH]) { m_offlineStorageDisk->Flush(); } m_isStorageFullNotificationSend = false; - // Flush is done, notify the waiters - m_flushComplete.post(); - m_flushPending = false; - m_logManager.EndActivity(); } bool OfflineStorageHandler::StoreRecord(StorageRecord const& record) @@ -253,16 +350,20 @@ namespace MAT_NS_BEGIN { // Perform periodic flush to disk if (memDbSize > cacheMemorySizeLimitInBytes) { - if (m_flushLock.try_lock()) + std::unique_lock flushLock(m_flushLock, std::try_to_lock); + if (flushLock.owns_lock()) { if (!m_flushPending) { - m_flushPending = true; - m_flushComplete.Reset(); - m_flushHandle = PAL::scheduleTask(&m_taskDispatcher, 0, this, &OfflineStorageHandler::Flush); - LOG_INFO("Requested Flush (%p)", m_flushHandle.m_task); + auto flushHandle = PAL::scheduleTask(&m_taskDispatcher, 0, this, &OfflineStorageHandler::Flush); + m_flushHandle = std::move(flushHandle); + if (m_flushHandle.GetTask() != nullptr) + { + m_flushComplete.Reset(); + m_flushPending = true; + LOG_INFO("Requested Flush (%p)", m_flushHandle.GetTask()); + } } - m_flushLock.unlock(); } } } diff --git a/lib/offline/OfflineStorageHandler.hpp b/lib/offline/OfflineStorageHandler.hpp index 9a1131aff..1e4aefaa4 100644 --- a/lib/offline/OfflineStorageHandler.hpp +++ b/lib/offline/OfflineStorageHandler.hpp @@ -100,6 +100,8 @@ namespace MAT_NS_BEGIN { private: void WaitForFlush(); + void FlushImpl(); + void SignalFlushComplete(); }; diff --git a/lib/offline/OfflineStorage_SQLite.cpp b/lib/offline/OfflineStorage_SQLite.cpp index b9b2ed83d..a65d910d8 100644 --- a/lib/offline/OfflineStorage_SQLite.cpp +++ b/lib/offline/OfflineStorage_SQLite.cpp @@ -152,7 +152,9 @@ namespace MAT_NS_BEGIN { // TODO: [MG] - this works, but may not play nicely with several LogManager instances // static SqliteStatement sql_insert(*m_db, m_stmtInsertEvent_id_tenant_prio_ts_data); - if (record.id.empty() || record.tenantToken.empty() || static_cast(record.latency) < 0 || record.timestamp <= 0) { + if (record.id.empty() || record.tenantToken.empty() + || record.latency < EventLatency_Off || record.latency > EventLatency_Max + || record.timestamp <= 0) { LOG_ERROR("Failed to store event %s:%s: Invalid parameters", tenantTokenToId(record.tenantToken).c_str(), record.id.c_str()); m_observer->OnStorageFailed("Invalid parameters"); @@ -1064,4 +1066,3 @@ namespace MAT_NS_BEGIN { } MAT_NS_END #endif - diff --git a/lib/pal/TaskDispatcher.hpp b/lib/pal/TaskDispatcher.hpp index ec6f2f690..81ee703f3 100644 --- a/lib/pal/TaskDispatcher.hpp +++ b/lib/pal/TaskDispatcher.hpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include "ITaskDispatcher.hpp" @@ -25,6 +26,12 @@ namespace PAL_NS_BEGIN { namespace detail { + struct TaskLifetimeState + { + std::recursive_mutex mutex; + MAT::Task* task {nullptr}; + }; + template class TaskCall : public Task { @@ -48,14 +55,36 @@ namespace PAL_NS_BEGIN { this->TargetTime = targetTime; } + TaskCall(TCall& call, int64_t targetTime, std::shared_ptr lifetimeState) : + Task(), + m_call(call), + m_lifetimeState(std::move(lifetimeState)) + { + this->TypeName = TYPENAME(call); + this->Type = Task::TimedCall; + this->TargetTime = targetTime; + std::lock_guard lock(m_lifetimeState->mutex); + m_lifetimeState->task = this; + } + virtual void operator()() override { m_call(); } - virtual ~TaskCall() noexcept = default; + virtual ~TaskCall() noexcept + { + if (m_lifetimeState) + { + std::lock_guard lock(m_lifetimeState->mutex); + m_lifetimeState->task = nullptr; + } + } const TCall m_call; + + private: + std::shared_ptr m_lifetimeState; }; } // namespace detail @@ -63,14 +92,11 @@ namespace PAL_NS_BEGIN { class DeferredCallbackHandle { public: - std::mutex m_mutex; - MAT::Task* m_task = nullptr; - MAT::ITaskDispatcher* m_taskDispatcher = nullptr; - - DeferredCallbackHandle(MAT::Task* task, MAT::ITaskDispatcher* taskDispatcher) : - m_task(task), + DeferredCallbackHandle(std::shared_ptr taskLifetimeState, MAT::ITaskDispatcher* taskDispatcher) : + m_taskLifetimeState(std::move(taskLifetimeState)), m_taskDispatcher(taskDispatcher) { } - DeferredCallbackHandle() {} + + DeferredCallbackHandle() = default; DeferredCallbackHandle(DeferredCallbackHandle&& h) { *this = std::move(h); @@ -78,28 +104,59 @@ namespace PAL_NS_BEGIN { DeferredCallbackHandle& operator=(DeferredCallbackHandle&& other) { - std::lock_guard lock(m_mutex); - std::lock_guard otherLock(other.m_mutex); - m_task = other.m_task; - other.m_task = nullptr; + if (this == &other) + { + return *this; + } + + std::unique_lock lock(m_mutex, std::defer_lock); + std::unique_lock otherLock(other.m_mutex, std::defer_lock); + std::lock(lock, otherLock); + m_taskLifetimeState = std::move(other.m_taskLifetimeState); m_taskDispatcher = other.m_taskDispatcher; + other.m_taskDispatcher = nullptr; return *this; } - bool Cancel(uint64_t waitTime = 0) + MAT::Task* GetTask() const { std::lock_guard lock(m_mutex); - if (m_task) + if (m_taskLifetimeState == nullptr) { - bool result = (m_taskDispatcher != nullptr) && (m_taskDispatcher->Cancel(m_task, waitTime)); - return result; + return nullptr; } - else { - // Canceled nothing successfully + std::lock_guard lifetimeLock(m_taskLifetimeState->mutex); + return m_taskLifetimeState->task; + } + + bool Cancel(uint64_t waitTime = 0) + { + std::lock_guard lock(m_mutex); + if (m_taskLifetimeState == nullptr) + { return true; } + + // Keep task destruction serialized with the dispatcher's pointer + // lookup so this address cannot be freed and reused for a different + // task between the lookup here and Cancel(). A recursive mutex is + // required because dispatchers may delete queued tasks synchronously + // from Cancel(), re-entering TaskCall's destructor on this thread. + std::lock_guard lifetimeLock(m_taskLifetimeState->mutex); + MAT::Task* task = m_taskLifetimeState->task; + if (task) + { + bool result = (m_taskDispatcher != nullptr) && (m_taskDispatcher->Cancel(task, waitTime)); + return result || (m_taskLifetimeState->task == nullptr); + } + return true; } + + private: + mutable std::mutex m_mutex; + std::shared_ptr m_taskLifetimeState; + MAT::ITaskDispatcher* m_taskDispatcher = nullptr; }; template @@ -121,9 +178,20 @@ namespace PAL_NS_BEGIN { DeferredCallbackHandle scheduleTask(MAT::ITaskDispatcher* taskDispatcher, unsigned delayMs, TObject* obj, void (TObject::*func)(TFuncArgs...), TPassedArgs&&... args) { auto bound = std::bind(std::mem_fn(func), obj, std::forward(args)...); - auto task = new detail::TaskCall(bound, getMonotonicTimeMs() + (int64_t)delayMs); + auto taskLifetimeState = std::make_shared(); + auto task = new detail::TaskCall( + bound, + getMonotonicTimeMs() + (int64_t)delayMs, + taskLifetimeState); taskDispatcher->Queue(task); - return DeferredCallbackHandle(task, taskDispatcher); + { + std::lock_guard lock(taskLifetimeState->mutex); + if (taskLifetimeState->task == nullptr) + { + return DeferredCallbackHandle(); + } + } + return DeferredCallbackHandle(taskLifetimeState, taskDispatcher); } template @@ -135,4 +203,3 @@ namespace PAL_NS_BEGIN { } PAL_NS_END #endif - diff --git a/lib/pal/TaskDispatcher_CAPI.cpp b/lib/pal/TaskDispatcher_CAPI.cpp index e75ee1924..cae75100c 100644 --- a/lib/pal/TaskDispatcher_CAPI.cpp +++ b/lib/pal/TaskDispatcher_CAPI.cpp @@ -45,6 +45,7 @@ namespace PAL_NS_BEGIN { (*m_task)(); } catch (const std::exception& ex) { + UNREFERENCED_PARAMETER(ex); LOG_ERROR("Unhandled exception in CAPI task: %s", ex.what()); } catch (...) { @@ -164,4 +165,3 @@ namespace PAL_NS_BEGIN { } } PAL_NS_END - diff --git a/lib/pal/WorkerThread.cpp b/lib/pal/WorkerThread.cpp index 3adfb9e61..0c4006410 100644 --- a/lib/pal/WorkerThread.cpp +++ b/lib/pal/WorkerThread.cpp @@ -229,8 +229,13 @@ namespace PAL_NS_BEGIN { } if (item->Type == MAT::Task::Shutdown) { + { + LOCKGUARD(self->m_lock); + if (self->m_itemInProgress == item.get()) { + self->m_itemInProgress = nullptr; + } + } item.reset(); - self->m_itemInProgress = nullptr; break; } @@ -248,19 +253,28 @@ namespace PAL_NS_BEGIN { (*item)(); } catch (const std::exception& ex) { + UNREFERENCED_PARAMETER(ex); LOG_ERROR("Unhandled exception in worker task: %s", ex.what()); } catch (...) { LOG_ERROR("Unhandled non-standard exception in worker task"); } - self->m_itemInProgress = nullptr; } if (item) { item->Type = MAT::Task::Done; - item = nullptr; } } + { + LOCKGUARD(self->m_lock); + if (self->m_itemInProgress == item.get()) { + self->m_itemInProgress = nullptr; + } + } + // Task destruction may synchronize with a cancellation caller. + // Never run it while holding m_execution_mutex, which Cancel() + // waits on while that caller owns the task lifetime lock. + item = nullptr; } } }; @@ -275,4 +289,3 @@ namespace PAL_NS_BEGIN { } PAL_NS_END #endif - diff --git a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp index f01992940..3c8fe6baf 100644 --- a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp @@ -13,16 +13,12 @@ MATSDK_LOG_INST_COMPONENT_NS("DeviceInfo", "Win32 Desktop Device Information") -#include #include #include #include #include #include -#include -#include - #pragma comment(lib, "iphlpapi.lib") #pragma comment(lib, "AdvAPI32.Lib") @@ -149,4 +145,3 @@ namespace PAL_NS_BEGIN { } } PAL_NS_END - diff --git a/lib/pal/desktop/desktop.vcxitems b/lib/pal/desktop/desktop.vcxitems index 0d8ae8def..45c6804cd 100644 --- a/lib/pal/desktop/desktop.vcxitems +++ b/lib/pal/desktop/desktop.vcxitems @@ -13,10 +13,23 @@ - + + + + + + HAVE_MAT_WININET_HTTP_CLIENT;%(PreprocessorDefinitions) + + + + + HAVE_MAT_WINHTTP_HTTP_CLIENT;%(PreprocessorDefinitions) + + - + + ..\..;..\..\include;$(ProjectDir);$(GeneratedFilesDir);$(IntDir);%(AdditionalIncludeDirectories);$(WindowsSDK_IncludePath) diff --git a/lib/pal/desktop/desktop.vcxitems.filters b/lib/pal/desktop/desktop.vcxitems.filters index a1d6dd857..b3756c63e 100644 --- a/lib/pal/desktop/desktop.vcxitems.filters +++ b/lib/pal/desktop/desktop.vcxitems.filters @@ -1,14 +1,16 @@  - + + - + + diff --git a/lib/utils/annex_k.hpp b/lib/utils/annex_k.hpp index 5aa4b73af..98df5ebf2 100644 --- a/lib/utils/annex_k.hpp +++ b/lib/utils/annex_k.hpp @@ -7,9 +7,8 @@ #include #include #include -#ifndef _MSC_VER #include -#else +#ifdef _MSC_VER #include #endif @@ -47,21 +46,24 @@ class BoundCheckFunctions private: static bool oneds_buffer_region_overlap(const char *buffer1, size_t buffer1_len, const char *buffer2, size_t buffer2_len) noexcept { - if (buffer2 >= buffer1) + // Compare half-open address ranges without pointer arithmetic: the + // arguments may refer to different objects, and invalid lengths must not + // wrap an end address before the overlap check. + if (buffer1_len == 0 || buffer2_len == 0) { - if (buffer1 + buffer1_len - 1 > buffer2 ) - { - return true; - } + return false; } - else + + uintptr_t begin1 = reinterpret_cast(buffer1); + uintptr_t begin2 = reinterpret_cast(buffer2); + if (buffer1_len > UINTPTR_MAX - begin1 || buffer2_len > UINTPTR_MAX - begin2) { - if (buffer2 + buffer2_len - 1 > buffer1) - { - return true; - } + return true; } - return false; + + uintptr_t end1 = begin1 + buffer1_len; + uintptr_t end2 = begin2 + buffer2_len; + return begin1 < end2 && begin2 < end1; } public: @@ -147,12 +149,16 @@ static errno_t oneds_strncpy_s(char * restrict dest, rsize_t destsz, const char // In case of error, the entire destination range [dest, dest+destsz) is zeroed out // (if both dest and destsz are valid)) +// +// NOTE: the constraint checks below are performed here rather than delegated to +// the platform's Annex K / CRT memcpy_s. On MSVC the CRT memcpy_s reports a +// constraint violation through the invalid parameter handler, whose default +// behaviour terminates the process (__fastfail / STATUS_STACK_BUFFER_OVERRUN) +// instead of returning EINVAL. Validating first keeps the documented +// "return EINVAL and zero the destination" contract on every platform. static errno_t oneds_memcpy_s( void *restrict dest, rsize_t destsz, const void *restrict src, rsize_t count ) noexcept { -#if (defined __STDC_LIB_EXT1__) || ( defined _MSC_VER) - return memcpy_s(dest, destsz, src, count); -#else if (dest == NULL) { return EINVAL; @@ -176,13 +182,8 @@ static errno_t oneds_memcpy_s( void *restrict dest, rsize_t destsz, memset(dest, 0, destsz); return EINVAL; } - void *result = memcpy(dest, src, count); - if (result == (void *)NULL) - { - return -1; - } + memcpy(dest, src, count); return 0; -#endif } }; } diff --git a/tests/common/MockIOfflineStorage.hpp b/tests/common/MockIOfflineStorage.hpp index d0bae4118..4c37df7d4 100644 --- a/tests/common/MockIOfflineStorage.hpp +++ b/tests/common/MockIOfflineStorage.hpp @@ -14,7 +14,7 @@ namespace testing { #pragma clang diagnostic ignored "-Winconsistent-missing-override" // GMock MOCK_METHOD* macros don't use override. #endif -class MockIOfflineStorage : public MAT::IOfflineStorage { +class MockIOfflineStorage : public MAT::IOfflineStorageModule { public: MockIOfflineStorage(); virtual ~MockIOfflineStorage(); @@ -46,4 +46,3 @@ class MockIOfflineStorage : public MAT::IOfflineStorage { #endif } // namespace testing - diff --git a/tests/common/Reactor.cpp b/tests/common/Reactor.cpp index 6cb55f13d..ddc82d2a2 100644 --- a/tests/common/Reactor.cpp +++ b/tests/common/Reactor.cpp @@ -179,23 +179,88 @@ namespace SocketTools { void Reactor::onThread() { LOG_INFO("Reactor: Thread started"); +#ifdef _WIN32 + size_t nextEventChunk = 0; +#endif while(!shouldTerminate()) { #ifdef _WIN32 - DWORD dwResult = ::WSAWaitForMultipleEvents(static_cast(m_events.size()), m_events.data(), FALSE, 500, FALSE); + if (m_events.empty()) + { + ::Sleep(10); + continue; + } + + const size_t maxEvents = WSA_MAXIMUM_WAIT_EVENTS; + const size_t chunkCount = (m_events.size() + maxEvents - 1) / maxEvents; + if (nextEventChunk >= chunkCount) + { + nextEventChunk = 0; + } + + DWORD dwResult = WSA_WAIT_TIMEOUT; + size_t selectedChunkStart = 0; + bool waitFailed = false; + for (size_t offset = 0; offset < chunkCount; ++offset) + { + const size_t chunk = (nextEventChunk + offset) % chunkCount; + const size_t chunkStart = chunk * maxEvents; + const DWORD chunkSize = static_cast( + std::min(maxEvents, m_events.size() - chunkStart)); + dwResult = ::WSAWaitForMultipleEvents( + chunkSize, m_events.data() + chunkStart, FALSE, 0, FALSE); + if (dwResult == WSA_WAIT_FAILED) + { + LOG_ERROR("WSAWaitForMultipleEvents failed: %d", ::WSAGetLastError()); + waitFailed = true; + continue; + } + if (dwResult != WSA_WAIT_TIMEOUT) + { + selectedChunkStart = chunkStart; + nextEventChunk = (chunk + 1) % chunkCount; + break; + } + } + + if (dwResult == WSA_WAIT_TIMEOUT) + { + const size_t chunkStart = nextEventChunk * maxEvents; + const DWORD chunkSize = static_cast( + std::min(maxEvents, m_events.size() - chunkStart)); + dwResult = ::WSAWaitForMultipleEvents( + chunkSize, m_events.data() + chunkStart, FALSE, 50, FALSE); + selectedChunkStart = chunkStart; + nextEventChunk = (nextEventChunk + 1) % chunkCount; + } + if (dwResult == WSA_WAIT_TIMEOUT) { continue; } + if (dwResult == WSA_WAIT_FAILED) + { + LOG_ERROR("WSAWaitForMultipleEvents failed: %d", ::WSAGetLastError()); + if (waitFailed) + { + ::Sleep(10); + } + continue; + } - assert(dwResult <= WSA_WAIT_EVENT_0 + m_events.size()); - int index = dwResult - WSA_WAIT_EVENT_0; + const size_t index = selectedChunkStart + + static_cast(dwResult - WSA_WAIT_EVENT_0); + if (index >= m_events.size() || index >= m_sockets.size()) + { + LOG_ERROR("WSAWaitForMultipleEvents returned invalid index %zu", index); + continue; + } Socket socket = m_sockets[index].socket; int flags = m_sockets[index].flags; WSANETWORKEVENTS ne; ::WSAEnumNetworkEvents(socket, m_events[index], &ne); - LOG_TRACE("Reactor: Handling socket 0x%x (index %d) with active flags 0x%x (armed 0x%x)", + LOG_TRACE("Reactor: Handling socket 0x%x (index %zu) with active flags 0x%x (armed 0x%x)", static_cast(socket), index, ne.lNetworkEvents, flags); if ((flags & Readable) && (ne.lNetworkEvents & FD_READ)) @@ -321,4 +386,3 @@ namespace SocketTools { }; } - diff --git a/tests/common/SocketTools.hpp b/tests/common/SocketTools.hpp index 0bfe350d3..fca85c110 100644 --- a/tests/common/SocketTools.hpp +++ b/tests/common/SocketTools.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -409,7 +410,7 @@ class Thread { private: std::thread m_thread; - volatile bool m_terminate { false }; + std::atomic m_terminate { false }; protected: Thread() @@ -437,7 +438,7 @@ class Thread bool shouldTerminate() const { - return m_terminate; + return m_terminate.load(); } virtual void onThread() = 0; @@ -466,4 +467,3 @@ struct SocketData } #endif - diff --git a/tests/functests/APITest.cpp b/tests/functests/APITest.cpp index baea0112e..e2fb39c93 100644 --- a/tests/functests/APITest.cpp +++ b/tests/functests/APITest.cpp @@ -15,7 +15,12 @@ #include #include +#include #include +#include +#include +#include +#include #include "PayloadDecoder.hpp" @@ -210,6 +215,32 @@ class TestDebugEventListener : public DebugEventListener { } }; +class HttpResponseWaiter final : public IHttpResponseCallback { +public: + void OnHttpResponse(IHttpResponse* response) override + { + std::lock_guard lock(m_mutex); + m_response.reset(response); + m_cv.notify_all(); + } + + void OnHttpStateEvent(HttpStateEvent, void*, size_t) override + { + } + + std::unique_ptr WaitForResponse(std::chrono::seconds timeout) + { + std::unique_lock lock(m_mutex); + m_cv.wait_for(lock, timeout, [this]() { return m_response != nullptr; }); + return std::move(m_response); + } + +private: + std::mutex m_mutex; + std::condition_variable m_cv; + std::unique_ptr m_response; +}; + // Keep requests in flight until teardown cancels them, then simulate a connection // reset while honoring IHttpClient's exactly-once callback contract. class NetworkFailureHttpClient final : public IHttpClient @@ -673,38 +704,32 @@ constexpr static unsigned MAX_THREADS = 25; /// The configuration. void StressUploadLockMultiThreaded(ILogConfiguration& config) { - std::srand(static_cast(std::time(nullptr))); TestDebugEventListener debugListener; addAllListeners(debugListener); size_t numIterations = MAX_ITERATIONS_MT; - std::mutex m_threads_mtx; - std::atomic threadCount(0); - while (numIterations--) { ILogger *result = LogManager::Initialize(TEST_TOKEN, config); - // Keep spawning UploadNow threads while the main thread is trying to perform - // Initialize and Teardown, but no more than MAX_THREADS at a time. + std::vector uploadThreads; + uploadThreads.reserve(MAX_THREADS); for (size_t i = 0; i < MAX_THREADS; i++) { - if (threadCount++ < MAX_THREADS) + uploadThreads.emplace_back([]() { - auto t = std::thread([&]() - { - std::this_thread::yield(); - LogManager::UploadNow(); - const auto randTimeSub2ms = std::rand() % 2; - PAL::sleep(randTimeSub2ms); - threadCount--; - }); - t.detach(); - } - }; + std::this_thread::yield(); + LogManager::UploadNow(); + PAL::sleep(0); + }); + } EventProperties props = testing::CreateSampleEvent("event_name", EventPriority_Normal); result->LogEvent(props); LogManager::FlushAndTeardown(); + for (auto& uploadThread : uploadThreads) + { + uploadThread.join(); + } } removeAllListeners(debugListener); } @@ -1252,8 +1277,54 @@ TEST(APITest, LogManager_BadStoragePath_Test) } -#ifdef HAVE_MAT_WININET_HTTP_CLIENT -/* This test requires WinInet HTTP client */ +#if defined(_WIN32) && defined(HAVE_MAT_DEFAULT_HTTP_CLIENT) +TEST(APITest, WindowsHttpTransport_MsRoot_Check) +{ + auto sendRequest = [](bool enforceMsRoot) { + HttpResponseWaiter callback; + auto client = HttpClientFactory::Create(); +#if defined(HAVE_MAT_WININET_HTTP_CLIENT) + auto windowsClient = dynamic_cast(client.get()); +#elif defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) + auto windowsClient = dynamic_cast(client.get()); +#else +#error A Windows HTTP transport must be selected. +#endif + EXPECT_NE(windowsClient, nullptr); + if (windowsClient == nullptr) + { + return std::unique_ptr(); + } + windowsClient->SetMsRootCheck(enforceMsRoot); + + std::unique_ptr request(client->CreateRequest()); + request->SetMethod("POST"); + request->SetUrl("https://mobile.events.data.microsoft.com/OneCollector/1.0/"); + std::vector body {'{', '}'}; + request->SetBody(body); + client->SendRequestAsync(request.release(), &callback); + + auto response = callback.WaitForResponse(std::chrono::seconds(10)); + if (response == nullptr) + { + client->CancelAllRequests(); + response = callback.WaitForResponse(std::chrono::seconds(2)); + } + client.reset(); + return response; + }; + + auto accepted = sendRequest(false); + ASSERT_NE(accepted, nullptr); + EXPECT_EQ(accepted->GetResult(), HttpResult_OK); + + auto rejected = sendRequest(true); + ASSERT_NE(rejected, nullptr); + EXPECT_EQ(rejected->GetResult(), HttpResult_NetworkFailure); + EXPECT_EQ(rejected->GetStatusCode(), 0u); +} + +/* This test verifies the certificate policy used by either Windows HTTP transport. */ TEST(APITest, LogConfiguration_MsRoot_Check) { TestDebugEventListener debugListener; @@ -1283,13 +1354,21 @@ TEST(APITest, LogConfiguration_MsRoot_Check) debugListener.reset(); addAllListeners(debugListener); logger->LogEvent("fooBar"); + LogManager::UploadNow(); + const auto deadline = PAL::getMonotonicTimeMs() + 10000; + while (PAL::getMonotonicTimeMs() < deadline && + debugListener.numHttpOK.load() == 0 && + debugListener.numHttpError.load() == 0) + { + PAL::sleep(50); + } LogManager::FlushAndTeardown(); removeAllListeners(debugListener); - // Connection is a best-effort, occasionally we can't connect, - // but we MUST NOT connect to end-point that doesn't have the - // right cert. - EXPECT_LE(debugListener.numHttpOK, expectedHttpCount); + // The successful cases establish that the runner can reach both + // endpoints, so the rejected case cannot pass merely because external + // networking is unavailable. + EXPECT_EQ(debugListener.numHttpOK.load(), expectedHttpCount); } } #endif diff --git a/tests/functests/BasicFuncTests.cpp b/tests/functests/BasicFuncTests.cpp index bc879d3e6..b86c651bd 100644 --- a/tests/functests/BasicFuncTests.cpp +++ b/tests/functests/BasicFuncTests.cpp @@ -128,6 +128,7 @@ class BasicFuncTests : public ::testing::Test, protected: std::mutex mtx_requests; std::vector receivedRequests; + std::string serverBaseAddress; std::string serverAddress; HttpServer server; @@ -155,7 +156,8 @@ class BasicFuncTests : public ::testing::Test, int port = server.addListeningPort(HTTP_PORT); std::ostringstream os; os << "127.0.0.1:" << port; - serverAddress = "http://" + os.str() + "/simple/"; + serverBaseAddress = "http://" + os.str(); + serverAddress = serverBaseAddress + "/simple/"; server.setServerName(os.str()); server.addHandler("/simple/", *this); server.addHandler("/slow/", *this); @@ -833,9 +835,8 @@ TEST_F(BasicFuncTests, restartRecoversEventsFromStorage) LogManager::SetTransmitProfile(TransmitProfile_RealTime); LogManager::UploadNow(); - // 1st request for realtime event - waitForEvents(10, 5); // start, first_event, second_event, ongoing, stop, start, fooEvent - // we drop two of the events during pause, though. + // A graceful paused shutdown persists every pending event for restart. + waitForEvents(10, 7); EXPECT_GE(receivedRequests.size(), (size_t)1); if (receivedRequests.size() != 0) { @@ -945,10 +946,10 @@ TEST_F(BasicFuncTests, sendMetaStatsOnStart) LogManager::ResumeTransmission(); // ? LogManager::SetTransmitProfile(TransmitProfile_RealTime); LogManager::UploadNow(); - waitForEvents(5, 4); // (start + stop) + (2 events + start) + waitForEvents(5, 6); auto r2 = records(); - ASSERT_GE(r2.size(), (size_t)4); // (start + stop) + (2 events + start) + ASSERT_GE(r2.size(), (size_t)6); for (const auto &evt : { event1, event2 }) { @@ -1260,6 +1261,7 @@ TEST_F(BasicFuncTests, killSwitchWorks) myLogger->LogEvent(event2); } // Expect all events to be dropped + EXPECT_TRUE(listener.waitForAtLeast(listener.numDropped, 100, 10000)); EXPECT_EQ(uint32_t { 100 }, listener.numDropped); LogManager::FlushAndTeardown(); @@ -1364,7 +1366,10 @@ TEST_F(BasicFuncTests, sendManyRequestsAndCancel) configuration[CFG_INT_RAM_QUEUE_SIZE] = 4096 * 20; configuration[CFG_STR_CACHE_FILE_PATH] = TEST_STORAGE_FILENAME; configuration[CFG_MAP_HTTP][CFG_BOOL_HTTP_COMPRESSION] = true; - configuration[CFG_STR_COLLECTOR_URL] = COLLECTOR_URL_PROD; + // Use the fixture's local slow endpoint so cancellation does not depend + // on how the CI runner handles connections to an unused port. + const std::string slowCollectorUrl = serverBaseAddress + "/slow/"; + configuration[CFG_STR_COLLECTOR_URL] = slowCollectorUrl.c_str(); configuration[CFG_INT_MAX_TEARDOWN_TIME] = (int64_t)(i % 2); configuration[CFG_INT_TRACE_LEVEL_MASK] = 0; configuration[CFG_INT_TRACE_LEVEL_MIN] = ACTTraceLevel_Warn; diff --git a/tests/functests/FuncTests.vcxproj b/tests/functests/FuncTests.vcxproj index f5977c7c7..79a8d84db 100644 --- a/tests/functests/FuncTests.vcxproj +++ b/tests/functests/FuncTests.vcxproj @@ -157,7 +157,7 @@ /machine:X86 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -206,9 +206,13 @@ /machine:X86 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) - No + Debug + true + true + true + $(OutDir)$(TargetName).map %(IgnoreSpecificDefaultLibraries) Console @@ -254,7 +258,7 @@ /machine:X64 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -304,7 +308,7 @@ /machine:ARM64 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -353,7 +357,7 @@ /machine:X64 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) Debug %(IgnoreSpecificDefaultLibraries) @@ -400,7 +404,7 @@ /machine:ARM64 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) Debug %(IgnoreSpecificDefaultLibraries) @@ -413,6 +417,16 @@ true + + + HAVE_MAT_WININET_HTTP_CLIENT;%(PreprocessorDefinitions) + + + + + HAVE_MAT_WINHTTP_HTTP_CLIENT;%(PreprocessorDefinitions) + + diff --git a/tests/unittests/AnnexKTests.cpp b/tests/unittests/AnnexKTests.cpp index fa74e23f5..0df63787c 100644 --- a/tests/unittests/AnnexKTests.cpp +++ b/tests/unittests/AnnexKTests.cpp @@ -30,3 +30,10 @@ TEST(AnnexKTests, memcpy_s) EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, src, dest_len + 1 ), EINVAL); EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s( dest, dest_len, (void *)((char *)dest + 1), src_len + 1 ), EINVAL); } + +TEST(AnnexKTests, memcpy_sAllowsAdjacentBuffers) +{ + char buffers[8] = {}; + + EXPECT_EQ(BoundCheckFunctions::oneds_memcpy_s(buffers, 4, buffers + 4, 4), 0); +} diff --git a/tests/unittests/HttpClientCurlTests.cpp b/tests/unittests/HttpClientCurlTests.cpp index 50a82a874..7b7909154 100644 --- a/tests/unittests/HttpClientCurlTests.cpp +++ b/tests/unittests/HttpClientCurlTests.cpp @@ -13,6 +13,15 @@ #include "http/HttpClient_Curl.hpp" #include "config/RuntimeConfig_Default.hpp" +#include +#include +#include +#include +#include +#include +#include +#include + using namespace testing; using namespace MAT; @@ -105,7 +114,9 @@ TEST_F(HttpClientCurlHeaderTests, CapturesResponseHeadersAndBody) (void)client; // Initialize curl globally before constructing the operation. CurlHttpOperation operation("GET", m_url, nullptr, requestHeaders, requestBody); - ASSERT_EQ(operation.Send(), 200L); + operation.Send(); + ASSERT_EQ(operation.GetTransportError(), CURLE_OK); + ASSERT_EQ(operation.GetHttpStatusCode(), 200L); const auto responseHeaders = operation.GetResponseHeaders(); const auto responseBody = operation.GetResponseBody(); @@ -184,6 +195,69 @@ TEST_F(HttpClientCurlTests, SetSslVerification_ConcurrentCallsNoRace) SUCCEED(); } +// --- Regression: EDEADLK self-join in ~CurlHttpOperation --- + +TEST_F(HttpClientCurlTests, SendAsync_DestroyOnWorkerThread_NoSelfJoin) +{ + struct TrackingCallback : public IHttpResponseCallback + { + std::atomic destroyEvents { 0 }; + void OnHttpResponse(IHttpResponse* response) override { delete response; } + void OnHttpStateEvent(HttpStateEvent state, void*, size_t) override + { + if (state == OnDestroy) + { + ++destroyEvents; + } + } + }; + + auto callback = std::make_shared(); + auto callbackDone = std::make_shared>(); + auto done = callbackDone->get_future(); + + auto op = std::make_shared( + "GET", "://malformed", callback.get(), m_headers, m_body, + false, 1 /*connTimeout*/, false /*sslVerify*/, ""); + + auto box = std::make_shared>(std::move(op)); + (*box)->SendAsync([box, callback, callbackDone](CurlHttpOperation&) { + box->reset(); + callbackDone->set_value(); + }); + + if (done.wait_for(std::chrono::seconds(5)) != std::future_status::ready) + { + ADD_FAILURE() << "curl worker did not finish before fixture teardown"; + std::abort(); + } + EXPECT_EQ(callback->destroyEvents.load(), 1); +} + +TEST_F(HttpClientCurlTests, SendAsync_CallbackCopyFailureStillCompletes) +{ + struct ThrowOnCopy + { + explicit ThrowOnCopy(bool& invoked) : invoked(&invoked) {} + ThrowOnCopy(ThrowOnCopy&&) = default; + ThrowOnCopy(const ThrowOnCopy&) { throw std::logic_error("copy failed"); } + void operator()(CurlHttpOperation&) const { *invoked = true; } + bool* invoked; + }; + + CurlHttpOperation op( + "GET", "://malformed", nullptr, m_headers, m_body, + false, 1 /*connTimeout*/, false /*sslVerify*/, ""); + bool callbackInvoked = false; + std::function callback { ThrowOnCopy(callbackInvoked) }; + + EXPECT_NO_THROW(op.SendAsync(std::move(callback))); + EXPECT_TRUE(callbackInvoked); + EXPECT_EQ(op.GetTransportError(), CURLE_FAILED_INIT); + EXPECT_EQ(op.GetSetupError(), CURLE_FAILED_INIT); + EXPECT_THROW(op.SendAsync(), std::logic_error); +} + // --- Response-size cap (memory-amplification DoS hardening) --- class HttpClientCurlResponseCapTests : public ::testing::Test, @@ -195,9 +269,7 @@ class HttpClientCurlResponseCapTests : public ::testing::Test, HttpClient_Curl m_client; // The client never takes ownership of the request (it only stores a raw pointer // and erases it); the fixture owns it and frees it in TearDown -- on the main - // thread, after the transfer has completed. Freeing it inside OnHttpResponse - // would destroy the CurlHttpOperation from within its own async task, whose - // destructor waits on that task (a self-join deadlock). + // thread, after the transfer has completed. std::unique_ptr m_request; std::string m_hostname; size_t m_responseBodySize {0}; diff --git a/tests/unittests/HttpClientManagerTests.cpp b/tests/unittests/HttpClientManagerTests.cpp index 287e420ed..034e37aee 100644 --- a/tests/unittests/HttpClientManagerTests.cpp +++ b/tests/unittests/HttpClientManagerTests.cpp @@ -4,10 +4,18 @@ #include "common/MockIHttpClient.hpp" #include "http/IBoundedHttpClientCancel.hpp" #include "http/HttpClientManager.hpp" +#include "pal/TaskDispatcher.hpp" #include "NullObjects.hpp" #include "ILogManager.hpp" +#include +#include +#include +#include +#include +#include + using namespace testing; using namespace MAT; @@ -31,6 +39,97 @@ class HttpClientManager4Test : public HttpClientManager { } }; +class AsyncHttpClientManager4Test : public HttpClientManager { + public: + AsyncHttpClientManager4Test(IHttpClient& httpClient) + : HttpClientManager(dummyLogManager, httpClient, *PAL::getDefaultTaskDispatcher()) + { + } + + void setCancelDrainTimeout(std::chrono::milliseconds timeout) + { + m_cancelDrainTimeout = timeout; + } +}; + +class ReentrantAsyncCompletionReceiver { + public: + void onRequestDone(EventsUploadContextPtr const& ctx) + { + if (ctx->httpRequestId == "async-reentrant-first") + { + { + std::unique_lock lock(mutex); + firstEntered = true; + cv.notify_all(); + cv.wait(lock, [this]() { return releaseFirst; }); + } + auto start = std::chrono::steady_clock::now(); + manager->cancelAllRequests(/* bestEffort */ true); + cancelDuration = std::chrono::duration_cast( + std::chrono::steady_clock::now() - start); + } + + { + std::lock_guard lock(mutex); + ++completed; + cv.notify_all(); + } + } + + HttpClientManager* manager {nullptr}; + std::mutex mutex; + std::condition_variable cv; + bool firstEntered {false}; + bool releaseFirst {false}; + size_t completed {0}; + std::chrono::milliseconds cancelDuration {0}; + RouteSink + sink {this, &ReentrantAsyncCompletionReceiver::onRequestDone}; +}; + +class BlockingAsyncCompletionReceiver { + public: + void onRequestDone(EventsUploadContextPtr const&) + { + std::unique_lock lock(mutex); + entered = true; + cv.notify_all(); + cv.wait(lock, [this]() { return released; }); + } + + std::mutex mutex; + std::condition_variable cv; + bool entered {false}; + bool released {false}; + RouteSink + sink {this, &BlockingAsyncCompletionReceiver::onRequestDone}; +}; + +class QueuedHttpResponseDelivery { + public: + void deliver(IHttpResponseCallback* callback, IHttpResponse* response) + { + callback->OnHttpResponse(response); + { + std::lock_guard lock(mutex); + ++completed; + } + cv.notify_all(); + } + + bool waitFor(size_t count) + { + std::unique_lock lock(mutex); + return cv.wait_for(lock, std::chrono::seconds(5), + [this, count]() { return completed == count; }); + } + + std::mutex mutex; + std::condition_variable cv; + size_t completed {0}; +}; + class HttpClientManagerTests : public StrictMock { protected: MockIHttpClient httpClientMock; @@ -87,6 +186,219 @@ TEST_F(HttpClientManagerTests, HandlesRequestFlow) EXPECT_THAT(ctx->durationMs, Gt(199)); } +TEST_F(HttpClientManagerTests, ThrowingRequestDoneStillDrainsCallback) +{ + auto ctx = std::make_shared(); + ctx->httpRequest = new SimpleHttpRequest("throwing-request-done"); + ctx->httpRequestId = ctx->httpRequest->GetId(); + ctx->recordIdsAndTenantIds["r1"] = "t1"; + ctx->latency = EventLatency_Normal; + ctx->packageIds["tenant1-token"] = 0; + + IHttpResponseCallback* callback = nullptr; + EXPECT_CALL(httpClientMock, SendRequestAsync(ctx->httpRequest, _)) + .WillOnce(SaveArg<1>(&callback)); + hcm.sendRequest(ctx); + ASSERT_THAT(callback, NotNull()); + + EXPECT_CALL(*this, resultRequestDone(ctx)) + .WillOnce(Throw(std::runtime_error("listener failed"))); + + EXPECT_NO_THROW(callback->OnHttpResponse(new SimpleHttpResponse("throwing-request-done"))); + EXPECT_THAT(hcm.requestCount(), 0u); +} + +TEST_F(HttpClientManagerTests, RequestDoneCanCancelAllRequests) +{ + SimpleHttpRequest* req = new SimpleHttpRequest("reentrant-cancel"); + auto ctx = std::make_shared(); + ctx->httpRequestId = req->GetId(); + ctx->httpRequest = req; + ctx->recordIdsAndTenantIds["r1"] = "t1"; + ctx->latency = EventLatency_Normal; + ctx->packageIds["tenant1-token"] = 0; + + IHttpResponseCallback* callback = nullptr; + EXPECT_CALL(httpClientMock, SendRequestAsync(ctx->httpRequest, _)) + .WillOnce(SaveArg<1>(&callback)); + hcm.sendRequest(ctx); + ASSERT_THAT(callback, NotNull()); + + EXPECT_CALL(*this, resultRequestDone(ctx)) + .WillOnce(Invoke([this](EventsUploadContextPtr const&) { + hcm.cancelAllRequests(); + })); + callback->OnHttpResponse(new SimpleHttpResponse("reentrant-cancel")); + + EXPECT_THAT(hcm.requestCount(), 0u); +} + +TEST_F(HttpClientManagerTests, ConcurrentRequestDoneCallbacksCanCancelAllRequests) +{ + std::vector callbacks; + std::vector contexts; + for (size_t i = 0; i < 2; ++i) + { + auto ctx = std::make_shared(); + ctx->httpRequest = new SimpleHttpRequest( + "concurrent-reentrant-cancel-" + std::to_string(i)); + ctx->httpRequestId = ctx->httpRequest->GetId(); + ctx->recordIdsAndTenantIds["r1"] = "t1"; + ctx->latency = EventLatency_Normal; + ctx->packageIds["tenant1-token"] = 0; + + IHttpResponseCallback* callback = nullptr; + EXPECT_CALL(httpClientMock, SendRequestAsync(ctx->httpRequest, _)) + .WillOnce(SaveArg<1>(&callback)); + hcm.sendRequest(ctx); + ASSERT_THAT(callback, NotNull()); + callbacks.push_back(callback); + contexts.push_back(std::move(ctx)); + } + + std::mutex barrierMutex; + std::condition_variable barrierCv; + size_t callbacksEntered = 0; + EXPECT_CALL(*this, resultRequestDone(_)) + .Times(2) + .WillRepeatedly(Invoke([this, &barrierMutex, &barrierCv, &callbacksEntered]( + EventsUploadContextPtr const&) { + { + std::unique_lock lock(barrierMutex); + ++callbacksEntered; + barrierCv.notify_all(); + barrierCv.wait_for(lock, std::chrono::seconds(5), + [&callbacksEntered]() { return callbacksEntered == 2; }); + } + hcm.cancelAllRequests(); + })); + + std::thread first([&callbacks]() { + callbacks[0]->OnHttpResponse( + new SimpleHttpResponse("concurrent-reentrant-cancel-0")); + }); + std::thread second([&callbacks]() { + callbacks[1]->OnHttpResponse( + new SimpleHttpResponse("concurrent-reentrant-cancel-1")); + }); + first.join(); + second.join(); + + EXPECT_THAT(callbacksEntered, 2u); + EXPECT_THAT(hcm.requestCount(), 0u); +} + +TEST(HttpClientManagerAsyncTests, ReentrantCancelDoesNotBlockQueuedCallbacks) +{ + MockIHttpClient httpClient; + AsyncHttpClientManager4Test manager(httpClient); + manager.setCancelDrainTimeout(std::chrono::seconds(1)); + ReentrantAsyncCompletionReceiver receiver; + receiver.manager = &manager; + manager.requestDone >> receiver.sink; + + std::vector callbacks; + for (const char* id : {"async-reentrant-first", "async-reentrant-second"}) + { + auto ctx = std::make_shared(); + ctx->httpRequest = new SimpleHttpRequest(id); + ctx->httpRequestId = id; + ctx->recordIdsAndTenantIds["r1"] = "t1"; + ctx->latency = EventLatency_Normal; + ctx->packageIds["tenant1-token"] = 0; + + IHttpResponseCallback* callback = nullptr; + EXPECT_CALL(httpClient, SendRequestAsync(ctx->httpRequest, _)) + .WillOnce(SaveArg<1>(&callback)); + manager.sendRequest(ctx); + ASSERT_THAT(callback, NotNull()); + callbacks.push_back(callback); + } + + EXPECT_CALL(httpClient, CancelRequestAsync("async-reentrant-first")); + EXPECT_CALL(httpClient, CancelRequestAsync("async-reentrant-second")); + + QueuedHttpResponseDelivery delivery; + auto dispatcher = PAL::getDefaultTaskDispatcher(); + PAL::scheduleTask( + dispatcher.get(), 0, &delivery, &QueuedHttpResponseDelivery::deliver, + callbacks[0], new SimpleHttpResponse("async-reentrant-first")); + { + std::unique_lock lock(receiver.mutex); + ASSERT_TRUE(receiver.cv.wait_for(lock, std::chrono::seconds(5), + [&receiver]() { return receiver.firstEntered; })); + } + + // This completion is now queued behind the first one on PAL's default + // single-thread dispatcher. + PAL::scheduleTask( + dispatcher.get(), 0, &delivery, &QueuedHttpResponseDelivery::deliver, + callbacks[1], new SimpleHttpResponse("async-reentrant-second")); + { + std::lock_guard lock(receiver.mutex); + receiver.releaseFirst = true; + } + receiver.cv.notify_all(); + + { + std::unique_lock lock(receiver.mutex); + ASSERT_TRUE(receiver.cv.wait_for(lock, std::chrono::seconds(5), + [&receiver]() { return receiver.completed == 2; })); + } + EXPECT_THAT(receiver.cancelDuration, Lt(std::chrono::milliseconds(500))); + EXPECT_THAT(manager.requestCount(), 0u); + EXPECT_TRUE(delivery.waitFor(2)); +} + +TEST(HttpClientManagerAsyncTests, DestructorWaitsForActiveCallback) +{ + MockIHttpClient httpClient; + auto manager = std::make_unique(httpClient); + BlockingAsyncCompletionReceiver receiver; + manager->requestDone >> receiver.sink; + + auto ctx = std::make_shared(); + ctx->httpRequest = new SimpleHttpRequest("async-destructor"); + ctx->httpRequestId = ctx->httpRequest->GetId(); + ctx->recordIdsAndTenantIds["r1"] = "t1"; + ctx->latency = EventLatency_Normal; + ctx->packageIds["tenant1-token"] = 0; + + IHttpResponseCallback* callback = nullptr; + EXPECT_CALL(httpClient, SendRequestAsync(ctx->httpRequest, _)) + .WillOnce(SaveArg<1>(&callback)); + manager->sendRequest(ctx); + ASSERT_THAT(callback, NotNull()); + QueuedHttpResponseDelivery delivery; + auto dispatcher = PAL::getDefaultTaskDispatcher(); + PAL::scheduleTask( + dispatcher.get(), 0, &delivery, &QueuedHttpResponseDelivery::deliver, + callback, new SimpleHttpResponse("async-destructor")); + + { + std::unique_lock lock(receiver.mutex); + ASSERT_TRUE(receiver.cv.wait_for(lock, std::chrono::seconds(5), + [&receiver]() { return receiver.entered; })); + } + + std::atomic destructorReturned {false}; + std::thread destroyer([&manager, &destructorReturned]() { + manager.reset(); + destructorReturned.store(true); + }); + + PAL::sleep(100); + EXPECT_FALSE(destructorReturned.load()); + { + std::lock_guard lock(receiver.mutex); + receiver.released = true; + } + receiver.cv.notify_all(); + destroyer.join(); + EXPECT_TRUE(destructorReturned.load()); + EXPECT_TRUE(delivery.waitFor(1)); +} + // Regression test: cancelAllRequests() must not spin/hang forever // when an in-flight callback never drains (e.g. the dispatcher or HTTP stack is // stalled). It waits for the drain via a condition variable, bounded by a timeout. diff --git a/tests/unittests/HttpClientTests.cpp b/tests/unittests/HttpClientTests.cpp index 4b17bcce5..a1c2c6f58 100644 --- a/tests/unittests/HttpClientTests.cpp +++ b/tests/unittests/HttpClientTests.cpp @@ -2,14 +2,23 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // SPDX-License-Identifier: Apache-2.0 // -#ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT #ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers #endif +// Must precede the guard below: HAVE_MAT_DEFAULT_HTTP_CLIENT comes from the SDK +// configuration header, so testing it before including this silently compiles +// the whole suite away (same ordering as HttpClientCurlTests.cpp). +#include "mat/config.h" + +#ifdef HAVE_MAT_DEFAULT_HTTP_CLIENT #include "common/Common.hpp" #include "common/HttpServer.hpp" #include "http/HttpClientFactory.hpp" +#include +#include +#include + using namespace testing; using namespace MAT; @@ -29,6 +38,25 @@ class HttpClientTests : public ::testing::Test, enum RequestState { Planned, Sent, Processed, Done }; std::vector _countedRequests; std::mutex _lock; + std::condition_variable _responseCv; + std::condition_variable _blockedRequestCv; + std::mutex _blockedRequestLock; + bool _blockedRequestReceived {false}; + bool _releaseBlockedRequest {false}; + bool _cancelOnConnecting {false}; + bool _blockStateEvent {false}; + HttpStateEvent _stateEventToBlock {OnConnecting}; + bool _stateEventEntered {false}; + bool _releaseConnecting {false}; + bool _blockResponseCallback {false}; + bool _responseCallbackEntered {false}; + bool _releaseResponseCallback {false}; + std::atomic _cancelAllOnResponse {0}; + std::atomic _synchronizeCancelAllResponses {false}; + size_t _cancelAllResponsesEntered {0}; + std::atomic _sendRequestOnResponse {false}; + bool _destroyClientOnConnecting {false}; + std::string _lateRequestId; public: HttpClientTests() @@ -59,6 +87,9 @@ class HttpClientTests : public ::testing::Test, _server.addHandler("/simple/", *this); _server.addHandler("/echo/", *this); _server.addHandler("/count/", *this); + _server.addHandler("/block/", *this); + _server.addHandler("/large/", *this); + _server.addHandler("/redirect/", *this); _server.start(); Clear(); @@ -66,12 +97,30 @@ class HttpClientTests : public ::testing::Test, virtual void TearDown() override { + { + std::lock_guard lock(_blockedRequestLock); + _releaseBlockedRequest = true; + _releaseConnecting = true; + _releaseResponseCallback = true; + } + _blockedRequestCv.notify_all(); _server.stop(); _client.reset(); Clear(); } protected: + // Deterministic filler whose every byte depends on its offset, so a + // truncated, duplicated or misordered chunk cannot pass unnoticed. + static std::string LargePayload(size_t size) + { + std::string payload(size, '\0'); + for (size_t i = 0; i < size; ++i) { + payload[i] = static_cast('a' + (i % 26)); + } + return payload; + } + virtual int onHttpRequest(HttpServer::Request const& request, HttpServer::Response& inResponse) override { if (request.uri.substr(0, 8) == "/simple/") { @@ -87,6 +136,29 @@ class HttpClientTests : public ::testing::Test, return 200; } + if (request.uri == "/block/") { + { + std::lock_guard lock(_blockedRequestLock); + _blockedRequestReceived = true; + } + _blockedRequestCv.notify_all(); + std::unique_lock lock(_blockedRequestLock); + _blockedRequestCv.wait(lock, [this]() { return _releaseBlockedRequest; }); + return 200; + } + + if (request.uri == "/redirect/") { + inResponse.headers["Location"] = "http://" + _hostname + "/simple/200"; + return 302; + } + + if (request.uri.substr(0, 7) == "/large/") { + size_t size = static_cast(atoi(request.uri.substr(7).c_str())); + inResponse.headers["Content-Type"] = "application/octet-stream"; + inResponse.content = LargePayload(size); + return 200; + } + if (request.uri.substr(0, 7) == "/count/") { int id = atoi(request.uri.substr(7).c_str()); if (id >= 0 && static_cast(id) < _countedRequests.size()) { @@ -117,10 +189,77 @@ class HttpClientTests : public ::testing::Test, virtual void OnHttpResponse(IHttpResponse* inResponse) override { + if (_sendRequestOnResponse.exchange(false)) + { + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/echo/"); + { + std::lock_guard lock(_blockedRequestLock); + _lateRequestId = request->GetId(); + } + _client->SendRequestAsync(request.release(), this); + } + bool cancelAll = false; + size_t remaining = _cancelAllOnResponse.load(); + while (remaining != 0) + { + if (_cancelAllOnResponse.compare_exchange_weak( + remaining, remaining - 1)) + { + cancelAll = true; + break; + } + } + if (cancelAll && _synchronizeCancelAllResponses.load()) + { + std::unique_lock lock(_blockedRequestLock); + ++_cancelAllResponsesEntered; + _blockedRequestCv.notify_all(); + _blockedRequestCv.wait_for(lock, std::chrono::seconds(5), [this]() { + return _cancelAllResponsesEntered == 2; + }); + } + if (cancelAll) + { + _client->CancelAllRequests(); + } + { + std::unique_lock lock(_blockedRequestLock); + if (_blockResponseCallback) + { + _responseCallbackEntered = true; + _blockedRequestCv.notify_all(); + _blockedRequestCv.wait(lock, [this]() { + return _releaseResponseCallback; + }); + } + } std::lock_guard lock(_lock); _responses.push_back(clone(inResponse)); + _responseCv.notify_all(); } + virtual void OnHttpStateEvent(HttpStateEvent state, void*, size_t) override + { + if (_destroyClientOnConnecting && state == OnConnecting) + { + _destroyClientOnConnecting = false; + _client.reset(); + } + if (_cancelOnConnecting && state == OnConnecting) + { + _cancelOnConnecting = false; + _client->CancelAllRequests(); + } + if (_blockStateEvent && state == _stateEventToBlock) + { + std::unique_lock lock(_blockedRequestLock); + _stateEventEntered = true; + _blockedRequestCv.notify_all(); + _blockedRequestCv.wait(lock, [this]() { return _releaseConnecting; }); + _blockStateEvent = false; + } + } }; std::vector Binary(std::string const& str) @@ -128,8 +267,86 @@ std::vector Binary(std::string const& str) return std::vector(str.data(), str.data() + str.size()); } +TEST_F(HttpClientTests, HandlesCancellationWhileResponseIsInFlight) +{ + Clear(); + { + std::lock_guard lock(_blockedRequestLock); + _blockedRequestReceived = false; + _releaseBlockedRequest = false; + } + + std::unique_ptr request(_client->CreateRequest()); + std::string requestId = request->GetId(); + request->SetUrl("http://" + _hostname + "/block/"); + _client->SendRequestAsync(request.release(), this); + + { + std::unique_lock lock(_blockedRequestLock); + ASSERT_TRUE(_blockedRequestCv.wait_for(lock, std::chrono::seconds(10), + [this]() { return _blockedRequestReceived; })); + } + + _client->CancelRequestAsync(requestId); + { + std::lock_guard lock(_blockedRequestLock); + _releaseBlockedRequest = true; + } + _blockedRequestCv.notify_all(); + + std::unique_ptr response; + { + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(2), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + response.reset(_responses[0]); + _responses.clear(); + } + + EXPECT_THAT(response->GetId(), requestId); + EXPECT_THAT(response->GetResult(), HttpResult_Aborted); +} + //--- +#ifdef MATSDK_PAL_WIN32 +TEST_F(HttpClientTests, UsesConfiguredWindowsTransport) +{ +#if defined(HAVE_MAT_WININET_HTTP_CLIENT) + EXPECT_THAT(dynamic_cast(_client.get()), NotNull()); +#elif defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) + EXPECT_THAT(dynamic_cast(_client.get()), NotNull()); +#else +#error A Windows HTTP transport must be selected. +#endif +} + +TEST_F(HttpClientTests, DisablesRedirectsWhenMicrosoftRootCheckIsEnabled) +{ +#if defined(HAVE_MAT_WININET_HTTP_CLIENT) + auto windowsClient = dynamic_cast(_client.get()); +#elif defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) + auto windowsClient = dynamic_cast(_client.get()); +#else +#error A Windows HTTP transport must be selected. +#endif + ASSERT_THAT(windowsClient, NotNull()); + windowsClient->SetMsRootCheck(true); + + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/redirect/"); + _client->SendRequestAsync(request.release(), this); + + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + EXPECT_THAT(_responses[0]->GetResult(), HttpResult_OK); + EXPECT_THAT(_responses[0]->GetStatusCode(), 302u); +} +#endif + TEST_F(HttpClientTests, HandlesSimpleRequest) { Clear(); @@ -276,6 +493,293 @@ TEST_F(HttpClientTests, HandlesCancellation) _response.release(); } +#if defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) || defined(HAVE_MAT_WININET_HTTP_CLIENT) +TEST_F(HttpClientTests, HandlesCancellationFromStateEvent) +{ + Clear(); + _cancelOnConnecting = true; + + std::unique_ptr request(_client->CreateRequest()); + std::string requestId = request->GetId(); + request->SetUrl("http://" + _hostname + "/echo/"); + _client->SendRequestAsync(request.release(), this); + + std::unique_ptr response; + { + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(2), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + response.reset(_responses[0]); + _responses.clear(); + } + + EXPECT_THAT(response->GetId(), requestId); + EXPECT_THAT(response->GetResult(), HttpResult_Aborted); +} + +TEST_F(HttpClientTests, HandlesConcurrentCancellationDuringStateEvent) +{ + Clear(); + { + std::lock_guard lock(_blockedRequestLock); + _blockStateEvent = true; + _stateEventToBlock = OnSending; + _stateEventEntered = false; + _releaseConnecting = false; + } + + std::unique_ptr request(_client->CreateRequest()); + std::string requestId = request->GetId(); + request->SetUrl("http://" + _hostname + "/echo/"); + IHttpRequest* requestPtr = request.release(); + std::thread sender([this, requestPtr]() { + _client->SendRequestAsync(requestPtr, this); + }); + + { + std::unique_lock lock(_blockedRequestLock); + ASSERT_TRUE(_blockedRequestCv.wait_for(lock, std::chrono::seconds(2), + [this]() { return _stateEventEntered; })); + } + _client->CancelRequestAsync(requestId); + { + std::lock_guard lock(_lock); + EXPECT_TRUE(_responses.empty()) + << "Terminal response overlapped the active state callback"; + } + { + std::lock_guard lock(_blockedRequestLock); + _releaseConnecting = true; + } + _blockedRequestCv.notify_all(); + sender.join(); + + std::unique_ptr response; + { + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(2), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + response.reset(_responses[0]); + _responses.clear(); + } + + EXPECT_THAT(response->GetId(), requestId); + EXPECT_THAT(response->GetResult(), HttpResult_Aborted); +} +#endif + +#if defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) || defined(HAVE_MAT_WININET_HTTP_CLIENT) +TEST_F(HttpClientTests, CancelAllWaitsForActiveStateCallback) +{ + { + std::lock_guard lock(_blockedRequestLock); + _blockStateEvent = true; + _stateEventToBlock = OnSending; + } + + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/echo/"); + IHttpRequest* requestPtr = request.release(); + std::thread sender([this, requestPtr]() { + _client->SendRequestAsync(requestPtr, this); + }); + + { + std::unique_lock lock(_blockedRequestLock); + ASSERT_TRUE(_blockedRequestCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return _stateEventEntered; })); + } + + std::atomic cancelReturned {false}; + std::thread canceller([this, &cancelReturned]() { + _client->CancelAllRequests(); + cancelReturned.store(true); + }); + + PAL::sleep(100); + EXPECT_FALSE(cancelReturned.load()); + { + std::lock_guard lock(_blockedRequestLock); + _releaseConnecting = true; + } + _blockedRequestCv.notify_all(); + sender.join(); + canceller.join(); + EXPECT_TRUE(cancelReturned.load()); +} + +TEST_F(HttpClientTests, CancelAllWaitsForTerminalCallback) +{ + { + std::lock_guard lock(_blockedRequestLock); + _blockResponseCallback = true; + } + + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/simple/200"); + _client->SendRequestAsync(request.release(), this); + + { + std::unique_lock lock(_blockedRequestLock); + ASSERT_TRUE(_blockedRequestCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return _responseCallbackEntered; })); + } + + std::atomic cancelReturned {false}; + std::thread canceller([this, &cancelReturned]() { + _client->CancelAllRequests(); + cancelReturned.store(true); + }); + + PAL::sleep(100); + EXPECT_FALSE(cancelReturned.load()); + { + std::lock_guard lock(_blockedRequestLock); + _releaseResponseCallback = true; + } + _blockedRequestCv.notify_all(); + canceller.join(); + EXPECT_TRUE(cancelReturned.load()); +} + +TEST_F(HttpClientTests, TerminalCallbackCanCancelAllRequests) +{ + _cancelAllOnResponse.store(1); + + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/simple/200"); + _client->SendRequestAsync(request.release(), this); + + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return !_responses.empty(); })); + EXPECT_THAT(_responses[0]->GetResult(), HttpResult_OK); +} + +#if defined(HAVE_MAT_WINHTTP_HTTP_CLIENT) +TEST_F(HttpClientTests, SynchronousFailureCallbackCanCancelAllRequests) +{ + _cancelAllOnResponse.store(1); + + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("://invalid-url"); + _client->SendRequestAsync(request.release(), this); + + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return !_responses.empty(); })); + EXPECT_THAT(_responses[0]->GetResult(), HttpResult_LocalFailure); +} +#endif + +TEST_F(HttpClientTests, ConcurrentTerminalCallbacksCanCancelAllRequests) +{ + _synchronizeCancelAllResponses.store(true); + _cancelAllOnResponse.store(2); + + for (size_t i = 0; i < 2; ++i) + { + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/simple/200"); + _client->SendRequestAsync(request.release(), this); + } + + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(10), + [this]() { return _responses.size() == 2; })); + EXPECT_THAT(_cancelAllResponsesEntered, 2u); +} + +TEST_F(HttpClientTests, StateCallbackCanDestroyClient) +{ + _destroyClientOnConnecting = true; + + std::unique_ptr request(_client->CreateRequest()); + std::string requestId = request->GetId(); + request->SetUrl("http://" + _hostname + "/simple/200"); + _client->SendRequestAsync(request.release(), this); + + EXPECT_THAT(_client, IsNull()); + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return !_responses.empty(); })); + EXPECT_THAT(_responses[0]->GetId(), requestId); + EXPECT_THAT(_responses[0]->GetResult(), HttpResult_Aborted); +} + +TEST_F(HttpClientTests, CancelAllIncludesRequestRegisteredDuringDrain) +{ + { + std::lock_guard lock(_blockedRequestLock); + _blockStateEvent = true; + _stateEventToBlock = OnSending; + _stateEventEntered = false; + _releaseConnecting = false; + } + _sendRequestOnResponse.store(true); + + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/echo/"); + IHttpRequest* requestPtr = request.release(); + std::thread sender([this, requestPtr]() { + _client->SendRequestAsync(requestPtr, this); + }); + + { + std::unique_lock lock(_blockedRequestLock); + ASSERT_TRUE(_blockedRequestCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return _stateEventEntered; })); + } + + std::atomic cancelStarted {false}; + std::thread canceller([this, &cancelStarted]() { + cancelStarted.store(true); + _client->CancelAllRequests(); + }); + while (!cancelStarted.load()) + { + std::this_thread::yield(); + } + PAL::sleep(100); + + { + std::lock_guard lock(_blockedRequestLock); + _stateEventEntered = false; + _releaseConnecting = true; + } + _blockedRequestCv.notify_all(); + + sender.join(); + canceller.join(); + + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return _responses.size() == 2; })); + auto lateResponse = std::find_if( + _responses.begin(), _responses.end(), [this](IHttpResponse* response) { + return response->GetId() == _lateRequestId; + }); + ASSERT_THAT(lateResponse, Ne(_responses.end())); + EXPECT_THAT((*lateResponse)->GetResult(), HttpResult_Aborted); +} + +TEST_F(HttpClientTests, ClientRemainsReusableAfterCancelAll) +{ + _client->CancelAllRequests(); + + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/simple/200"); + _client->SendRequestAsync(request.release(), this); + + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(5), + [this]() { return !_responses.empty(); })); + EXPECT_THAT(_responses[0]->GetResult(), HttpResult_OK); +} +#endif + TEST_F(HttpClientTests, Handles100Continue) { Clear(); @@ -304,6 +808,104 @@ TEST_F(HttpClientTests, Handles100Continue) _response.release(); } +TEST_F(HttpClientTests, HandlesResponseLargerThanReadBuffer) +{ + Clear(); + // Several times the transport's fixed 8 KB read buffer, so the response can + // only be assembled by chaining many read completions. + const size_t responseSize = 300 * 1024; + + std::unique_ptr request(_client->CreateRequest()); + std::string requestId = request->GetId(); + request->SetUrl("http://" + _hostname + "/large/" + std::to_string(responseSize)); + _client->SendRequestAsync(request.release(), this); + + std::unique_ptr response; + { + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(30), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + response.reset(_responses[0]); + _responses.clear(); + } + + EXPECT_THAT(response->GetId(), requestId); + EXPECT_THAT(response->GetResult(), HttpResult_OK); + EXPECT_THAT(response->GetStatusCode(), 200u); + ASSERT_THAT(response->GetBody().size(), responseSize); + EXPECT_THAT(response->GetBody(), Eq(Binary(LargePayload(responseSize)))); +} + +TEST_F(HttpClientTests, HandlesRequestAndResponseLargerThanReadBuffer) +{ + Clear(); + // Exercises the send side too: the body is written separately from the + // request headers, and the echoed response is then drained in chunks. + const size_t bodySize = 200 * 1024; + auto body = Binary(LargePayload(bodySize)); + + std::unique_ptr request(_client->CreateRequest()); + std::string requestId = request->GetId(); + request->SetMethod("POST"); + request->GetHeaders().set("Content-Type", "application/octet-stream"); + request->SetUrl("http://" + _hostname + "/echo/"); + request->SetBody(body); + _client->SendRequestAsync(request.release(), this); + + std::unique_ptr response; + { + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(30), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + response.reset(_responses[0]); + _responses.clear(); + } + + EXPECT_THAT(response->GetId(), requestId); + EXPECT_THAT(response->GetResult(), HttpResult_OK); + EXPECT_THAT(response->GetStatusCode(), 200u); + ASSERT_THAT(response->GetBody().size(), bodySize); + EXPECT_THAT(response->GetBody(), Eq(Binary(LargePayload(bodySize)))); +} + +TEST_F(HttpClientTests, HandlesCancellationOfLargeResponse) +{ + Clear(); + // Cancel while the response is still being drained through the read buffer: + // the request must still produce exactly one terminal response, and the + // buffers WinHTTP was given must outlive it. + const size_t responseSize = 4 * 1024 * 1024; + + std::unique_ptr request(_client->CreateRequest()); + std::string requestId = request->GetId(); + request->SetUrl("http://" + _hostname + "/large/" + std::to_string(responseSize)); + _client->SendRequestAsync(request.release(), this); + _client->CancelRequestAsync(requestId); + + std::unique_ptr response; + { + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(30), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + response.reset(_responses[0]); + _responses.clear(); + } + + EXPECT_THAT(response->GetId(), requestId); + // The race is intentional: cancellation may land before or after the + // response has been fully read, but never both results and never neither. + EXPECT_TRUE(response->GetResult() == HttpResult_Aborted || + response->GetResult() == HttpResult_OK); + + // No duplicate terminal response arrives afterwards. + std::unique_lock lock(_lock); + EXPECT_FALSE(_responseCv.wait_for(lock, std::chrono::milliseconds(500), + [this]() { return !_responses.empty(); })); +} + TEST_F(HttpClientTests, SurvivesManyRequests) { Clear(); @@ -346,4 +948,3 @@ TEST_F(HttpClientTests, SurvivesManyRequests) } #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT - diff --git a/tests/unittests/OfflineStorageTests.cpp b/tests/unittests/OfflineStorageTests.cpp index bbb8da8e0..fec177225 100644 --- a/tests/unittests/OfflineStorageTests.cpp +++ b/tests/unittests/OfflineStorageTests.cpp @@ -1,9 +1,15 @@ // Copyright (c) Microsoft Corporation. All rights reserved. #include "common/Common.hpp" +#include "common/MockIRuntimeConfig.hpp" #include "common/MockIOfflineStorage.hpp" +#include "common/MockIOfflineStorageObserver.hpp" +#include "NullObjects.hpp" +#include "offline/OfflineStorageHandler.hpp" #include "offline/StorageObserver.hpp" +#include + using namespace testing; using namespace MAT; @@ -162,3 +168,269 @@ TEST_F(OfflineStorageTests, ReleaseRecordsIsForwarded) .WillOnce(Return()); EXPECT_THAT(offlineStorage.releaseRecordsIncRetryCount(ctx), true); } + +namespace MAT_NS_BEGIN +{ + class OfflineStorageHandlerTests : public ::testing::Test + { + protected: + class ConfigurableLogManager : public NullLogManager + { + public: + ILogConfiguration& GetLogConfiguration() override + { + return m_configuration; + } + + private: + ILogConfiguration m_configuration; + }; + + class NoCheckpointRuntimeConfig final : public testing::MockIRuntimeConfig + { + public: + bool HasConfig(const char*) override + { + return false; + } + }; + + class CountingLogManager final : public ConfigurableLogManager + { + public: + bool StartActivity() override + { + ++activeActivities; + return true; + } + + void EndActivity() override + { + --activeActivities; + } + + int activeActivities = 0; + }; + + class PausedLogManager final : public ConfigurableLogManager + { + public: + bool StartActivity() override + { + ++startActivityCalls; + return false; + } + + int startActivityCalls = 0; + }; + + class NoopTaskDispatcher final : public ITaskDispatcher + { + public: + void Join() override {} + void Queue(Task*) override {} + bool Cancel(Task*, uint64_t = 0) override { return true; } + }; + + class ThrowingTaskDispatcher final : public ITaskDispatcher + { + public: + void Join() override {} + + void Queue(Task* task) override + { + ++queueCalls; + std::unique_ptr ownedTask(task); + throw std::runtime_error("queue failed"); + } + + bool Cancel(Task*, uint64_t = 0) override { return true; } + + int queueCalls = 0; + }; + + class DroppingTaskDispatcher final : public ITaskDispatcher + { + public: + void Join() override {} + void Queue(Task* task) override + { + ++queueCalls; + delete task; + } + bool Cancel(Task*, uint64_t = 0) override { return true; } + + int queueCalls = 0; + }; + + static void ConfigureMemoryCache( + testing::MockIRuntimeConfig& config, + uint32_t sizeInBytes) + { + config[CFG_INT_RAM_QUEUE_SIZE] = sizeInBytes; + config[CFG_INT_RAMCACHE_FULL_PCT] = 75; + } + + static std::shared_ptr> + AttachDiskStorage(ConfigurableLogManager& logManager) + { + auto storage = + std::make_shared>(); + logManager.GetLogConfiguration().AddModule( + CFG_MODULE_OFFLINE_STORAGE, + storage); + return storage; + } + + static StorageRecord MakeRecord( + const char* id, + EventPersistence persistence = EventPersistence_Normal) + { + return StorageRecord( + id, + "tenant-token", + EventLatency_Normal, + persistence, + 1234567890, + std::vector{1}); + } + }; + + TEST_F(OfflineStorageHandlerTests, FlushExceptionReleasesActivityAndAllowsRetry) + { + CountingLogManager logManager; + NoCheckpointRuntimeConfig config; + NoopTaskDispatcher taskDispatcher; + ConfigureMemoryCache(config, 1024 * 1024); + auto diskStorage = AttachDiskStorage(logManager); + StrictMock observer; + OfflineStorageHandler handler(logManager, config, taskDispatcher); + EXPECT_CALL(*diskStorage, Initialize(_)); + handler.Initialize(observer); + ASSERT_TRUE(handler.StoreRecord(MakeRecord("persisted-id"))); + EXPECT_CALL(*diskStorage, StoreRecords(_)) + .WillOnce(Invoke([](std::vector& records) -> size_t + { + records.clear(); + throw std::runtime_error("flush failed"); + })); + + EXPECT_THROW(handler.Flush(), std::runtime_error); + + EXPECT_EQ(logManager.activeActivities, 0); + EXPECT_CALL(*diskStorage, StoreRecords(_)).WillOnce(Return(1)); + EXPECT_CALL(observer, OnStorageRecordsSaved(1)); + EXPECT_NO_THROW(handler.Flush()); + EXPECT_EQ(logManager.activeActivities, 0); + EXPECT_CALL(*diskStorage, Shutdown()); + handler.Shutdown(); + } + + TEST_F(OfflineStorageHandlerTests, SchedulingExceptionAllowsAnotherFlushAttempt) + { + ConfigurableLogManager logManager; + NoCheckpointRuntimeConfig config; + ThrowingTaskDispatcher taskDispatcher; + ConfigureMemoryCache(config, 1); + auto diskStorage = AttachDiskStorage(logManager); + StrictMock observer; + OfflineStorageHandler handler(logManager, config, taskDispatcher); + EXPECT_CALL(*diskStorage, Initialize(_)); + handler.Initialize(observer); + + ASSERT_TRUE(handler.StoreRecord(MakeRecord("first"))); + EXPECT_THROW( + handler.StoreRecord(MakeRecord("second")), + std::runtime_error); + EXPECT_THROW( + handler.StoreRecord(MakeRecord("third")), + std::runtime_error); + EXPECT_EQ(taskDispatcher.queueCalls, 2); + } + + TEST_F(OfflineStorageHandlerTests, DroppedTaskAllowsAnotherFlushAttempt) + { + ConfigurableLogManager logManager; + NoCheckpointRuntimeConfig config; + DroppingTaskDispatcher taskDispatcher; + ConfigureMemoryCache(config, 1); + auto diskStorage = AttachDiskStorage(logManager); + StrictMock observer; + OfflineStorageHandler handler(logManager, config, taskDispatcher); + EXPECT_CALL(*diskStorage, Initialize(_)); + handler.Initialize(observer); + + ASSERT_TRUE(handler.StoreRecord(MakeRecord("first"))); + EXPECT_TRUE(handler.StoreRecord(MakeRecord("second"))); + EXPECT_TRUE(handler.StoreRecord(MakeRecord("third"))); + EXPECT_EQ(taskDispatcher.queueCalls, 2); + } + + TEST_F(OfflineStorageHandlerTests, PartialFlushRestoresBatchForRetry) + { + CountingLogManager logManager; + NoCheckpointRuntimeConfig config; + NoopTaskDispatcher taskDispatcher; + ConfigureMemoryCache(config, 1024 * 1024); + auto diskStorage = AttachDiskStorage(logManager); + StrictMock observer; + OfflineStorageHandler handler(logManager, config, taskDispatcher); + EXPECT_CALL(*diskStorage, Initialize(_)); + handler.Initialize(observer); + ASSERT_TRUE(handler.StoreRecord(MakeRecord("first"))); + ASSERT_TRUE(handler.StoreRecord(MakeRecord("second"))); + + EXPECT_CALL(*diskStorage, StoreRecords(_)) + .WillOnce(Invoke([](std::vector& records) + { + EXPECT_THAT(records, SizeIs(2)); + records.clear(); + return 1; + })); + EXPECT_CALL(observer, OnStorageRecordsSaved(1)); + handler.Flush(); + + EXPECT_CALL(*diskStorage, StoreRecords(_)) + .WillOnce(Invoke([](const std::vector& records) + { + EXPECT_THAT(records, SizeIs(2)); + return records.size(); + })); + EXPECT_CALL(observer, OnStorageRecordsSaved(2)); + handler.Flush(); + + EXPECT_CALL(*diskStorage, Shutdown()); + handler.Shutdown(); + } + + TEST_F(OfflineStorageHandlerTests, ShutdownFlushesMemoryAfterActivityPause) + { + PausedLogManager logManager; + NoCheckpointRuntimeConfig config; + NoopTaskDispatcher taskDispatcher; + ConfigureMemoryCache(config, 1024 * 1024); + auto diskStorage = AttachDiskStorage(logManager); + StrictMock observer; + OfflineStorageHandler handler(logManager, config, taskDispatcher); + EXPECT_CALL(*diskStorage, Initialize(_)); + handler.Initialize(observer); + ASSERT_TRUE(handler.StoreRecord(MakeRecord("persisted-id"))); + ASSERT_TRUE(handler.StoreRecord(MakeRecord( + "memory-only-id", + EventPersistence_DoNotStoreOnDisk))); + + EXPECT_CALL(*diskStorage, StoreRecords(_)) + .WillOnce(Invoke([](const std::vector& persistedRecords) + { + EXPECT_THAT(persistedRecords, SizeIs(1)); + EXPECT_EQ(persistedRecords.front().id, "persisted-id"); + return persistedRecords.size(); + })); + EXPECT_CALL(observer, OnStorageRecordsSaved(1)); + EXPECT_CALL(*diskStorage, Shutdown()); + + handler.Shutdown(); + + EXPECT_EQ(logManager.startActivityCalls, 0); + } +} MAT_NS_END diff --git a/tests/unittests/PalTests.cpp b/tests/unittests/PalTests.cpp index c931ff376..907758a92 100644 --- a/tests/unittests/PalTests.cpp +++ b/tests/unittests/PalTests.cpp @@ -10,10 +10,13 @@ #include "Version.hpp" #include +#include #include +#include #include #include #include +#include #ifdef HAVE_MAT_LOGGING #include "pal/PAL.hpp" @@ -225,6 +228,72 @@ namespace void ThrowNonStdException() { throw 123; } void Signal(std::atomic* ran) { ran->store(true); } }; + + class DroppingTaskDispatcher final : public ITaskDispatcher + { + public: + void Join() override {} + void Queue(Task* task) override { delete task; } + + bool Cancel(Task*, uint64_t = 0) override + { + cancelCalled = true; + return false; + } + + bool cancelCalled = false; + }; + + class ScheduledTaskTarget + { + public: + explicit ScheduledTaskTarget(std::atomic& callbackRan) : + m_callbackRan(callbackRan) + { + } + + void Callback() + { + m_callbackRan.store(true); + } + + private: + std::atomic& m_callbackRan; + }; + + class BlockingScheduledTaskTarget + { + public: + void Callback() + { + std::unique_lock lock(m_mutex); + m_entered = true; + m_condition.notify_all(); + m_condition.wait(lock, [this]() { return m_released; }); + } + + bool WaitUntilEntered() + { + std::unique_lock lock(m_mutex); + return m_condition.wait_for( + lock, std::chrono::seconds(2), [this]() { return m_entered; }); + } + + void Release() + { + { + std::lock_guard lock(m_mutex); + m_released = true; + } + m_condition.notify_all(); + } + + private: + std::mutex m_mutex; + std::condition_variable m_condition; + bool m_entered {false}; + bool m_released {false}; + }; } // A task throwing an exception must be contained by the worker thread loop; @@ -253,6 +322,90 @@ TEST_F(PalTests, WorkerThreadContainsThrowingTask) dispatcher->Join(); } +TEST_F(PalTests, ScheduleTaskReturnsNoOpHandleWhenDispatcherDropsTask) +{ + DroppingTaskDispatcher dispatcher; + std::atomic callbackRan(false); + ScheduledTaskTarget target(callbackRan); + + auto handle = PAL::scheduleTask(&dispatcher, 0, &target, &ScheduledTaskTarget::Callback); + + EXPECT_EQ(handle.GetTask(), nullptr); + EXPECT_TRUE(handle.Cancel()); + EXPECT_FALSE(dispatcher.cancelCalled); + EXPECT_FALSE(callbackRan.load()); +} + +TEST_F(PalTests, ScheduleTaskHandleClearsAfterCallbackCompletes) +{ + auto dispatcher = PAL::WorkerThreadFactory::Create(); + std::atomic callbackRan(false); + ScheduledTaskTarget target(callbackRan); + auto handle = PAL::scheduleTask(dispatcher.get(), 0, &target, &ScheduledTaskTarget::Callback); + + for (int i = 0; i < 500 && (!callbackRan.load() || handle.GetTask() != nullptr); ++i) + { + PAL::sleep(10); + } + + EXPECT_TRUE(callbackRan.load()); + EXPECT_EQ(handle.GetTask(), nullptr); + EXPECT_TRUE(handle.Cancel()); + + dispatcher->Join(); +} + +TEST_F(PalTests, ScheduleTaskCancelSerializesTaskDestruction) +{ + auto dispatcher = PAL::WorkerThreadFactory::Create(); + std::atomic callbackRan(false); + ScheduledTaskTarget target(callbackRan); + auto handle = PAL::scheduleTask( + dispatcher.get(), 60000, &target, &ScheduledTaskTarget::Callback); + + ASSERT_NE(handle.GetTask(), nullptr); + EXPECT_TRUE(handle.Cancel()); + EXPECT_EQ(handle.GetTask(), nullptr); + EXPECT_FALSE(callbackRan.load()); + + dispatcher->Join(); +} + +TEST_F(PalTests, ScheduleTaskCancelWaitDoesNotDeadlockTaskDestruction) +{ + auto dispatcher = PAL::WorkerThreadFactory::Create(); + BlockingScheduledTaskTarget target; + auto handle = PAL::scheduleTask( + dispatcher.get(), 0, &target, &BlockingScheduledTaskTarget::Callback); + + ASSERT_TRUE(target.WaitUntilEntered()); + + std::atomic cancelReturned(false); + bool cancelResult = false; + std::thread canceller([&]() { + cancelResult = handle.Cancel(2000); + cancelReturned.store(true); + }); + + PAL::sleep(50); + target.Release(); + for (int i = 0; i < 50 && !cancelReturned.load(); ++i) + { + PAL::sleep(10); + } + + EXPECT_TRUE(cancelReturned.load()); + canceller.join(); + EXPECT_TRUE(cancelResult); + for (int i = 0; i < 50 && handle.GetTask() != nullptr; ++i) + { + PAL::sleep(10); + } + EXPECT_EQ(handle.GetTask(), nullptr); + + dispatcher->Join(); +} + #ifdef HAVE_MAT_LOGGING class LogInitTest : public Test { diff --git a/tests/unittests/UnitTests.vcxproj b/tests/unittests/UnitTests.vcxproj index faf465e97..4a9e3d5e7 100644 --- a/tests/unittests/UnitTests.vcxproj +++ b/tests/unittests/UnitTests.vcxproj @@ -157,7 +157,7 @@ /machine:X86 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -206,7 +206,7 @@ /machine:X86 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -253,7 +253,7 @@ /machine:X64 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -302,7 +302,7 @@ /machine:ARM64 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -351,7 +351,7 @@ /machine:X64 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -398,7 +398,7 @@ /machine:ARM64 %(AdditionalOptions) - crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;iphlpapi.lib;%(AdditionalDependencies) + crypt32.lib;pdh.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;comdlg32.lib;advapi32.lib;version.lib;rpcrt4.lib;wininet.lib;winhttp.lib;iphlpapi.lib;%(AdditionalDependencies) %(AdditionalLibraryDirectories) true %(IgnoreSpecificDefaultLibraries) @@ -411,6 +411,16 @@ true + + + HAVE_MAT_WININET_HTTP_CLIENT;%(PreprocessorDefinitions) + + + + + HAVE_MAT_WINHTTP_HTTP_CLIENT;%(PreprocessorDefinitions) + + diff --git a/tests/vcpkg/README.md b/tests/vcpkg/README.md index 3e758a394..d05012ce3 100644 --- a/tests/vcpkg/README.md +++ b/tests/vcpkg/README.md @@ -35,6 +35,12 @@ Best run from a **VS Developer Command Prompt** (ensures the same compiler versi .\tests\vcpkg\test-vcpkg-windows.ps1 -VcpkgRoot C:\path\to\vcpkg ``` +Use `-WinInet` to exercise the opt-in WinInet feature instead of the default +WinHTTP transport: +```powershell +.\tests\vcpkg\test-vcpkg-windows.ps1 -VcpkgRoot C:\path\to\vcpkg -WinInet +``` + > **Note:** Visual Studio's `vcvarsall.bat` overrides the `VCPKG_ROOT` environment variable. > Always pass `-VcpkgRoot` explicitly to point at your vcpkg installation. diff --git a/tests/vcpkg/test-vcpkg-windows.ps1 b/tests/vcpkg/test-vcpkg-windows.ps1 index 5073daa56..759834413 100644 --- a/tests/vcpkg/test-vcpkg-windows.ps1 +++ b/tests/vcpkg/test-vcpkg-windows.ps1 @@ -4,7 +4,8 @@ # .\tests\vcpkg\test-vcpkg-windows.ps1 -Triplet x64-windows param( [string]$VcpkgRoot = "", - [string]$Triplet = "" + [string]$Triplet = "", + [switch]$WinInet ) $ErrorActionPreference = "Stop" @@ -55,7 +56,8 @@ if ([string]::IsNullOrEmpty($Triplet)) { $Triplet = "x64-windows-static" } } -$BuildDir = Join-Path $ScriptDir "build-windows-$Triplet" +$Transport = if ($WinInet) { "WinInet" } else { "WinHTTP" } +$BuildDir = Join-Path $ScriptDir "build-windows-$Triplet-$($Transport.ToLowerInvariant())" # Map triplet to vcvarsall architecture $VcvarsArch = switch -Regex ($Triplet) { @@ -67,6 +69,7 @@ $VcvarsArch = switch -Regex ($Triplet) { Write-Host "Repository root: $RepoRoot" Write-Host "vcpkg root: $VcpkgRoot" Write-Host "Triplet: $Triplet" +Write-Host "HTTP transport: $Transport" # Clean previous build if (Test-Path $BuildDir) { @@ -84,6 +87,9 @@ $CmakeArgs = @( "-DVCPKG_OVERLAY_PORTS=$OverlayPorts", "-DCMAKE_BUILD_TYPE=Release" ) +if ($WinInet) { + $CmakeArgs += "-DVCPKG_MANIFEST_FEATURES=wininet" +} # Detect whether cl.exe is on PATH (i.e., running from VS Developer Command Prompt) $clExe = Get-Command cl.exe -ErrorAction SilentlyContinue diff --git a/tests/vcpkg/vcpkg.json b/tests/vcpkg/vcpkg.json index 1f1f4a536..dbcee9cfc 100644 --- a/tests/vcpkg/vcpkg.json +++ b/tests/vcpkg/vcpkg.json @@ -4,5 +4,19 @@ "description": "Integration test for cpp-client-telemetry vcpkg port", "dependencies": [ "cpp-client-telemetry" - ] + ], + "features": { + "wininet": { + "description": "Exercise the cpp-client-telemetry WinInet feature on Windows.", + "supports": "windows & !mingw", + "dependencies": [ + { + "name": "cpp-client-telemetry", + "features": [ + "wininet" + ] + } + ] + } + } } diff --git a/tools/ports/cpp-client-telemetry/portfile.cmake b/tools/ports/cpp-client-telemetry/portfile.cmake index 5bc5fddf4..57fa8a54e 100644 --- a/tools/ports/cpp-client-telemetry/portfile.cmake +++ b/tools/ports/cpp-client-telemetry/portfile.cmake @@ -140,6 +140,11 @@ if(MATSDK_ROOT_CMAKE MATCHES "MATSDK_MINIMAL_SQLITE" list(APPEND MATSDK_PINNED_SOURCE_OPTIONS -DMATSDK_MINIMAL_SQLITE=ON) endif() +set(MATSDK_USE_WININET OFF) +if("wininet" IN_LIST FEATURES) + set(MATSDK_USE_WININET ON) +endif() + vcpkg_cmake_configure( SOURCE_PATH "${SOURCE_PATH}" OPTIONS @@ -147,6 +152,7 @@ vcpkg_cmake_configure( -DMATSDK_SQLITE_PROVIDER=${MATSDK_VCPKG_SQLITE_PROVIDER} -DBUILD_SHARED_LIBS=${MATSDK_VCPKG_BUILD_SHARED_LIBS} -DMATSDK_ANDROID_HTTP_CLIENT=${MATSDK_ANDROID_HTTP_CLIENT} + -DMATSDK_USE_WININET=${MATSDK_USE_WININET} -DMATSDK_BUILD_HEADERS=ON -DMATSDK_BUILD_LIBRARY=ON -DMATSDK_BUILD_TEST_TOOL=OFF diff --git a/tools/ports/cpp-client-telemetry/vcpkg.json b/tools/ports/cpp-client-telemetry/vcpkg.json index d183bf6ca..14ab091c5 100644 --- a/tools/ports/cpp-client-telemetry/vcpkg.json +++ b/tools/ports/cpp-client-telemetry/vcpkg.json @@ -67,7 +67,7 @@ ] }, "curl-openssl": { - "description": "Built-in libcurl HTTP client with the OpenSSL TLS backend (default). Affects Linux only; Android uses the Java/JNI bridge unless an android-curl-* feature is selected, Windows uses WinInet, and Apple uses NSURLSession.", + "description": "Built-in libcurl HTTP client with the OpenSSL TLS backend (default). Affects Linux only; Android uses the Java/JNI bridge unless an android-curl-* feature is selected, Windows uses WinHTTP by default, and Apple uses NSURLSession.", "dependencies": [ { "name": "curl", @@ -91,6 +91,10 @@ "platform": "!osx & !ios" } ] + }, + "wininet": { + "description": "On Windows, explicitly use WinInet instead of the default WinHTTP transport for IE-integrated proxy or cookie behavior.", + "supports": "windows & !mingw" } } }