[Enhancement] Unclosed OkHttp Responses in ApiTestActivity

[CyberSecurity-NCC]

Summary

The ApiTestActivity in the sample app contains multiple methods where OkHttp Response objects are obtained but never closed.

• Affected File: app/src/main/java/com/networking/ApiTestActivity.java
• Class: com.networking.ApiTestActivity
• Affected Methods:
  • checkOkHttpResponse(View view)
  • checkSynchronousCall(View view)

Impact

• Misleading Documentation: The current implementation promotes a pattern that causes resource leaks.
• Resource Exhaustion: If these methods are triggered frequently (e.g., rapid button clicks or stress testing), the app may leak file descriptors and network connections, eventually causing the app to crash or network requests to hang.
• Severity: Moderate.

Code Analysis

1. In checkOkHttpResponse, the pattern looks like this:

// Current Implementation
.getAsOkHttpResponse(new OkHttpResponseListener() {
    @Override
    public void onResponse(Response response) {
        if (response.isSuccessful()) {
            // ... logs response.body().source().readUtf8()
            // ISSUE: 'response' is never closed after reading
        }
    }
    // ...
});

2. n checkSynchronousCall, multiple requests are executed sequentially. The Response objects are retrieved to log headers or body but are left open.

// Current Implementation
Response response = responseOne.getOkHttpResponse();
// ... access headers ...
// ISSUE: 'response' is strictly not closed before the next request starts

This pattern repeats for responseTwo, responseThree, and responseFour.

Suggested Fix

Ensure all Response objects are closed. Using try-with-resources (or try-finally) is the recommended approach to handle both success and failure paths safely.

Patch Suggestion (Logic)

For checkOkHttpResponse

@Override
public void onResponse(Response response) {
    if (response == null) return;
    
    // Fix: Use try-with-resources to auto-close the response
    try (Response r = response) {
        if (r.isSuccessful()) {
            // Use r.body().string() for simpler consumption
            Log.d(TAG, "response : " + (r.body() != null ? r.body().string() : "null"));
        } else {
            Log.d(TAG, "response is not successful");
        }
    } catch (IOException e) {
        e.printStackTrace();
    }
}

For checkSynchronousCall

// Example for one of the synchronous blocks
Response response = responseOne.getOkHttpResponse();
try {
    if (response != null) {
        Log.d(TAG, "headers : " + response.headers().toString());
    }
} finally {
    if (response != null) response.close(); // Fix: Explicit close
}

// ... Apply similar try-finally blocks to responseTwo, responseThree, and responseFour

Context & Acknowledgement:

This issue was identified during our academic research on Android resource management. We have manually verified these findings.

Thank you for maintaining Fast-Android-Networking! We hope this report helps.