Wiremock: NullPointerException in com.github.tomakehurst.wiremock.matching.EqualToJsonPattern.getNode during record

[mattiasmgn]

The`ret.get(keyInt) in the row
return getNode(ret.get(keyInt), path, ++pos);
will under certain conditions be null because keyInt is larger than the size of ret.

How to reproduce:
Add the two following classes directly on the same level as com.github.tomakehurst.wiremock.WireMockServer and add
compile "commons-io:commons-io:2.4"
compile 'io.rest-assured:rest-assured:3.0.3'

to build.gradle.

package com.github.tomakehurst.wiremock;

import com.github.tomakehurst.wiremock.common.SingleRootFileSource;
import org.apache.commons.io.FileUtils;

import java.io.File;

import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;

public class Buggtest {

public static String URL = "/post";
public static String PROXYBASEURL = "http://httpbin.org";


public static void main(String[] args) {
    new Buggtest().tst();
}
void tst() {
    WireMockServer wireMockServer=null;
    try {
        wireMockServer = new WireMockServer(options().port(8099));
        wireMockServer.start();
        FileUtils.cleanDirectory(new File("/tmp/mappings"));
        FileUtils.cleanDirectory(new File("/tmp/__files"));
        wireMockServer.enableRecordMappings(new SingleRootFileSource("/tmp/mappings"), new SingleRootFileSource("/tmp/__files"));
        configureFor("localhost", 8099);
        stubFor(post(urlEqualTo(URL)).willReturn(aResponse().proxiedFrom(PROXYBASEURL)));
        Caller c = new Caller();
        c.call();
        wireMockServer.stop();
    }
    catch (Exception e){
        e.printStackTrace();
    }
    finally {
        if (wireMockServer!=null && wireMockServer.isRunning()) {
            wireMockServer.stop();
        }
    }
}

}

---

package com.github.tomakehurst.wiremock;

import com.jayway.jsonpath.JsonPath;
import io.restassured.RestAssured;
import io.restassured.response.Response;

import static io.restassured.RestAssured.given;

public class Caller {
void call() {

    String[] request=new String[2];
    request[0] = "{\"columns\": [{\"name\": \"x\",\"y\": 3},{\"name\": \"agreementnumber\",\"a\": 1},{\"name\": \"agreementstatus\",\"b\": 2}]}";
    request[1] = "{\"columns\": [{\"name\": \"agreementnumber\",\"a\": 1},{\"name\": \"utilizerstatus\",\"b\": 2}]}";


    //for(int i=request.length-1;i>=0;i--) {
    for(int i=0;i<request.length;i++) {
        System.out.println("Doing request: " + request[i]);
        Response r = given().contentType("application/json").body(request[i]).when().post("http://localhost:8099"+ Buggtest.URL);
        String body = r.getBody().asString();
        System.out.println("Request response:\n------8<----------------------------" );
        System.out.println(body);
        System.out.println("--------------------------------------->8-------");

    }
}

public static void main(String[] args) {
    Caller c = new Caller();
    c.call();
}

}

---

The second call will result in:
Error 500
Problem accessing /post. Reason:
java.lang.NullPointerException

The error do not happen if you change the order of the calls (reverse the for-loop in class Caller) or if you change columns to columnsZ the second request string.

[mattiasmgn]

Sorry about the formatting in my post. Don't know how to fix it.

[tomakehurst]

Thanks for all the detail you've provided.

[mattiasmgn]

Hi Tom! Thanks for quick response. I hope you can reproduce the bug easily. If you have any problems doing so, don't hesitate to contact me directly.

[tomakehurst]

Thanks. I can't guarantee I'll be able to work on this soon unfortunately.

[mattiasmgn]

OK. I check with you in a while and see how it goes.

[mattiasmgn]

Buggtest.java.txt
Caller.java.txt

[mattiasmgn]

Any news on this?

[Zomis]

This issue is caused by the diff between

expected {"columns":[{"name":"agreementnumber","a":1},{"name":"utilizerstatus","b":2}]}
and
actual {"columns":[{"name":"x","y":3},{"name":"agreementnumber","a":1},{"name":"agreementstatus","b":2}]}

is: [{"op":"add","path":"/columns/0","value":{"name":"x","y":3}},{"op":"replace","path":"/columns/2/name","value":"agreementstatus"}]

The replace option is being scanned without the add-operation having been performed, which leads to /columns/2 being null (it only has 2 elements).

It doesn't seem like any of these operations are actually performed, and the add-operation is skipped at if (!arrayOrderIgnoredAndIsArrayMove(operation, path) && !extraElementsIgnoredAndIsAddition(operation)). (This made me spot another bug which I will add a new issue about)

A possible solution seems to be to replace

if (valueNode == null) {
    acc += deepSize(referencedExpectedNode);
} else {
    acc += maxDeepSize(referencedExpectedNode, valueNode);
}

with

if (valueNode == null) {
    acc += deepSize(referencedExpectedNode);
} else if (referencedExpectedNode == null) {
    acc += deepSize(valueNode);
} else {
    acc += maxDeepSize(referencedExpectedNode, valueNode);
}

And in private static JsonNode getNode(JsonNode ret, List<String> path, int pos) add this as a precondition:

if (ret == null) {
    return null;
}

I do not know if there is any side effect impact of this change, as I'm not sure about the entire purpose of the diffSize calculations. I did the tests and I see nothing that was impacted by this change. A new test should probably be added to make sure this doesn't break again in the future though.

[Zomis]

@tomakehurst Have you read my investigationa above? Any comments?

[tomakehurst]

I've not had time yet, but rest assured I'll post a comment on here as soon as I do.

[foikila]

@tomakehurst Had any time yet? :)

[joakimreichler]

Any news? If you haven't had time to work on this yet, could you please give a rough estimate when you might have time to fix this?

[tomakehurst]

To be honest, given the size of my backlog at the moment, I'm unlikely to look at this in the next four weeks. If you'd like this fixed quicker I can only suggest raising creating a PR, which I'll merge quickly if it's good.

[endrisj]

Did you had a chance to look at the pull request?

[tomakehurst]

Has anyone checked whether this is an issue with the new record feature?