From 5e1d273fd925ce51e0de1e3bd27f8b56466cafb5 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Mon, 15 Jan 2024 16:57:59 +0800 Subject: [PATCH 1/8] Backend(,Tests): don't marshall hybrid type names This is the 1st part (out of three) to fix bug 242: in certain circumstances, the call to Type.GetType(fullTypeName) was returning null (and then causing an NRE like the stacktrace in bug 242 proves) or a FileLoadException [3]. It turns out that .NET's BCL, despite having separate properties for the Type names "FullName" vs "AssemblyQualifiedName", the first one still adds assembly qualified names to types that are part of a generic type definition. I.e., it will not add them in a type such as "System.String" but it will in one such as "List" (see [1]): ``` "System.Collections.Generic.List`1[[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]] ``` This could be dangerous because it would embed the version number (even though we already have a JSON field for that in our MarshallingWrapper type, which could make the type be not found by Type.GetType (and as a consequence return null, or throw the exception mentioned before). The fix, or rather workaround, here, is to use ToString() instead of the .FullName property, as hinted by this S.O. answer: [2]. [1] https://learn.microsoft.com/en-us/dotnet/api/system.type.fullname [2] https://stackoverflow.com/a/4662878/23158491 [3] Stack trace: ``` System.IO.FileLoadException: Could not load file or assembly 'GWallet.Backend, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'. The located assembly's manifest definition does not match the assembly reference. (0x80131040) File name: 'GWallet.Backend, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, StackCrawlMarkHandle stackMark, ObjectHandleOnStack assemblyLoadContext, ObjectHandleOnStack type, ObjectHandleOnStack keepalive) at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, StackCrawlMark& stackMark, AssemblyLoadContext assemblyLoadContext) at System.RuntimeType.GetType(String typeName, Boolean throwOnError, Boolean ignoreCase, StackCrawlMark& stackMark) at System.Type.GetType(String typeName) at GWallet.Backend.Marshalling.ExtractType(String json) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Marshalling.fs:line 156 at GWallet.Backend.Account.ImportSignedTransactionFromJson(String jsonOrCompressedJson) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Account.fs:line 713 at GWallet.Backend.Account.LoadSignedTransactionFromFile(String filePath) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Account.fs:line 766 at Program.BroadcastPayment() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line 62 at Program.PerformOperation(UInt32 numActiveAccounts, UInt32 numHotAccounts) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line 398 at Program.ProgramMainLoop[a]() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line 478 at Program.NormalStartWithNoParameters() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line 490 ``` --- .../GWallet.Backend.Tests.fsproj | 1 + src/GWallet.Backend.Tests/MetaMarshalling.fs | 20 +++++++++++++++++++ .../signedAndFormattedBtcTransaction.json | 2 +- .../signedAndFormattedEtherTransaction.json | 2 +- .../signedAndFormattedSaiTransaction.json | 2 +- .../unsignedAndFormattedBtcTransaction.json | 2 +- .../unsignedAndFormattedEtherTransaction.json | 2 +- .../unsignedAndFormattedSaiTransaction.json | 2 +- src/GWallet.Backend/Marshalling.fs | 12 ++++++++--- 9 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 src/GWallet.Backend.Tests/MetaMarshalling.fs diff --git a/src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj b/src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj index 7407d1b20..a61297f78 100644 --- a/src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj +++ b/src/GWallet.Backend.Tests/GWallet.Backend.Tests.fsproj @@ -23,6 +23,7 @@ + diff --git a/src/GWallet.Backend.Tests/MetaMarshalling.fs b/src/GWallet.Backend.Tests/MetaMarshalling.fs new file mode 100644 index 000000000..57b26a353 --- /dev/null +++ b/src/GWallet.Backend.Tests/MetaMarshalling.fs @@ -0,0 +1,20 @@ +namespace GWallet.Backend.Tests + +open NUnit.Framework + +open GWallet.Backend + +[] +type MetaMarshalling() = + + [] + member __.``wrapper's TypeName property doesn't contain assembly-qualified-name``() = + let json = Marshalling.Serialize MarshallingData.SignedBtcTransactionExample + 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) + + Assert.That(wrapperTypeString, Does.Not.Contain "Version=") diff --git a/src/GWallet.Backend.Tests/data/signedAndFormattedBtcTransaction.json b/src/GWallet.Backend.Tests/data/signedAndFormattedBtcTransaction.json index de46d58d2..d9b96a394 100644 --- a/src/GWallet.Backend.Tests/data/signedAndFormattedBtcTransaction.json +++ b/src/GWallet.Backend.Tests/data/signedAndFormattedBtcTransaction.json @@ -1,6 +1,6 @@ { "Version": "{version}", - "TypeName": "GWallet.Backend.SignedTransaction`1[[GWallet.Backend.UtxoCoin.TransactionMetadata, GWallet.Backend, Version={version}, Culture=neutral, PublicKeyToken=null]]", + "TypeName": "GWallet.Backend.SignedTransaction`1[GWallet.Backend.UtxoCoin.TransactionMetadata]", "Value": { "TransactionInfo": { "Proposal": { diff --git a/src/GWallet.Backend.Tests/data/signedAndFormattedEtherTransaction.json b/src/GWallet.Backend.Tests/data/signedAndFormattedEtherTransaction.json index b93428cc2..a3004b0c5 100644 --- a/src/GWallet.Backend.Tests/data/signedAndFormattedEtherTransaction.json +++ b/src/GWallet.Backend.Tests/data/signedAndFormattedEtherTransaction.json @@ -1,6 +1,6 @@ { "Version": "{version}", - "TypeName": "GWallet.Backend.SignedTransaction`1[[GWallet.Backend.Ether.TransactionMetadata, GWallet.Backend, Version={version}, Culture=neutral, PublicKeyToken=null]]", + "TypeName": "GWallet.Backend.SignedTransaction`1[GWallet.Backend.Ether.TransactionMetadata]", "Value": { "TransactionInfo": { "Proposal": { diff --git a/src/GWallet.Backend.Tests/data/signedAndFormattedSaiTransaction.json b/src/GWallet.Backend.Tests/data/signedAndFormattedSaiTransaction.json index af562adaa..8b6c833dc 100644 --- a/src/GWallet.Backend.Tests/data/signedAndFormattedSaiTransaction.json +++ b/src/GWallet.Backend.Tests/data/signedAndFormattedSaiTransaction.json @@ -1,6 +1,6 @@ { "Version": "{version}", - "TypeName": "GWallet.Backend.SignedTransaction`1[[GWallet.Backend.Ether.TransactionMetadata, GWallet.Backend, Version={version}, Culture=neutral, PublicKeyToken=null]]", + "TypeName": "GWallet.Backend.SignedTransaction`1[GWallet.Backend.Ether.TransactionMetadata]", "Value": { "TransactionInfo": { "Proposal": { diff --git a/src/GWallet.Backend.Tests/data/unsignedAndFormattedBtcTransaction.json b/src/GWallet.Backend.Tests/data/unsignedAndFormattedBtcTransaction.json index 837a3f363..9c4ca1cf6 100644 --- a/src/GWallet.Backend.Tests/data/unsignedAndFormattedBtcTransaction.json +++ b/src/GWallet.Backend.Tests/data/unsignedAndFormattedBtcTransaction.json @@ -1,6 +1,6 @@ { "Version": "{version}", - "TypeName": "GWallet.Backend.UnsignedTransaction`1[[GWallet.Backend.UtxoCoin.TransactionMetadata, GWallet.Backend, Version={version}, Culture=neutral, PublicKeyToken=null]]", + "TypeName": "GWallet.Backend.UnsignedTransaction`1[GWallet.Backend.UtxoCoin.TransactionMetadata]", "Value": { "Proposal": { "OriginAddress": "16pKBjGGZkUXo1afyBNf5ttFvV9hauS1kR", diff --git a/src/GWallet.Backend.Tests/data/unsignedAndFormattedEtherTransaction.json b/src/GWallet.Backend.Tests/data/unsignedAndFormattedEtherTransaction.json index b0c131f11..3a1fe5066 100644 --- a/src/GWallet.Backend.Tests/data/unsignedAndFormattedEtherTransaction.json +++ b/src/GWallet.Backend.Tests/data/unsignedAndFormattedEtherTransaction.json @@ -1,6 +1,6 @@ { "Version": "{version}", - "TypeName": "GWallet.Backend.UnsignedTransaction`1[[GWallet.Backend.Ether.TransactionMetadata, GWallet.Backend, Version={version}, Culture=neutral, PublicKeyToken=null]]", + "TypeName": "GWallet.Backend.UnsignedTransaction`1[GWallet.Backend.Ether.TransactionMetadata]", "Value": { "Proposal": { "OriginAddress": "0xf3j4m0rjx94sushh03j", diff --git a/src/GWallet.Backend.Tests/data/unsignedAndFormattedSaiTransaction.json b/src/GWallet.Backend.Tests/data/unsignedAndFormattedSaiTransaction.json index 843fe18f5..50dd0f279 100644 --- a/src/GWallet.Backend.Tests/data/unsignedAndFormattedSaiTransaction.json +++ b/src/GWallet.Backend.Tests/data/unsignedAndFormattedSaiTransaction.json @@ -1,6 +1,6 @@ { "Version": "{version}", - "TypeName": "GWallet.Backend.UnsignedTransaction`1[[GWallet.Backend.Ether.TransactionMetadata, GWallet.Backend, Version={version}, Culture=neutral, PublicKeyToken=null]]", + "TypeName": "GWallet.Backend.UnsignedTransaction`1[GWallet.Backend.Ether.TransactionMetadata]", "Value": { "Proposal": { "OriginAddress": "0xba766d6d13E2Cc921Bf6e896319D32502af9e37E", diff --git a/src/GWallet.Backend/Marshalling.fs b/src/GWallet.Backend/Marshalling.fs index 94ab966b4..4f0cd0ab4 100644 --- a/src/GWallet.Backend/Marshalling.fs +++ b/src/GWallet.Backend/Marshalling.fs @@ -94,7 +94,7 @@ type MarshallingWrapper<'T> = { Value = value Version = VersionHelper.CURRENT_VERSION - TypeName = typeof<'T>.FullName + TypeName = typeof<'T>.ToString() } type private PascalCase2LowercasePlusUnderscoreContractResolver() = @@ -145,12 +145,18 @@ module Marshalling = let private currentVersion = VersionHelper.CURRENT_VERSION - let ExtractType(json: string): Type = + let ExtractStringType(json: string): string = + 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 + + let ExtractType(json: string): Type = + let typeName = ExtractStringType json try - Type.GetType wrapper.TypeName + Type.GetType typeName with | :? NullReferenceException as _nre -> failwith <| SPrintF1 "Failed to extract type from JSON (NRE): %s" json From 689276c7cd71046fab7917051dfd2082097ecee0 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 3 Jan 2024 12:14:49 +0800 Subject: [PATCH 2/8] Backend.Tests: test to check red CI in --- src/GWallet.Backend.Tests/Serialization.fs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/GWallet.Backend.Tests/Serialization.fs b/src/GWallet.Backend.Tests/Serialization.fs index afc048db4..28b336af4 100644 --- a/src/GWallet.Backend.Tests/Serialization.fs +++ b/src/GWallet.Backend.Tests/Serialization.fs @@ -8,6 +8,11 @@ open GWallet.Backend [] type Serialization() = + + [] + member __.``Versioning works``() = + Assert.Fail "This is a test to see if CI is red" + [] member __.``basic caching export does not fail``() = let json = Marshalling.Serialize MarshallingData.EmptyCachingDataExample From f64e2471650dc237f73078dfbd0ef54cf7b22589 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 3 Jan 2024 12:53:06 +0800 Subject: [PATCH 3/8] Backend.Tests: acceptance t for bug 242's 2nd part --- src/GWallet.Backend.Tests/Serialization.fs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/GWallet.Backend.Tests/Serialization.fs b/src/GWallet.Backend.Tests/Serialization.fs index 28b336af4..e8e64e2f1 100644 --- a/src/GWallet.Backend.Tests/Serialization.fs +++ b/src/GWallet.Backend.Tests/Serialization.fs @@ -11,7 +11,12 @@ type Serialization() = [] member __.``Versioning works``() = - Assert.Fail "This is a test to see if CI is red" + Assert.That( + VersionHelper.CURRENT_VERSION, + Is.Not.EqualTo "1.0.0.0", + "Proper version was somehow not properly assigned as assembly version" + ) + [] member __.``basic caching export does not fail``() = From b97ff6abedd1c540b5a4357ae21f9d0a11628483 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 3 Jan 2024 13:03:08 +0800 Subject: [PATCH 4/8] Backend.Tests: move SetUp to Deserialization too --- src/GWallet.Backend.Tests/Deserialization.fs | 4 ++++ src/GWallet.Backend.Tests/MarshallingData.fs | 8 ++++++++ src/GWallet.Backend.Tests/Serialization.fs | 7 +------ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/GWallet.Backend.Tests/Deserialization.fs b/src/GWallet.Backend.Tests/Deserialization.fs index a7b7866e6..28f744870 100644 --- a/src/GWallet.Backend.Tests/Deserialization.fs +++ b/src/GWallet.Backend.Tests/Deserialization.fs @@ -10,6 +10,10 @@ open GWallet.Backend.UtxoCoin [] type Deserialization() = + [] + member __.``Versioning works``() = + MarshallingData.AssertAssemblyVersion() + [] member __.``deserialize cache does not fail``() = diff --git a/src/GWallet.Backend.Tests/MarshallingData.fs b/src/GWallet.Backend.Tests/MarshallingData.fs index d39aaf5e9..ed1526044 100644 --- a/src/GWallet.Backend.Tests/MarshallingData.fs +++ b/src/GWallet.Backend.Tests/MarshallingData.fs @@ -325,3 +325,11 @@ module MarshallingData = let UnsignedEtherTransactionExampleInJson = ReadEmbeddedResource "unsignedAndFormattedEtherTransaction.json" + + let AssertAssemblyVersion() = + Assert.That( + VersionHelper.CURRENT_VERSION, + Is.Not.EqualTo "1.0.0.0", + "Proper version was somehow not properly assigned as assembly version" + ) + diff --git a/src/GWallet.Backend.Tests/Serialization.fs b/src/GWallet.Backend.Tests/Serialization.fs index e8e64e2f1..a0d4f821d 100644 --- a/src/GWallet.Backend.Tests/Serialization.fs +++ b/src/GWallet.Backend.Tests/Serialization.fs @@ -11,12 +11,7 @@ type Serialization() = [] member __.``Versioning works``() = - Assert.That( - VersionHelper.CURRENT_VERSION, - Is.Not.EqualTo "1.0.0.0", - "Proper version was somehow not properly assigned as assembly version" - ) - + MarshallingData.AssertAssemblyVersion() [] member __.``basic caching export does not fail``() = From 2d10da7c7734ac0647d6911f3b7912f41a5b5869 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 3 Jan 2024 13:13:54 +0800 Subject: [PATCH 5/8] Backend.*: fix assembly version attribs Somehow, even if CommonAssemblyInfo.fs is included in Backend.NetStandard project, it was not being applied (as the recent snap testing commit [1] revealed, when it calls geewallet with the --version flag), so we now use MSBuild attributes instead, which work and the best proof that they do, is that we've had to change a LOC in Backend.Tests for them not to break after this change. NOTE: this commit was decided to be backported from master to stable branch, cherry-picking [2], where Backend.NetStandard project doesn't exist, but Backend project does. This is the 2nd part of the bugfix for bug 242. [1] ce93f84c23b8ce8a682c56a3df3ebca2e4df528a [2] 2edb96ead1abc1ab2450c5e197cfbc50b2cec70a --- scripts/bump.fsx | 1 + src/GWallet.Backend.Tests/MarshallingData.fs | 2 +- src/GWallet.Backend/GWallet.Backend.fsproj | 6 ++++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/scripts/bump.fsx b/scripts/bump.fsx index 33ae883f3..0286233d3 100755 --- a/scripts/bump.fsx +++ b/scripts/bump.fsx @@ -49,6 +49,7 @@ let filesToBumpMiniVersion: seq = let filesToBumpFullVersion: seq = Seq.append filesToBumpMiniVersion [ + "src/GWallet.Backend/GWallet.Backend.fsproj" "src/GWallet.Backend/Properties/CommonAssemblyInfo.fs" "snap/snapcraft.yaml" ] diff --git a/src/GWallet.Backend.Tests/MarshallingData.fs b/src/GWallet.Backend.Tests/MarshallingData.fs index ed1526044..03ddc295f 100644 --- a/src/GWallet.Backend.Tests/MarshallingData.fs +++ b/src/GWallet.Backend.Tests/MarshallingData.fs @@ -14,7 +14,7 @@ open GWallet.Backend.Ether module MarshallingData = let private executingAssembly = Assembly.GetExecutingAssembly() - let private version = executingAssembly.GetName().Version.ToString() + let private version = VersionHelper.CURRENT_VERSION let private binPath = executingAssembly.Location |> FileInfo let private prjPath = Path.Combine(binPath.Directory.FullName, "..") |> DirectoryInfo diff --git a/src/GWallet.Backend/GWallet.Backend.fsproj b/src/GWallet.Backend/GWallet.Backend.fsproj index 21de2f640..b05f35205 100644 --- a/src/GWallet.Backend/GWallet.Backend.fsproj +++ b/src/GWallet.Backend/GWallet.Backend.fsproj @@ -5,6 +5,12 @@ true + + 0.4.407.0 + 0.4.407.0 + 0.4.407.0 + + From 29b2a5670c30bf3a120e416bbec3086538c72c85 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 3 Jan 2024 13:30:06 +0800 Subject: [PATCH 6/8] Backend.Tests: fix typo sofis...->sophisticated --- src/GWallet.Backend.Tests/Deserialization.fs | 2 +- src/GWallet.Backend.Tests/MarshallingData.fs | 6 +++--- src/GWallet.Backend.Tests/Serialization.fs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/GWallet.Backend.Tests/Deserialization.fs b/src/GWallet.Backend.Tests/Deserialization.fs index 28f744870..1d7a1f535 100644 --- a/src/GWallet.Backend.Tests/Deserialization.fs +++ b/src/GWallet.Backend.Tests/Deserialization.fs @@ -18,7 +18,7 @@ type Deserialization() = member __.``deserialize cache does not fail``() = let deserializedCache: DietCache = Marshalling.Deserialize - MarshallingData.SofisticatedCachingDataExampleInJson + MarshallingData.SophisticatedCachingDataExampleInJson Assert.That(deserializedCache, Is.Not.Null) diff --git a/src/GWallet.Backend.Tests/MarshallingData.fs b/src/GWallet.Backend.Tests/MarshallingData.fs index 03ddc295f..90e1e6a37 100644 --- a/src/GWallet.Backend.Tests/MarshallingData.fs +++ b/src/GWallet.Backend.Tests/MarshallingData.fs @@ -175,9 +175,9 @@ module MarshallingData = .Add("0xFOOBARBAZ", [Currency.ETC.ToString()]) let private fiatValues = Map.empty.Add(Currency.ETH.ToString(), 161.796m) .Add(Currency.ETC.ToString(), 169.99999999m) - let SofisticatedCachingDataExample = { UsdPrice = fiatValues; Addresses = addresses; Balances = balances; } + let SophisticatedCachingDataExample = { UsdPrice = fiatValues; Addresses = addresses; Balances = balances; } - let SofisticatedCachingDataExampleInJson = + let SophisticatedCachingDataExampleInJson = sprintf """{ "Version": "%s", "TypeName": "%s", @@ -312,7 +312,7 @@ module MarshallingData = let someEtherTransactionInfo = { Proposal = someUnsignedEtherTransactionProposal; - Cache = SofisticatedCachingDataExample; + Cache = SophisticatedCachingDataExample Metadata = someEtherTxMetadata; } let SignedEtherTransactionExample = diff --git a/src/GWallet.Backend.Tests/Serialization.fs b/src/GWallet.Backend.Tests/Serialization.fs index a0d4f821d..9a4aeb861 100644 --- a/src/GWallet.Backend.Tests/Serialization.fs +++ b/src/GWallet.Backend.Tests/Serialization.fs @@ -29,12 +29,12 @@ type Serialization() = [] member __.``complex caching export works``() = - let json = Marshalling.Serialize MarshallingData.SofisticatedCachingDataExample + let json = Marshalling.Serialize MarshallingData.SophisticatedCachingDataExample Assert.That(json, Is.Not.Null) Assert.That(json, Is.Not.Empty) Assert.That(json, - Is.EqualTo (MarshallingData.SofisticatedCachingDataExampleInJson)) + Is.EqualTo (MarshallingData.SophisticatedCachingDataExampleInJson)) [] member __.``unsigned BTC transaction export``() = From bd94cb7c228376d93dbf5b2d69d039a4cfdcfc15 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 3 Jan 2024 17:10:08 +0800 Subject: [PATCH 7/8] Backend/Account: fix typos in var names --- src/GWallet.Backend/Account.fs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/GWallet.Backend/Account.fs b/src/GWallet.Backend/Account.fs index e5505a445..01b0f2ba4 100644 --- a/src/GWallet.Backend/Account.fs +++ b/src/GWallet.Backend/Account.fs @@ -641,13 +641,13 @@ module Account = match transType with | _ when transType = typeof> -> - let deserializedBtcTransaction: SignedTransaction = + let deserializedTransaction: SignedTransaction = Marshalling.Deserialize json - deserializedBtcTransaction.ToAbstract() + deserializedTransaction.ToAbstract() | _ when transType = typeof> -> - let deserializedBtcTransaction: SignedTransaction = + let deserializedTransaction: SignedTransaction = Marshalling.Deserialize json - deserializedBtcTransaction.ToAbstract() + deserializedTransaction.ToAbstract() | _ when transType.GetGenericTypeDefinition() = typedefof> -> raise TransactionNotSignedYet | unexpectedType -> From 80a2b843acebbc4047f29db40371d1daea8826dd Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Mon, 15 Jan 2024 19:46:00 +0800 Subject: [PATCH 8/8] 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 | 41 ++++++++++++++++---- 2 files changed, 40 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..ca8e3aced 100644 --- a/src/GWallet.Backend/Marshalling.fs +++ b/src/GWallet.Backend/Marshalling.fs @@ -145,21 +145,46 @@ 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 = + SPrintF2 "Problem when trying to find type '%s' (version '%s')" + wrapper.TypeName + wrapper.Version + raise <| Exception(errMsg, ex) + if isNull res then + failwith + <| SPrintF2 + "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 )