Don't try to JSON serialize binary data

[korylprince]

Profiles (plist) allow binary data, but JSON does not. An example of this in the wild is using a profile to set the logo for NoMAD Login.

Interestingly, this issue is masked, at least in my case, with a ValueError: Circular reference detected. Only after digging into the code did I discover the issue was actually with trying to JSON encode binary data. Fixing this issue also fixed the Circular reference error (for me), so the issues with PR #90 may be solved with this instead.

This PR just replaces the binary data with the literal "BINARY DATA" using the existing serializer.

[sheagcraig]

I like this; awesome detective work.

I think we have another option too, which would be easy to implement. I'm not sure one is a clear winner. If we base64 encode the binary data (and then decode the bytes to str) we could have the actual binary data available. Not sure this is useful, but it's an option.

[korylprince]

I thought about that, and I can see it from both sides. Having the data might be useful if say you wanted to check that the data matches an expected value. But including it could possibly make the request quite a bit bigger, and would cause the database to grow.

I would think that if someone really wanted that binary data to do something with, they could write an additional plugin to specifically fetch it.

I'm happy to do a PR with either method, just let me know which way you want to go.

[kbotnen]

I had the exact same problem as you with: ValueError: Circular reference detected , I built a new version which included this fix and now my machine finally start ship the reports to sal. Hope it will be merged as soon as possible so I can use the sal-scripts through autopkg instead of my homebuilt package.

Anyway: Great work of you on an already great product 👍