From 256cf338a7169eed9269f31eb3c6abcbecea962c Mon Sep 17 00:00:00 2001 From: Jackson Fields Date: Thu, 2 Dec 2021 16:29:58 -0800 Subject: [PATCH] Fix removing saved anchors Ensure anchor names are properly null terminated so xrUnpersistSpatialAnchorMSFT names match anchors that were persisted. This fixes a bug where RemoveSavedARPin was always failing with XR_ERROR_SPATIAL_ANCHOR_NAME_NOT_FOUND_MSFT Update version to 1.1.8 --- .../MicrosoftOpenXR/MicrosoftOpenXR.uplugin | 2 +- .../Private/SpatialAnchorPlugin.cpp | 34 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/MsftOpenXRGame/Plugins/MicrosoftOpenXR/MicrosoftOpenXR.uplugin b/MsftOpenXRGame/Plugins/MicrosoftOpenXR/MicrosoftOpenXR.uplugin index 327dd57..3598687 100644 --- a/MsftOpenXRGame/Plugins/MicrosoftOpenXR/MicrosoftOpenXR.uplugin +++ b/MsftOpenXRGame/Plugins/MicrosoftOpenXR/MicrosoftOpenXR.uplugin @@ -1,7 +1,7 @@ { "FileVersion": 3, "Version": 1, - "VersionName": "1.1.7", + "VersionName": "1.1.8", "FriendlyName": "Microsoft OpenXR", "Description": "The Microsoft OpenXR plugin is a game plugin which provides additional features available on Microsoft's Mixed Reality devices like the HoloLens 2 when using OpenXR.", "Category": "Mixed Reality", diff --git a/MsftOpenXRGame/Plugins/MicrosoftOpenXR/Source/MicrosoftOpenXR/Private/SpatialAnchorPlugin.cpp b/MsftOpenXRGame/Plugins/MicrosoftOpenXR/Source/MicrosoftOpenXR/Private/SpatialAnchorPlugin.cpp index a3bc7eb..56a3bdc 100644 --- a/MsftOpenXRGame/Plugins/MicrosoftOpenXR/Source/MicrosoftOpenXR/Private/SpatialAnchorPlugin.cpp +++ b/MsftOpenXRGame/Plugins/MicrosoftOpenXR/Source/MicrosoftOpenXR/Private/SpatialAnchorPlugin.cpp @@ -362,7 +362,16 @@ namespace MicrosoftOpenXR XrSpatialAnchorPersistenceInfoMSFT PersistenceInfo{ XR_TYPE_SPATIAL_ANCHOR_PERSISTENCE_INFO_MSFT }; FTCHARToUTF8 UTF8ConvertedString(*SaveId); - memcpy(PersistenceInfo.spatialAnchorPersistenceName.name, UTF8ConvertedString.Get(), UTF8ConvertedString.Length()); + // Length() returns size without null terminator. + // Anchor name is valid up to XR_MAX_SPATIAL_ANCHOR_NAME_SIZE_MSFT - 1 to ensure room for a null terminator. + if (UTF8ConvertedString.Length() >= XR_MAX_SPATIAL_ANCHOR_NAME_SIZE_MSFT) + { + UE_LOG(LogHMD, Warning, TEXT("Pin name is too long. ARPin will not be saved.")); + return false; + } + + // Length + 1 to ensure null terminator is included + FPlatformString::Strncpy(PersistenceInfo.spatialAnchorPersistenceName.name, UTF8ConvertedString.Get(), UTF8ConvertedString.Length() + 1); PersistenceInfo.spatialAnchor = AnchorMSFT->Anchor; XrResult result = xrPersistSpatialAnchorMSFT(SpatialAnchorStoreMSFT, &PersistenceInfo); @@ -406,7 +415,16 @@ namespace MicrosoftOpenXR XrSpatialAnchorPersistenceNameMSFT SpatialAnchorPersistenceName; FTCHARToUTF8 UTF8ConvertedString(*SaveId); - memcpy(SpatialAnchorPersistenceName.name, UTF8ConvertedString.Get(), UTF8ConvertedString.Length()); + // Length() returns size without null terminator. + // Anchor name is valid up to XR_MAX_SPATIAL_ANCHOR_NAME_SIZE_MSFT - 1 to ensure room for a null terminator. + if (UTF8ConvertedString.Length() >= XR_MAX_SPATIAL_ANCHOR_NAME_SIZE_MSFT) + { + UE_LOG(LogHMD, Warning, TEXT("Pin name is too long. Anchor will not be removed.")); + return; + } + + // Length + 1 to ensure null terminator is included + FPlatformString::Strncpy(SpatialAnchorPersistenceName.name, UTF8ConvertedString.Get(), UTF8ConvertedString.Length() + 1); xrUnpersistSpatialAnchorMSFT(SpatialAnchorStoreMSFT, &SpatialAnchorPersistenceName); return; @@ -487,7 +505,17 @@ namespace MicrosoftOpenXR XrSpatialAnchorPersistenceInfoMSFT PersistenceInfo{ XR_TYPE_SPATIAL_ANCHOR_PERSISTENCE_INFO_MSFT }; FTCHARToUTF8 UTF8ConvertedString(*InPinId.ToLower()); - memcpy(PersistenceInfo.spatialAnchorPersistenceName.name, UTF8ConvertedString.Get(), UTF8ConvertedString.Length()); + + // Length() returns size without null terminator. + // Anchor name is valid up to XR_MAX_SPATIAL_ANCHOR_NAME_SIZE_MSFT - 1 to ensure room for a null terminator. + if (UTF8ConvertedString.Length() >= XR_MAX_SPATIAL_ANCHOR_NAME_SIZE_MSFT) + { + UE_LOG(LogHMD, Warning, TEXT("Pin name is too long. Perception anchor will not be stored.")); + return false; + } + + // Length + 1 to ensure null terminator is included + FPlatformString::Strncpy(PersistenceInfo.spatialAnchorPersistenceName.name, UTF8ConvertedString.Get(), UTF8ConvertedString.Length() + 1); PersistenceInfo.spatialAnchor = AnchorMSFT->Anchor;