KevinDockx/JsonPatch

Null Reference Exception when trying to copy a null value

paulparas opened this issue · 10 comments

Hi
I'm getting a Null Reference Exception when trying to copy a null value. Is it desired behaviour?
[
{
"path": "addressLine3",
"op": "copy",
"from": "addressLine2"
}
]
Here addressLine2 is null.

Thank you!
Regards
Paras Paul

I'm also getting this and have observed that it's been fixed in the .NET version of this here.

diff --git a/src/Marvin.JsonPatch/Internal/ConversionResultProvider.cs b/src/Marvin.JsonPatch/Internal/ConversionResultProvider.cs
index e008250..db96401 100644
--- a/src/Marvin.JsonPatch/Internal/ConversionResultProvider.cs
+++ b/src/Marvin.JsonPatch/Internal/ConversionResultProvider.cs
@@ -35,7 +35,7 @@ namespace Marvin.JsonPatch.Internal
             var targetType = typeToConvertTo;
             if (value == null)
             {
-                return new ConversionResult(IsNullableType(typeToConvertTo), null);
+                return new ConversionResult(canBeConverted: true, convertedInstance: null);
             }
             else if (typeToConvertTo.IsAssignableFrom(value.GetType()))
             {

If @KevinDockx is open to PRs, I can fix it.

Hi @mklaber , I'm happy to accept a PR! :)

@KevinDockx #107 is ready at your leisure. (As is #108 )

Thanks a lot @mklaber - just merged both PR's, I'll release an RC to NuGet & a final version next week (if no issues are reported from the RC).

Thanks for getting to it so quickly @KevinDockx !

Any chance you can push the RC to nuget? Or use GitHub packages? My work is going in to QA in the next day or two and could help identify issues. (And my company's infrastructure isn't well suited for libraries that aren't in nuget.)

Never mind. Reread your comment :)

Shouldn't that be something like 2.2.1-rc ? (given the latest is 2.2.0)

Woops - that's what happens when I try to quickly release something using a freshly-installed environment due to not having worked on it for a while :) Here you go: https://www.nuget.org/packages/Marvin.JsonPatch/2.2.1-rc

Don't make me jealous with your fancy new environment and your "dot net core" (or whatever the kids are calling it these days...)...

Haha, alright, valid point ;-)