Skip to content

Commit

Permalink
AzureConnection: remove our dependency on cpprestsdk (microsoft#14776)
Browse files Browse the repository at this point in the history
This pull request removes, in full, our dependency on cpprestsdk. This
allows us to shed 500KiB-1.2MiB from our package off the top and enables
the following future investments:

- Removal of the App CRT forwarders to save an additional ~500KiB
- Switching over to the HybridCRT and removing our dependency on _any
  CRT_.

cpprest was built on my dev box two or so years ago, and is in _utter_
violation of our compliance guidelines on SBOM et al.

In addition, this change allows us to use the proxy server configured
in Windows Settings.

I did this in four steps (represented roughly by the individual commits):

1. Switch from cpprest's http_client/json to Windows.Web.Http and
   Windows.Data.Json
2. Switch from websocketpp to winhttp.dll's WebSocket implementation¹
3. Remove all remaining utility classes
4. Purge all dependencies from all projects and scripts on cpprest.

I also took this opportunity to add a feature flag that allows Dev
builds to run AzureConnection in-process.

¹ Windows.Networking.Sockets' API is so unergonomic that it was simply
infeasible (and also _horrible_) to use it.

## Validation Steps

I've run the Azure Connection quite a bit inproc.

Closes microsoft#4575.
Might be related to microsoft#5977, microsoft#11714, and with the user agent thing maybe microsoft#14403.
  • Loading branch information
DHowett authored Feb 7, 2023
1 parent 42e8de3 commit 4903cfd
Show file tree
Hide file tree
Showing 16 changed files with 292 additions and 347 deletions.
5 changes: 5 additions & 0 deletions .github/actions/spelling/allow/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ GETMOUSEHOVERTIME
Hashtable
HIGHCONTRASTON
HIGHCONTRASTW
hinternet
HINTERNET
hotkeys
href
hrgn
Expand Down Expand Up @@ -214,13 +216,16 @@ Viewbox
virtualalloc
wcsstr
wcstoui
WDJ
winhttp
winmain
winsta
winstamin
wmemcmp
wpc
WSF
wsregex
WWH
wwinmain
xchg
XDocument
Expand Down
3 changes: 0 additions & 3 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,6 @@ CPLINFO
cplusplus
CPPCORECHECK
cppcorecheckrules
cpprest
cpprestsdk
cppwinrt
CProc
Expand Down Expand Up @@ -1437,7 +1436,6 @@ PPEB
ppf
ppguid
ppidl
pplx
PPROC
PPROCESS
ppropvar
Expand Down Expand Up @@ -2114,7 +2112,6 @@ WDDMCONSOLECONTEXT
wdm
webpage
websites
websockets
wekyb
wex
wextest
Expand Down
45 changes: 0 additions & 45 deletions build/config/ESRPSigning_Terminal.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,51 +67,6 @@
}
]
}
},
{
// THIRD PARTY SOFTWARE
"MatchedPath": [
"cpprest*.dll"
],
"SigningInfo": {
"Operations": [
{
"KeyCode": "CP-231522",
"OperationSetCode": "SigntoolSign",
"Parameters": [
{
"parameterName": "OpusName",
"parameterValue": "Microsoft"
},
{
"parameterName": "OpusInfo",
"parameterValue": "http://www.microsoft.com"
},
{
"parameterName": "FileDigest",
"parameterValue": "/fd \"SHA256\""
},
{
"parameterName": "PageHash",
"parameterValue": "/NPH"
},
{
"parameterName": "TimeStamp",
"parameterValue": "/tr \"http://rfc3161.gtm.corp.microsoft.com/TSS/HttpTspServer\" /td sha256"
}
],
"ToolName": "sign",
"ToolVersion": "1.0"
},
{
"KeyCode": "CP-231522",
"OperationSetCode": "SigntoolVerify",
"Parameters": [],
"ToolName": "sign",
"ToolVersion": "1.0"
}
]
}
}
]
}
2 changes: 1 addition & 1 deletion build/pipelines/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ parameters:

variables:
MakeAppxPath: 'C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x86\MakeAppx.exe'
TerminalInternalPackageVersion: "0.0.7"
TerminalInternalPackageVersion: "0.0.8"
# If we are building a branch called "release-*", change the NuGet suffix
# to "preview". If we don't do that, XES will set the suffix to "release1"
# because it truncates the value after the first period.
Expand Down
2 changes: 1 addition & 1 deletion build/pipelines/templates/build-console-compliance-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ jobs:
inputs:
TargetPattern: guardianGlob
# See https://aka.ms/gdn-globs for how to do match patterns
AnalyzeTargetGlob: $(Build.SourcesDirectory)\bin\**\*.dll;$(Build.SourcesDirectory)\bin\**\*.exe;-:file|**\Microsoft.UI.Xaml.dll;-:file|**\Microsoft.Toolkit.Win32.UI.XamlHost.dll;-:file|**\vcruntime*.dll;-:file|**\vcomp*.dll;-:file|**\vccorlib*.dll;-:file|**\vcamp*.dll;-:file|**\msvcp*.dll;-:file|**\concrt*.dll;-:file|**\TerminalThemeHelpers*.dll;-:file|**\cpprest*.dll
AnalyzeTargetGlob: $(Build.SourcesDirectory)\bin\**\*.dll;$(Build.SourcesDirectory)\bin\**\*.exe;-:file|**\Microsoft.UI.Xaml.dll;-:file|**\Microsoft.Toolkit.Win32.UI.XamlHost.dll;-:file|**\vcruntime*.dll;-:file|**\vcomp*.dll;-:file|**\vccorlib*.dll;-:file|**\vcamp*.dll;-:file|**\msvcp*.dll;-:file|**\concrt*.dll;-:file|**\TerminalThemeHelpers*.dll
continueOnError: true

