Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
piaste opened this issue Oct 29, 2019 · 1 comment
Open

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

piaste opened this issue Oct 29, 2019 · 1 comment

Comments

@piaste
Copy link

piaste commented Oct 29, 2019

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants