From 662596a016e28f09f175876cb75461fdf2f2615c Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Mon, 15 Jan 2024 19:46:00 +0800 Subject: [PATCH] Backend(Tests): extra safety measures against null This is the 3rd and last part of the bugfix for bug 242, which makes sure that Type.GetType() won't return null again, or if it does, that we will have enough info (or innerEx with info) to find culprits and make better decisions later. --- src/GWallet.Backend.Tests/MetaMarshalling.fs | 11 +++--- src/GWallet.Backend/Marshalling.fs | 38 +++++++++++++++----- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/GWallet.Backend.Tests/MetaMarshalling.fs b/src/GWallet.Backend.Tests/MetaMarshalling.fs index 57b26a353..eb60590b8 100644 --- a/src/GWallet.Backend.Tests/MetaMarshalling.fs +++ b/src/GWallet.Backend.Tests/MetaMarshalling.fs @@ -13,8 +13,11 @@ type MetaMarshalling() = Assert.That(json, Is.Not.Null) Assert.That(json, Is.Not.Empty) - let wrapperTypeString = Marshalling.ExtractStringType json - Assert.That(wrapperTypeString, Is.Not.Null) - Assert.That(wrapperTypeString, Is.Not.Empty) + let wrapper = Marshalling.ExtractWrapper json + Assert.That(wrapper, Is.Not.Null) + Assert.That(wrapper.TypeName, Is.Not.Null) + Assert.That(wrapper.TypeName, Is.Not.Empty) + Assert.That(wrapper.Version, Is.Not.Null) + Assert.That(wrapper.Version, Is.Not.Empty) - Assert.That(wrapperTypeString, Does.Not.Contain "Version=") + Assert.That(wrapper.TypeName, Does.Not.Contain "Version=") diff --git a/src/GWallet.Backend/Marshalling.fs b/src/GWallet.Backend/Marshalling.fs index 4f0cd0ab4..16c726172 100644 --- a/src/GWallet.Backend/Marshalling.fs +++ b/src/GWallet.Backend/Marshalling.fs @@ -145,21 +145,43 @@ module Marshalling = let private currentVersion = VersionHelper.CURRENT_VERSION - let ExtractStringType(json: string): string = + let ExtractWrapper(json: string): MarshallingWrapper = if String.IsNullOrEmpty json then raise <| ArgumentNullException "json" let wrapper = JsonConvert.DeserializeObject> json if Object.ReferenceEquals(wrapper, null) then failwith <| SPrintF1 "Failed to extract type from JSON (null check): %s" json - wrapper.TypeName + if String.IsNullOrEmpty wrapper.TypeName then + failwith <| SPrintF1 "Failed to extract type from JSON (inner null check 1): %s" json + if String.IsNullOrEmpty wrapper.Version then + failwith <| SPrintF1 "Failed to extract type from JSON (inner null check 2): %s" json + wrapper let ExtractType(json: string): Type = - let typeName = ExtractStringType json - try - Type.GetType typeName - with - | :? NullReferenceException as _nre -> - failwith <| SPrintF1 "Failed to extract type from JSON (NRE): %s" json + let wrapper = ExtractWrapper json + + let res = + try + // we prefer an ex with innerException than an NRE caused by the + // consumer of this function + let throwOnError = true + + Type.GetType(wrapper.TypeName, throwOnError) + with + | :? NullReferenceException as _nre -> + failwith <| SPrintF1 "Failed to extract type from JSON (NRE): %s" json + | ex -> + let errMsg = + sprintf "Problem when trying to find type '%s' (version '%s')" + wrapper.TypeName + wrapper.Version + raise <| Exception(errMsg, ex) + if isNull res then + failwithf "Could not find type '%s' (version '%s')" + wrapper.TypeName + wrapper.Version + res + // FIXME: should we rather use JContainer.Parse? it seems JObject.Parse wouldn't detect error in this: {A:{"B": 1}} // (for more info see replies of https://stackoverflow.com/questions/6903477/need-a-string-json-validator )