# Set XES_SERIALPOSTBUILDREADY to run Security and Compliance task once per build
Expand Down
5 changes: 0 additions & 5 deletions build/scripts/Test-WindowsTerminalPackage.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,6 @@ Try {
Throw "Failed to find App.xbf (TerminalApp project) in resources.pri"
}

If (($null -eq (Get-Item "$AppxPackageRootPath\cpprest142_2_10.dll" -EA:Ignore)) -And
($null -eq (Get-Item "$AppxPackageRootPath\cpprest142_2_10d.dll" -EA:Ignore))) {
Throw "Failed to find cpprest142_2_10.dll -- check the WAP packaging project"
}

If (($null -eq (Get-Item "$AppxPackageRootPath\wtd.exe" -EA:Ignore)) -And
($null -eq (Get-Item "$AppxPackageRootPath\wt.exe" -EA:Ignore))) {
Throw "Failed to find wt.exe/wtd.exe -- check the WAP packaging project"
Expand Down
1 change: 0 additions & 1 deletion dep/nuget/packages.config
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
<package id="Microsoft.Internal.PGO-Helpers.Cpp" version="0.2.34" targetFramework="native" />
<package id="Microsoft.Taef" version="10.60.210621002" targetFramework="native" />
<package id="Microsoft.Windows.CppWinRT" version="2.0.210825.3" targetFramework="native" />
<package id="vcpkg-cpprestsdk" version="2.10.14" targetFramework="native" />
<package id="Microsoft.VCRTForwarders.140" version="1.0.4" targetFramework="native" />
<package id="Microsoft.Internal.Windows.Terminal.ThemeHelpers" version="0.6.220404001" targetFramework="native" />
<package id="Microsoft.VisualStudio.Setup.Configuration.Native" version="2.3.2262" targetFramework="native" developmentDependency="true" />
Expand Down
20 changes: 0 additions & 20 deletions scratch/ScratchIslandApp/WindowExe/WindowExe.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -197,26 +197,6 @@
<!-- **END VC LIBS HACK** -->


<!-- **BEGIN TERMINAL CONNECTION HACK** -->
<!-- This is the same as the above VC libs hack, but for TerminalConnection.
TerminalConnection depends on cpprest*.dll, and if we don't include it in
the packaging output, we'll crash as soon as we try to load
TerminalConnection.dll.
The Sample sln needs to do this manually - the real exe has a
ProjectReference to TerminalConnection.vcxproj and can figure this out on
its own. -->
<ItemGroup>
<_TerminalConnectionDlls Include="$(OpenConsoleCommonOutDir)\TerminalConnection\*.dll" />

<PackagingOutputs Include="@(_TerminalConnectionDlls)">
<ProjectName>$(ProjectName)</ProjectName>
<OutputGroup>BuiltProjectOutputGroup</OutputGroup>
<TargetPath>%(Filename)%(Extension)</TargetPath>
</PackagingOutputs>
</ItemGroup>
<!-- **END TERMINAL CONNECTION HACK** -->

<!-- Same thing again here, with WindowsTerminal.exe -->
<ItemGroup>
<_WindowsTerminalExe Include="$(OpenConsoleCommonOutDir)\WindowsTerminal\*.exe" />
Expand Down
11 changes: 9 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1247,10 +1247,17 @@ namespace winrt::TerminalApp::implementation
if (connectionType == TerminalConnection::AzureConnection::ConnectionType() &&
TerminalConnection::AzureConnection::IsAzureConnectionAvailable())
{
// TODO GH#4661: Replace this with directly using the AzCon when our VT is better
std::filesystem::path azBridgePath{ wil::GetModuleFileNameW<std::wstring>(nullptr) };
azBridgePath.replace_filename(L"TerminalAzBridge.exe");
connection = TerminalConnection::ConptyConnection();
if constexpr (Feature_AzureConnectionInProc::IsEnabled())
{
connection = TerminalConnection::AzureConnection{};
}
else
{
connection = TerminalConnection::ConptyConnection{};
}

auto valueSet = TerminalConnection::ConptyConnection::CreateSettings(azBridgePath.wstring(),
L".",
L"Azure",
Expand Down
22 changes: 15 additions & 7 deletions src/cascadia/TerminalConnection/AzureClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,31 @@

#pragma once

#include "cpprest/json.h"

namespace Microsoft::Terminal::Azure
{
class AzureException : public std::runtime_error
{
std::wstring _code;

public:
static bool IsErrorPayload(const web::json::value& errorObject)
static bool IsErrorPayload(const winrt::Windows::Data::Json::JsonObject& errorObject)
{
return errorObject.has_string_field(L"error");
if (!errorObject.HasKey(L"error"))
{
return false;
}

if (errorObject.GetNamedValue(L"error").ValueType() != winrt::Windows::Data::Json::JsonValueType::String)
{
return false;
}

return true;
}

AzureException(const web::json::value& errorObject) :
runtime_error(til::u16u8(errorObject.at(L"error_description").as_string())), // surface the human-readable description as .what()
_code(errorObject.at(L"error").as_string())
AzureException(const winrt::Windows::Data::Json::JsonObject& errorObject) :
runtime_error(til::u16u8(errorObject.GetNamedString(L"error_description"))), // surface the human-readable description as .what()
_code(errorObject.GetNamedString(L"error"))
{
}

Expand Down
Loading

0 comments on commit 4903cfd

Please sign in to comment.