Remove unused vendored PlayFab REST API SDK from the C++ GSDK (reverts #200) - #203
Conversation
The cpp/cppsdk/{source,include}/playfab tree is a fork of XPlatCppSdk
frozen at version 2.0.180420 (April 2018). It is dead code:
- Not compiled by CMakeLists.txt or GSDK_CPP_Windows.vcxproj. Only
GSDK_CPP_Linux.vcxproj referenced it, and CI never builds that project.
- Not linked even when compiled: nm/dumpbin report zero PlayFabHttp
symbols in a linked game server on both Linux and Windows, because
nothing references it.
- Not shipped: com.playfab.cppgsdk.v140.nuspec exports only gsdk.h, so
package consumers cannot reach these APIs at all.
- Not maintained: touched twice in repo history, the initial import and
a recent drive-by change.
This also reverts #200. That PR set CURLOPT_SSL_VERIFYPEER to true in
PlayFabHttp.cpp to address a reported MITM risk. Verified on both
platforms that the change had no effect on any shipped artifact: with
the PR applied versus reverted, the Linux library and game server
binaries are byte-identical.
The GSDK heartbeat to the VM Agent is unaffected either way. It uses a
separate curl handle in gsdk.cpp over a hardcoded plain-HTTP URL and
never sets CURLOPT_SSL_VERIFYPEER.
Worth noting the change was not a safe no-op if that code were ever
revived on Windows. The bundled libcurl links OpenSSL 1.1, which ignores
the Windows certificate store, and no CA bundle ships with the SDK.
Exercising PlayFabHttp::ExecuteRequest with VERIFYPEER=true fails with
curl error 60; with false it completes the handshake. The deleted
comment "TODO: Replace this with a ca-bundle ref???" described the work
that was still outstanding. Removing the code retires that risk instead
of leaving a broken path behind.
Games needing PlayFab APIs should use PlayFab/XPlatCppSdk directly.
Also strips the PlayFab calls from cppLinuxTestApp, which were self
described as "super hacky" and hardcoded titleId 6195, and drops the now
inert DISABLE_PLAYFABCLIENT_API define from the two Windows test apps.
Finally, adds a missing #include <cstdint> to gsdk.h. The header uses
uint32_t but never included it, so it failed to compile under GCC. That
is pre-existing and unrelated to the deletion, but it blocked building
the Linux test apps used to verify this change.
Verified: CMake build of the SDK, cppLinuxTestApp and cppLinuxRunnerGame
compile and link, and an end-to-end run against LocalMultiplayerAgent is
unchanged (42 heartbeats, StandingBy -> Active -> Terminating, all over
plain HTTP).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6defd46f-b276-4683-bedd-6ba3aecdaa7f
There was a problem hiding this comment.
Pull request overview
This PR removes the unused, vendored PlayFab REST API C++ SDK implementation from the C++ GSDK (cpp/cppsdk/include/playfab/ and cpp/cppsdk/source/playfab/), reverting the prior security tweak from #200 by retiring the entire dead codepath rather than maintaining it.
Changes:
- Delete the vendored PlayFab REST API SDK sources/headers from
cpp/cppsdk. - Remove PlayFab API calls from
cppLinuxTestAppand clean up project definitions that referenced PlayFab toggles/files. - Fix a C++ portability issue by adding a missing standard header include (
<cstdint>) togsdk.h.
Reviewed changes
Copilot reviewed 21 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cpp/testapps/cppWindowsTestApp/cppWindowsTestApp.vcxproj | Removes DISABLE_PLAYFABCLIENT_API define from Windows test app build definitions. |
| cpp/testapps/cppWindowsRunnerGame/cppWindowsRunnerGame.vcxproj | Removes DISABLE_PLAYFABCLIENT_API define from Windows runner build definitions. |
| cpp/testapps/cppLinuxTestApp/main.cpp | Removes PlayFab login/profile calls; keeps GSDK lifecycle test behavior. |
| cpp/cppsdk/source/playfab/PlayFabSettings.cpp | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/source/playfab/PlayFabMatchmakerApi.cpp | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/source/playfab/PlayFabHttp.cpp | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/source/playfab/PlayFabError.cpp | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/source/playfab/PlayFabEntityApi.cpp | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabSettings.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabServerApi.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabMatchmakerDataModels.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabMatchmakerApi.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabHttp.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabError.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabEntityApi.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabClientApi.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabBaseModel.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/include/playfab/PlayFabAdminApi.h | Deleted (vendored PlayFab SDK removal). |
| cpp/cppsdk/gsdk.h | Adds <cstdint> to ensure uint32_t is available for GCC/Clang builds. |
| cpp/cppsdk/GSDK_CPP_Linux.vcxproj.filters | Removes PlayFab filter entries now that files are deleted. |
| cpp/cppsdk/GSDK_CPP_Linux.vcxproj | Removes PlayFab include/compile items from the Linux vcxproj. |
Addresses review feedback on cppLinuxTestApp using std::exit without including <cstdlib>. The file relied on a transitive include for std::exit. On libstdc++ the declaration leaks in via <string> and <stdexcept>, both of which gsdk.h includes, so this compiles today and did so before this PR as well. That is not guaranteed by the standard, and other standard library implementations need not pull <cstdlib> in, so the include is now explicit. While confirming the above, found that gsdk.h itself is not self-contained: it declares a tm member and a callback taking const tm& but never includes <ctime>, so it only compiles if the consumer includes <ctime> first. Since gsdk.h is the only header the NuGet package ships, that is worth fixing here alongside the <cstdint> addition. Verified: gsdk.h now compiles standalone, the SDK builds, and cppLinuxTestApp, cppLinuxRunnerGame and a GSDK consumer game server all compile and link. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6defd46f-b276-4683-bedd-6ba3aecdaa7f
Addresses review feedback: <cstdint> and <ctime> only guarantee the std-qualified names. Whether those names are also injected into the global namespace is unspecified, so on a conforming standard library that does not expose them globally, gsdk.h would still fail to compile. It uses uint32_t and tm unqualified at lines 47, 50 and 147. Qualifying the header to std::uint32_t/std::tm would not be enough on its own, because registerMaintenanceCallback takes const tm& and consumers write that spelling in their own callbacks. Since gsdk.h is the only header the NuGet package ships, the global names have to stay available. Includes both spellings. The C compatibility headers guarantee the global names the header itself relies on, and the C++ headers keep the std-qualified names available. Dropping the latter measurably removes std::tm on libstdc++ under C++14, so both are kept. Verified under C++11/14/17/20 with -pedantic: gsdk.h compiles standalone and consumers can use tm, std::tm, uint32_t and std::uint32_t. SDK, cppLinuxTestApp, cppLinuxRunnerGame and a consumer game server all build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6defd46f-b276-4683-bedd-6ba3aecdaa7f
|
Nit (non-blocking, post-merge): two dangling references to the now-deleted
Both are harmless: neither MSBuild nor CMake errors on a nonexistent While in those two vcxproj lines, That duplication is pre-existing and unrelated to this change, but it is easy to tidy in the same sweep. |
Summary
Removes the vendored PlayFab REST API SDK (
cpp/cppsdk/source/playfab/andcpp/cppsdk/include/playfab/) from the C++ GSDK — 22 files, ~48k lines. It is a fork ofXPlatCppSdkfrozen at 2.0.180420 (April 2018) that no shipping artifact contains.This also reverts #200, which is the reason I went looking.
Three commits, easy to split if preferred:
f11474f— the removal (+5 / -55,436)9367a38— standard-header include fixes (+2)8e7e39d— guarantee the globaluint32_t/tmnames, from review feedback (+5)Why this is dead code
CMakeLists.txt?GSDK_CPP_Windows.vcxproj?GSDK_CPP_Linux.vcxproj?nm/dumpbinshow 0 PlayFabHttp symbols on either platform, even when force-compiledcom.playfab.cppgsdk.v140.nuspecexports onlygsdk.hOn #200
#200 set
CURLOPT_SSL_VERIFYPEERtotrueinPlayFabHttp.cppto fix a reported MITM risk. I tested this on Windows and Linux.It had no effect on anything shipped. Building the SDK with the PR applied vs. reverted produces byte-identical output (a control build confirms determinism first):
63d97fac=63d97fac7f3e80ee=7f3e80ee63d97fac7f3e80ee63d97fac7f3e80eea3fc67ca7f3e80ee8bb621567f3e80eeThe PlayFab-enabled lib hashes do differ, which confirms the toggle works and the null result above is real. The executable is identical in all four cases.
The GSDK heartbeat was never affected. It uses a separate curl handle in
gsdk.cppover a hardcoded plain-HTTP URL and never setsCURLOPT_SSL_VERIFYPEER:CURLOPT_SSL_VERIFYPEERis a no-op onhttp://— verified byte-identical responses on both platforms.But it was not a safe no-op either. If that path were ever revived on Windows, the change breaks it. The bundled libcurl links OpenSSL 1.1, which ignores the Windows certificate store, and no CA bundle ships with the SDK. Exercising
PlayFabHttp::ExecuteRequestfor real:VERIFYPEER=falseVERIFYPEER=trueInvalidAPIEndpoint(TLS OK)curl error: 60, cert verification failed#200 deleted the comment
// TODO: Replace this with a ca-bundle ref???. That TODO was the outstanding work, and it was removed without being done. Deleting the code retires the finding permanently rather than leaving a broken path for the next person.Games needing PlayFab APIs should use PlayFab/XPlatCppSdk directly.
Other changes
cppLinuxTestApp— stripped its PlayFab calls (self-described as "Super hacky short-term functionality PlayFab Test", hardcodingtitleId = "6195"and hitting a live API on startup). The GSDK coverage is kept.DISABLE_PLAYFABCLIENT_API— removed from the two Windows test app projects; inert once the vendored headers are gone.gsdk.hinclude fixes — the header is the only one the NuGet package ships, and it was not self-contained. It usesuint32_tandtmunqualified (lines 47, 50, 147) but included neither header, so it failed to compile under GCC and otherwise relied on the consumer including<ctime>first.It now includes
<cstdint>/<ctime>and<stdint.h>/<time.h>. Both spellings are deliberate: the C compatibility headers guarantee the global names the header itself uses, while the C++ headers keepstd::tm/std::uint32_tavailable to consumers. Using only the C headers measurably dropsstd::tmon libstdc++ under C++14. Verified with-pedanticunder C++11/14/17/20.Pre-existing and unrelated to the deletion, but it blocked building the Linux test apps used to verify this change. Happy to split out.
cppLinuxTestApp#include <cstdlib>— from review feedback;std::exitwas resolving through a transitive include. Note this was not caused by removing the PlayFab headers:std::exitleaks in via<string>and<stdexcept>, both included bygsdk.h, so it compiled the same before and after. Made explicit anyway, since transitive includes aren't guaranteed by the standard.Testing
Verified locally on both platforms with the deletion applied.
Linux (Ubuntu 24.04, GCC 13.3, libcurl 8.5.0/OpenSSL 3):
CMakeLists.txt;libGSDK_CPP_Linux.acontains the expected 6 objectscppLinuxTestAppandcppLinuxRunnerGamecompile and linkgsdk.hcompiles standalone under C++11/14/17/20 with-pedantic; consumers can usetm,std::tm,uint32_tandstd::uint32_tWindows (MSVC 14.44, SDK 10.0.26100):
GSDK_CPP_Windows(the shipped library) buildsGSDK_CPP_UnitTestsbuilds — the exact CI targetvstest.consoleBoth platforms match each other and the pre-deletion baseline exactly. Remaining warnings (
C4244ingsdkUtils.h, deprecatedregisterMaintenanceCallback) are all pre-existing.The Windows run above covers
f11474f. The two later commits are header-include-only changes; CI covers them onwindows-latest.Risk
Effectively nil for NuGet consumers — the published library never contained these symbols, so there is no ABI break. The only theoretical breakage is someone vendoring the full repo, building
GSDK_CPP_Linux.vcxproj, and calling PlayFab APIs — who is already on an 8-year-stale SDK. Probably worth a release-note line.