Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected result while applying copy operation #125

Open
jiangsuxd opened this issue Dec 18, 2020 · 1 comment
Open

Unexpected result while applying copy operation #125

jiangsuxd opened this issue Dec 18, 2020 · 1 comment

Comments

@jiangsuxd
Copy link

jiangsuxd commented Dec 18, 2020

As the original source and target json file size is too big, here I just describe the problem because the bug is explicit.
The generated copy operation is {"op": "copy", "from": "/data/109/id", "path": "/data/75/id"} which would cause wrong copied value when one or more item of 'data' array has been removed. For example, when /data/50 was removed by remove operation before copy, then /data/75/id will actually get the value from /data/110/id rather than /data/109/id.

I noticed you have already come across similar problem and provided JsonDiff.isAllowed() function(attached below) to try to solve it. But there exists an bug in the function. when both srcStr and dstStr are numeric, we should compare their number value rather than string value. 109 > 75, but "109" < "75" which cause JsonDiff.isAllowed(JsonPointer.parse("/data/109/id"), JsonPointer.parse("/data/75/id")) passed this function.

private static boolean isAllowed(JsonPointer source, JsonPointer destination) {
    boolean isSame = source.equals(destination);
    int i = 0;
    int j = 0;
    // Hack to fix broken COPY operation, need better handling here
    while (i < source.size() && j < destination.size()) {
        JsonPointer.RefToken srcValue = source.get(i);
        JsonPointer.RefToken dstValue = destination.get(j);
        String srcStr = srcValue.toString();
        String dstStr = dstValue.toString();
        if (isNumber(srcStr) && isNumber(dstStr)) {
            if (srcStr.compareTo(dstStr) > 0) {
                return false;
            }
        }
        i++;
        j++;
    }
    return !isSame;
}

Specifications

Java 1.8 zjsonpatch 0.4.11

@jiangsuxd
Copy link
Author

I think this function only solved 'array index out of bound' exception although number comparison is fixed. There still exists array index disalignment problem when element is removed from array before copy operation applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant