Add SpotBugs / Find Security Bugs

[bdemers]

Sets the default encoding in GsonDeserializer to UTF_8 Previously this was unset, and used the system default

[coveralls]

Coverage remained the same at 100.0% when pulling a620bf735e77b0d21b6d6a71c2044b200aabbf11 on gson-utf8 into eadf0ce4fc252341f04c50a45aac6f9c670283e7 on master.

[bdemers]

Wow @lhazlewood, I had convinced myself 0.11 was already out. 🤦‍♂

I changed the note to say that Gson support was added.

As for the test, I added SpotBugs and Find Security Bugs (which is how I found this initially) Added a few fixes for those issues as well, a few of them are less critical but were simple (and easier to fix than exclude)

Thoughts? (spotbugs changes are in a08c1fba109aa3335f38fd782d2f9c78ff5d7e75)

[lhazlewood]

I meant an actual unit test that inspects the String instance and ensures the encoding is UTF-8.

[bdemers]

Probably just change the name, but let's wrap up the other open questions/issues first and then figure out the best option

[bdemers]

@lhazlewood have a suggestion on a good way to do this?

I meant an actual unit test that inspects the String instance and ensures the encoding is UTF-8.

The only option I could think of was to create a byte array with characters known to render differently, but IMHO, it's easier to scan the source tree for these types of issues 🤔

[lhazlewood]

@bdemers just create a mock gson instance and mock gson.fromJson, and assert that the argument is UTF-8 compatible?

I found this: https://www.turro.org/publications/?item=114&page=0

But if you don't want to mess with that, no objection from me. I'm just paranoid 😅

[bdemers]

I was messing with creating keys today with BC (via JCA), and it's really really easy to get a RuntimeException to be thrown. I think if we just tack on a | RuntimeException we should be good for backward compatibility.

[lhazlewood]

I think if we just tack on a | RuntimeException we should be good for backward compatibility.

If that's the case, why change the code at all? Just curious.

[bdemers]

Valid point, I was just thinking it would be easier to change in the future, but it's probably just noise

[bdemers]

@lhazlewood back to the topic of the test, we actually do have tests that fail when the system's default encoding is NOT UTF-8 we just don't see this because that is usually what Linux and MacOS is set to.

Adding a test for this is challenging because it would require forking a JVM to change the file.encoding property.

I tried this from my IDE, but you should be able to reproduce it by running the tests against master with following arg-Dfile.encoding=ISO-8859-1

[bdemers]

I updated the PR name to better reflect the changes here, and I've moved the Gson change log note to: #547

[lhazlewood]

@bdemers what do you want to do with this? Thoughts?

[bdemers]

I'll take another pass at this in a different PR, based off the latest code