Raw Execution Request for DELETE method with null payload is causing a problem

[alfonzjanfrithz]

To reproduce this issue:

IClient client = new ClientBuilder("...").build();
client.execute(HttpMethod.DELETE.name(), "Job", "batch/v1", "test-namespace", "pi", null, null, null);

This leads to a the following problem:

java.lang.IllegalArgumentException: Parameter specified as non-null is null: method okio.Okio.source, parameter $receiver
    at okio.Okio.source(Okio.kt)
    at com.openshift.internal.restclient.DefaultClient$1.writeTo(DefaultClient.java:351)
    at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.kt:59)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:112)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:87)
    at com.openshift.restclient.ClientBuilder$UserAgentInterceptor.intercept(ClientBuilder.java:390)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:112)
    at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.kt:37)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:112)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:87)
    at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.kt:82)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:112)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:87)
    at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:84)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:112)
    at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:71)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:112)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:87)
    at com.openshift.internal.restclient.okhttp.AuthenticatorInterceptor.intercept(AuthenticatorInterceptor.java:62)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:112)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:87)
    at com.openshift.internal.restclient.okhttp.ResponseCodeInterceptor.intercept(ResponseCodeInterceptor.java:57)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:112)
    at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:87)
    at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.kt:184)
    at okhttp3.RealCall.execute(RealCall.kt:66)
    at com.openshift.internal.restclient.DefaultClient.request(DefaultClient.java:318)
    at com.openshift.internal.restclient.DefaultClient.execute(DefaultClient.java:310)
    at com.openshift.internal.restclient.DefaultClient.execute(DefaultClient.java:268)
    at com.openshift.internal.restclient.DefaultClient.execute(DefaultClient.java:256)

The reason why is that the following method in DefaultClient.java does have a mechanism to handle a null payload. When the null is passed, the isPayloadlessMethod(method) only only cater for GET.

Okio.source() can't accept null value

    private RequestBody getPayload(InputStream payload, String method) {
        if(isPayloadlessMethod(method)) {
            return null;
        }
        LOGGER.debug("About to send binary payload");
        return new RequestBody() {
            @Override
            public void writeTo(BufferedSink sink) throws IOException {
                Source source = Okio.source(payload);
                sink.writeAll(source);
            }

            @Override
            public MediaType contentType() {
                return MediaType.parse(MEDIATYPE_APPLICATION_OCTET_STREAM);
            }
        };
    }

Workaround I have to create input stream that has empty string.

client.execute(HttpMethod.DELETE.name(), "Job", "batch/v1", "test-namespace", "pi", null, IOUtils.toInputStream(""), null);

Note The overloaded getPayload() method didnt get this issue, because it handle the null payload.

   private RequestBody getPayload(JSONSerializeable payload, String method) {
        if(isPayloadlessMethod(method)) {
            return null;
        }
        String json = payload == null ? "" : payload.toJson(true);
        LOGGER.debug("About to send payload: {}", json);
        return RequestBody.create(json, MediaType.parse(MEDIATYPE_APPLICATION_JSON));
    }

I am happy to raise the PR for this if needed. Thanks!

[adietish]

Hi @alfonzjanfrithz

Sorry for the late response, I was off on PTO. Gotcha, great that you found this, I wasn't aware at all. And yes, if it's possible for you to create a PR, I'd love to verify it and get it in, appreciate a lot. Ideally your PR would also include unit-tests so that we have the confidence not breaking your fix in future evolutions.

[adietish]

PR was merged, closing.

[adietish]

Hi @alfonzjanfrithz

sorry, for the late reply. Your PR was merged, thanks a lot!