Strange behavior observed in cJSON_ReplaceItemInArray

[tregua87]

I encountered this strange behavior in the API cJSON_ReplaceItemInArray. Here below I paste a minimal example and an explanation. I replicated this error in the latest cJSON commit 87d8f0961a01bf09bef98ff89bae9fdec42181ee, and compiling with these options:

clang++ -g -std=c++11 -I$INSTALL_DIR/include -fsanitize=address \
		test.c -Wl,--whole-archive $INSTALL_DIR/libcjson.a -Wl,--no-whole-archive \
		-lz -ljpeg -Wl,-Bstatic -llzma -Wl,-Bdynamic -lstdc++ -o test

(some options might be redundant , sorry)

The issue appears when trying to replace an item with itself: cJSON_ReplaceItemInArray(json, 0, json). What I observe is that the library generates a loop in the object graphs, thus making the final object unuseful, i.e., item[0] -> item One of the consequence is that the cJSON_Delete produces stack exhaustion. This is alarming because the return value ret_v is 1, thus the program assumes the replacement was correct.

Even though I understand this might not be the intended usage of cJSON_ReplaceItemInArray, we should be able to detect loops in object graphs. Another approach is to generate a copy for the newitem argument for cJSON_ReplaceItemInArray. This will avoid recursions, but it might lead to memory leaks.

#include <cjson/cJSON.h>

#include <string.h>
#include <stdio.h>

int main(int argn, char** argc) {

    int ret_v;
    cJSON *json;
    
    char *x = "[true]";
    
    printf("start\n");

    json =  cJSON_ParseWithLength(x, strlen(x)+1);
    if (json == 0) {
        printf("error 1\n");
        return 0;
    }

    printf("json: %s\n", cJSON_Print(json));

    ret_v =  cJSON_ReplaceItemInArray(json, 0, json);
    if (ret_v == 0) 
        printf("Replace failed!\n");
    else
        printf("Replaced success!!\n");
    
    printf("end\n");

    cJSON_Delete(json);

    return 0;
}

[daschfg]

So with the call to cJSON_ReplaceItemInArray, you're replacing the first entry in the array with a reference to the whole array, thus creating a circular reference.

This is somewhat similar to #807 and probably would require a similar solution.

I expect generating a copy for every insert and replace operation would not be possible without a breaking change in the API. Checking the whole object for duplicates on the other hand would probably lead to performance issues down the road, so I'm not sure about the best solution on this.