Erroneous check in JSONObject merge Method

[jstueckrath]

I noticed a problem in the method protected static JSONArray merge(JSONArray o1, Object o2) of the JSONObject class. It contains the following check:

if (o1 instanceof JSONArray)
    return merge(o1, (JSONArray) o2);

which does not make much sense, since o1 is already of type JSONArray. I believe o2 was meant to be checked here. The implementation would then be analogous to the method protected static JSONObject merge(JSONObject o1, Object o2).

Since I only spotted this by coincidence, I do not know the effects of this bug.

[UrielCh]

Thanks for the notice, I may have done that on purpose.

I won't try to fix it, without a sample that will cause an invalid behavior.

[jstueckrath]

I wonder what purpose there could be for the check as it is. Since o1 is already of type JSONArray the check o1 instanceof JSONArray is equivalent to o1 != null (ignoring performance) and if o1 would actually be null the next line would immediately cause a null pointer exception.

If this has not been a problem so far, then in practise the method is never called with null, the check is always true and the last two lines of the method are dead code.

Just to be clear, I was taking about this method:

json-smart-v2/json-smart/src/main/java/net/minidev/json/JSONObject.java

Line 237 in 00955f7

protected static JSONArray merge(JSONArray o1, Object o2) {

[UrielCh]

So let me read the code.

JSONObject.java, I have 2 method:
protected static JSONArray merge(JSONArray o1, Object o2);
and
private static JSONArray merge(JSONArray o1, JSONArray o2);

I think that the purpose of this code is to be able to merge:

{a: ["v1", "v2"]} with {a: "v3"}
like:
{a: ["v1", "v2"]} with {a: ["v3"]}

in both case the result will be: {a: ["v1", "v2", "v3"]}

even if only the second one is valid.

By the way, the merge function can not merge
{a: "v3"} with {a: ["v1", "v2"]}

json-smart was not build to be strict, the strict mode had been added to match JSON community requests.

[jstueckrath]

I had a look at this from the Java perspective, so what Java object would {a: "v3"} be in your library? If it was a JSONObject (for instance) calling this method with o1 = {a: ["v1", "v2"]} and o2 = {a: "v3"} would throw a ClassCastException when trying to cast JSONObject to JSONArray. If {a: "v3"} is a JSONArray already, everything is fine.

If I understand correctly the code should do the following:

• If the second parameter is an array, add all items of the second array to the first array, i.e. the result is an array with o1.length + o2.length items.
• If the second parameter is anything else (except null), add the second parameter as an item to the array, i.e. the result is an array with o1.length + 1 items.

To achieve this you need to check that the second parameter is an array or not. Thus, the type of o2 must be checked. The type of o1 is always an array.

The problem will occur if you want to merge {a: ["v1", "v2"]} with v3: {...} to get {a: ["v1", "v2", "v3": {...}]}, which I assumed the method should also be able to do due to its last two lines (which would otherwise be dead code).

P.S.: I will not be able to answer the next few day, but will at the end of the week.