improved json parsing

[bradleypeabody]

To follow up on this issue:
#4

I did some adjustments on the JSON parser as follows:

• it now remembers if a string was quoted with single or double quotes and deals with both scenarios properly
• some cases of bad json were going into an infinite loop, it now throws an exception
• unneeded backslashes (something like $ for example) now return the literal character instead of throwing it away
• test cases created for the above and just for json functionality in general

Please merge this in so makes it into your next release.

[hyperthunk]

Thanks for the patch - we will take a look at this and #4 over the next week or so (basically we're in the process of releasing 3.1.1, but after that we will pay proper attention to these pull requests). And sorry for the delay!

[bradleypeabody]

Awesome, thanks.

[hyperthunk]

Right. This question (of whether or not to switch to another json parsing library, or to use JSR 353 or whatever) is now under discussion. Please bear with us while we think about the best approach to take. This pull request will not be ignored and I'll update you once the internal bug has seen some movement.

[bradleypeabody]

Sounds good. FWIW, my 2 cents on it is this:

Right now it looks like you have very few dependencies - which is cool. So I guess you have to look at the (workload) cost of adding a dependency vs. maintaining your own parser. Personally, I think since your build is set up with Maven and you already have a couple dependencies, adding one more (and probably a small one) wouldn't be a big deal.

If you're going to add a dependency, I would opt for using a 3rd party library rather than something like JSR 353. I'm not sure what the JDK requirements are for JSR 353, but having any at all for something as simple as JSON parsing would seem to be counter productive. There are plenty of alternatives that are small, lightweight, well maintained and would easily get the job done.

You might also consider merging my changes as a quick fix, and then do a 3rd party library as a separate task later. I don't know overall how much work it would be to totally replace all of the JSON reading and writing in the project - maybe's simple, maybe not.

[hyperthunk]

| You might also consider merging my changes as a quick fix, and then do a 3rd party library as a separate task later.

Yes, we'll probably go with that approach. Have you previously signed a Contributor Agreement for us? If not, would you mind pinging me either on the rabbitmq-discuss mailing list or on our IRC channel on freenode (#rabbitmq) and asking for one please?

Thanks.

[bradleypeabody]

Alright, I just posted a message to rabbitmq-discuss, re "contributor agreement".

[bradleypeabody]

Okay, I emailed the contributor agreement.

[bradleypeabody]

Just checking in - are we able to roll forward with this now?

[hyperthunk]

Hi @bradleypeabody, yes we will try to get this patch into the next bugfix release. Please bear in mind that we have other priority bugs in front of this though, and you should also be aware that we don't merge via github, since our master repositories are hosted internally using mercurial, so you won't see this pull-request closed until we've completed the release and it gets closed by hand.

Cheers.

[bradleypeabody]

Great - that all makes sense and thanks.

[hyperthunk]

This fix was included in the 3.1.2 release that went out today. See the release-notes and commit e41e1ba for details and proper attribution.

Thanks for contributing to RabbitMQ!