From b62c531487ff3368543d6f7d6575c7e2497fff09 Mon Sep 17 00:00:00 2001 From: Harald Csaszar Date: Wed, 28 Oct 2020 19:30:55 +0100 Subject: [PATCH] [unity] Now failing more gracefully when loading of binary skeleton data fails. Fixed memory leak when loading incompatible binary skeleton asset. Closes #1799. See #1497. --- spine-csharp/src/SkeletonBinary.cs | 37 +++++++++++++++++-- .../Asset Types/SkeletonDataAssetInspector.cs | 4 +- .../Components/SkeletonGraphicInspector.cs | 10 +++-- .../Components/SkeletonRendererInspector.cs | 9 +++++ .../Asset Types/SkeletonDataCompatibility.cs | 10 +++-- 5 files changed, 57 insertions(+), 13 deletions(-) diff --git a/spine-csharp/src/SkeletonBinary.cs b/spine-csharp/src/SkeletonBinary.cs index 5816efd24..0e584e939 100644 --- a/spine-csharp/src/SkeletonBinary.cs +++ b/spine-csharp/src/SkeletonBinary.cs @@ -122,8 +122,8 @@ namespace Spine { skeletonData.hash = hash == 0 ? null : hash.ToString(); skeletonData.version = input.ReadString(); if (skeletonData.version.Length == 0) skeletonData.version = null; - if ("3.8.75" == skeletonData.version) - throw new Exception("Unsupported skeleton data, please export with a newer version of Spine."); + // early return for old 3.8 format instead of reading past the end + if (skeletonData.version.Length > 13) return null; skeletonData.x = input.ReadFloat(); skeletonData.y = input.ReadFloat(); skeletonData.width = input.ReadFloat(); @@ -1049,13 +1049,42 @@ namespace Spine { /// Returns the version string of binary skeleton data. public string GetVersionString () { try { + // try reading 4.0+ format + var initialPosition = input.Position; ReadLong(); // long hash - string version = ReadString(); - return version; + + var stringPosition = input.Position; + int stringByteCount = ReadInt(true); + input.Position = stringPosition; + if (stringByteCount <= 13) { + string version = ReadString(); + if (char.IsDigit(version[0])) + return version; + } + // fallback to old version format + input.Position = initialPosition; + return GetVersionStringOld3X(); } catch (Exception e) { throw new ArgumentException("Stream does not contain a valid binary Skeleton Data.\n" + e, "input"); } } + + /// Returns old 3.8 and earlier format version string of binary skeleton data. + public string GetVersionStringOld3X () { + // Hash. + int byteCount = ReadInt(true); + if (byteCount > 1) input.Position += byteCount - 1; + + // Version. + byteCount = ReadInt(true); + if (byteCount > 1) { + byteCount--; + var buffer = new byte[byteCount]; + ReadFully(buffer, 0, byteCount); + return System.Text.Encoding.UTF8.GetString(buffer, 0, byteCount); + } + return null; + } } } } diff --git a/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Asset Types/SkeletonDataAssetInspector.cs b/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Asset Types/SkeletonDataAssetInspector.cs index 0d28252d2..574a892a4 100644 --- a/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Asset Types/SkeletonDataAssetInspector.cs +++ b/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Asset Types/SkeletonDataAssetInspector.cs @@ -602,12 +602,12 @@ namespace Spine.Unity.Editor { warnings.Add("Skeleton data file is not a valid Spine JSON or binary file."); } else { #if SPINE_TK2D - bool searchForSpineAtlasAssets = true; + bool searchForSpineAtlasAssets = (compatibilityProblemInfo == null); bool isSpriteCollectionNull = spriteCollection.objectReferenceValue == null; if (!isSpriteCollectionNull) searchForSpineAtlasAssets = false; #else // Analysis disable once ConvertToConstant.Local - bool searchForSpineAtlasAssets = true; + bool searchForSpineAtlasAssets = (compatibilityProblemInfo == null); #endif if (searchForSpineAtlasAssets) { diff --git a/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Components/SkeletonGraphicInspector.cs b/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Components/SkeletonGraphicInspector.cs index e1406ca27..9426731af 100644 --- a/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Components/SkeletonGraphicInspector.cs +++ b/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Components/SkeletonGraphicInspector.cs @@ -152,15 +152,19 @@ namespace Spine.Unity.Editor { forceReloadQueued = true; } - EditorGUILayout.PropertyField(material); - EditorGUILayout.PropertyField(color); - if (thisSkeletonGraphic.skeletonDataAsset == null) { EditorGUILayout.HelpBox("You need to assign a SkeletonDataAsset first.", MessageType.Info); serializedObject.ApplyModifiedProperties(); serializedObject.Update(); return; } + if (!SpineEditorUtilities.SkeletonDataAssetIsValid(thisSkeletonGraphic.skeletonDataAsset)) { + EditorGUILayout.HelpBox("Skeleton Data Asset error. Please check Skeleton Data Asset.", MessageType.Error); + return; + } + + EditorGUILayout.PropertyField(material); + EditorGUILayout.PropertyField(color); string errorMessage = null; if (SpineEditorUtilities.Preferences.componentMaterialWarning && diff --git a/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Components/SkeletonRendererInspector.cs b/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Components/SkeletonRendererInspector.cs index 2bb52f43d..c73a04041 100644 --- a/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Components/SkeletonRendererInspector.cs +++ b/spine-unity/Assets/Spine/Editor/spine-unity/Editor/Components/SkeletonRendererInspector.cs @@ -54,6 +54,7 @@ namespace Spine.Unity.Editor { [CanEditMultipleObjects] public class SkeletonRendererInspector : UnityEditor.Editor { public static bool advancedFoldout; + protected bool loadingFailed = false; const string SeparatorSlotNamesFieldName = "separatorSlotNames"; @@ -107,6 +108,7 @@ namespace Spine.Unity.Editor { isInspectingPrefab = (PrefabUtility.GetPrefabType(target) == PrefabType.Prefab); #endif SpineEditorUtilities.ConfirmInitialization(); + loadingFailed = false; // Labels SkeletonDataAssetLabel = new GUIContent("SkeletonData Asset", Icons.spine); @@ -160,7 +162,14 @@ namespace Spine.Unity.Editor { public void OnSceneGUI () { var skeletonRenderer = (SkeletonRenderer)target; + if (loadingFailed) + return; + var skeleton = skeletonRenderer.Skeleton; + if (skeleton == null) { + loadingFailed = true; + return; + } var transform = skeletonRenderer.transform; if (skeleton == null) return; diff --git a/spine-unity/Assets/Spine/Runtime/spine-unity/Asset Types/SkeletonDataCompatibility.cs b/spine-unity/Assets/Spine/Runtime/spine-unity/Asset Types/SkeletonDataCompatibility.cs index 46d3472c3..545ba0d27 100644 --- a/spine-unity/Assets/Spine/Runtime/spine-unity/Asset Types/SkeletonDataCompatibility.cs +++ b/spine-unity/Assets/Spine/Runtime/spine-unity/Asset Types/SkeletonDataCompatibility.cs @@ -91,7 +91,8 @@ namespace Spine.Unity { } } catch (System.Exception e) { - Debug.LogErrorFormat("Failed to read '{0}'. It is likely not a binary Spine SkeletonData file.\n{1}", asset.name, e); + Debug.LogError(string.Format("Failed to read '{0}'. It is likely not a binary Spine SkeletonData file.\n{1}", + asset.name, e), asset); return null; } } @@ -103,13 +104,13 @@ namespace Spine.Unity { else { object obj = Json.Deserialize(new StringReader(asset.text)); if (obj == null) { - Debug.LogErrorFormat("'{0}' is not valid JSON.", asset.name); + Debug.LogError(string.Format("'{0}' is not valid JSON.", asset.name), asset); return null; } var root = obj as Dictionary; if (root == null) { - Debug.LogErrorFormat("'{0}' is not compatible JSON. Parser returned an incorrect type while parsing version info.", asset.name); + Debug.LogError(string.Format("'{0}' is not compatible JSON. Parser returned an incorrect type while parsing version info.", asset.name), asset); return null; } @@ -133,7 +134,8 @@ namespace Spine.Unity { int.Parse(versionSplit[1], CultureInfo.InvariantCulture) }; } catch (System.Exception e) { - Debug.LogErrorFormat("Failed to read version info at skeleton '{0}'. It is likely not a valid Spine SkeletonData file.\n{1}", asset.name, e); + Debug.LogError(string.Format("Failed to read version info at skeleton '{0}'. It is likely not a valid Spine SkeletonData file.\n{1}", + asset.name, e), asset); return null; } return fileVersion;