JSON-Patch icon indicating copy to clipboard operation
JSON-Patch copied to clipboard

Array patching, over empty elements..

Open KpjComp opened this issue 4 years ago • 2 comments
trafficstars

has a slight problem with empty array elements and applying diffs.

Lets assume we have we have an array [1,,3,,5], and then create a patch for [1,,3,,5,,7,,9], The patch created is correct, eg: add for array element 6 & 8 with values 7 & 9. But if we apply this patch to the original, you will get [1,,3,,5,7,9].. IOW: it will scrunch the newly added array elements and put the first array element at index 5 & 6, instead of 6 & 8.

This function ->

var arrOps = {
    add: function (arr, i, document) {
        if (helpers_js_1.isInteger(i)) {
            arr.splice(i, 0, this.value);
        }
        else { // array props
            arr[i] = this.value;
        }
        // this may be needed when using '-' in an array
        return { newDocument: document, index: i };
    },

Now I'm not 100% sure why splice was used here, unless it's maybe a performance optimization, but the problem with splice here, if you did splice(6, 0, value), the index position it will use if the index is greater than length, is the length (wrong). IOW: It will update index 5, and not index 6, and then index 8 will be 6 (again because of length).

Now a simple solution is just don't use splice, and use arr[i] = , or alternatively set the length of the array first before doing splice.

KpjComp avatar Nov 02 '21 13:11 KpjComp

The spec says "The specified index MUST NOT be greater than the number of elements in the array."

I think the error here is on the creation of the patch. It should be adding undefined elements before the 7 and the 9.

wickning1 avatar Jun 06 '22 18:06 wickning1

@wickning1 After doing the modification I mention, I've not had any issues with patching arrays.

But what is odd is that this issue is still marked OPEN, and for such a simple bug, with a very simple fix, this worries me a tad. IOW: is this project dead?

KpjComp avatar Jul 12 '22 12:07 KpjComp