OpenBD's attitude disappoints me sometimes

G'day:
This is not the article I was thinking of writing today. But this is just too daft not to repeat.

The other day there was some discussion on the Railo Google group about some weirdness with JSON validation, which is worth a quick read. In summary, it seemed to the original poster that there was some false positives coming through. Being a literalist I observed that none of the examples were actually JSON according to JSON's own spec, as well as the RFC. Micha and I agreed to disagree - kinda... I'm sure Micha doesn't care about my opinion ;-) - as to whether Railo should stick to the spec at the expense of breaking existing code. He suggested the more moderate route.


In that thread Igal asked what other CFML engines did with the sample code, so I ran it on ColdFusion (9 & 10), and OpenBD (3.0). ColdFusion got the whole thing right: none of them were JSON. OpenBD has a bug in that in some situations isJson() will return true for something that is a) not JSON; b) will cause deserializeJson() to error.  Here's an example to demonstrate this:

s = "{'key':'value'}";
writeOutput('isJson("#s#"): #isJson(s)#<br>');
try {
    writeDump(deserializeJson(s));
}
catch (any e){
    writeOutput("IT BROKE: #e.message# #e.detail#<br>");    
}
writeOutput("<hr>");

s = '{"key":"value"}';
writeOutput('isJson("#s#"): #isJson(s)#<br>');
try {
    writeDump(deserializeJson(s));
}
catch (any e){
    writeOutput("IT BROKE: #e.message# #e.detail#<br>");    
}

The key bits here are highlighted. The first example - in red - is not actually valid JSON. The JSON spec/RFC states that strings in JSON need to be quoted with double quotes. Single quotes are not strictly valid here (that said, all the browsers I tested on accepted either / or).

The results were as follows.

ColdFusion:

isJson("{'key':'value'}"): NO
IT BROKE: JSON parsing failure: Expected '"' at character 2:''' in {'key':'value'}

---

isJson("{"key":"value"}"): YES

struct
key	value

Railo:

isJson("{'key':'value'}"): true

Struct
key	string value

---

isJson("{"key":"value"}"): true

Struct
key	string value

OpenBD:

isJson("{'key':'value'}"): YES
IT BROKE: [Invalid JSON] Line=1; Column=3; Offset=1

---

isJson("{"key":"value"}"): YES

struct
key	value

I think ColdFusion has the most correct answer here, but at least Railo's is uniform: isJson() says the things are JSON, and deserializeJson() deserialises it.

OpenBD has a bug in that isJson() says "yep", but deserializeJson() says "actually: no, I don't like that one bit". These two functions need to be reading from the same hymn sheet!

I raised this on the OpenBD Google group, and was just going to leave it there because now they know about it, and can fix it or not at their whim.

However I found Alan's reaction / justification to why this is not a problem to be so astonishing I needed to react both on their mailing list, and here too. Here's what he said:

This is known and our documentation clearly states that this is not a
true exhaustive test:

http://openbd.org/manual/?/function/isjson

For performance reasons we take a "guess-estimate" as to whether or not
the string looks like JSon.   We don't want to do a complete conversion
just so someone can then do a complete conversion again.

The link says this:

Determines if the object represents a JSON packet; Note that this is not an exhaustive test. Use DeserializeJSon()

I'm sorry, but that's just stupid. If isJson() doesn't work reliably, then they should blimin' bother with it. Because - as I say in my follow-up to Alan - if one cares about code being robust, there's simply no point in using isJson(), because it doesn't work. Basically one needs to do this:

try {
    foo = deserializeJson(bar);
} catch(any e){
    // if only I could have reliably verified it was JSON first.
}    

I get what Alan is saying: to execute isJson(), basically the string needs to be parsed which is kinda going to be covered by what deserializeJson() will do; and once isJson() is run, the next thing one is gonna do is probably deserialise it. So there's some double-processing going on there. However whether or not this is an issue is not Alan's call. As the person writing the CFML code for my app, it's my call. I am fine with having isJson() then deserializeJson() repeating some steps, as it makes my logically clear and robust. And I'm far more fine with it than having to try/catch every call I make to deserializeJson() because that's the only way of reliably not erroring when I try to process some JSON. Having to use try/catch around a language construct is just wrong. Especially if there's already supposedly a mechanism in place to mitigate the need for it.

Bleah.

--
Adam