mbraceproject / FsPickler

A fast multi-format message serializer for .NET
http://mbraceproject.github.io/FsPickler/
MIT License
323 stars 54 forks source link

Different XML serialization between `.Pickle` and `.PickleToString #115

Open piaste opened 4 years ago

piaste commented 4 years ago

Due to a well-known behaviour of the XML writer classes, FsPickler currently emits UTF-16 declared XML when calling PickleToString, but UTF-8 declared XML when going though Pickle:

> open MBrace.FsPickler;;
> let s = FsPickler.CreateXmlSerializer();;
val s : XmlSerializer

> s.PickleToString {| Foo = "aaaa"; Bar = 145 |};;
val it : string =
  "<?xml version="1.0" encoding="utf-16"?><FsPickler version="4.0.0.0" type="&lt;&gt;f__AnonymousType3044928180`2[System.Int32,System.String]"><value><Bar>145</Bar><Foo>aaaa</Foo></value></FsPickler>"

> let b = s.Pickle {| Foo = "aaaa"; Bar = 145 |} in
> System.Text.Encoding.UTF8.GetString b;;
val it : string =
  "<?xml version="1.0" encoding="utf-8"?><FsPickler version="4.0.0.0" type="&lt;&gt;f__AnonymousType3044928180`2[System.Int32,System.String]"><value><Bar>145</Bar><Foo>aaaa</Foo></value></FsPickler>"

I'm not sure if this is a considered a bug given the focused scope of the library, but it's certainly surprising.

(In my particular case, I had a pickled string rejected by SQL Server due to the different encoding.)

Fixing this so it emits UTF-8 XML through both methods should be straightforward - just generate a custom StringWriter or even an object expression in the pickleString function, but it would also technically be a breaking change. And it might be better to allow a configuration option.

Not particularly urgent since, as the above example shows, the workaround is a one-liner.

eiriktsarpalis commented 4 years ago

Given that you're proposing to change the string output as opposed to the binary output, I should say that it doesn't feel as much of a breaking change. I'd merge a PR that changes this, provided that we have verified that utf-16 xml can still be consumed by the deserializer, perhaps by adding a test that checks this.