Requests are cancelled unexpectedly - connectTimeout limits request duration

[grillbaer]

Describe the bug
Longer-running requests are cancelled unexpectedly after 10 seconds, or Config.connectTimeout() if set, or HttpRequest.connectTimeout() if set. The request then fails (and still returns status 200). I would not expect a connect timeout to limit the request duration and having a 10 second default for this. This also contradicts the quite clear javadoc of Config.connectTimeout().

To Reproduce
Try to download a larger file which takes longer than 10 seconds and see the download failing with an incomplete file and status 200:

import kong.unirest.core.Unirest;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Instant;

public class UnirestConnectionTimeout {
    public static void main(String[] args) throws IOException {
        var largeFileUrl = "https://github.com/szalony9szymek/large/releases/download/free/large";
        var targetFile = Path.of("large-testfile");
        Files.delete(targetFile);

        System.out.println("START " + Instant.now());

        // Unirest.config().connectTimeout(5_000); // override default connect/request timeout of 10s
        Unirest.get(largeFileUrl)
                // .connectTimeout(20_000)  // override the config's connect/request timeout
                .asFile(targetFile.toString())
                .ifSuccess(resp -> System.err.println("SUCCESS status " + resp.getStatus()))
                .ifFailure(resp -> System.err.println("FAILURE status " + resp.getStatus()));

        System.out.println("STOP " + Instant.now() + ", file size " + Files.size(targetFile));
    }
}

Expected behavior
The connect timeout should not limit the request duration. A request timeout should be handled separately and be unlimited by default.

Screenshots
n/a

Environmental Data:

• Java Version 17.0.7
• Version 4.3.2

Additional context
I think the cause is that kong.unirest.core.HttpRequest.getConnectionTimeout() is used for java.net.http.HttpRequest.Builder.timeout(Duration) in kong.unirest.core.java.JavaClient.getRequest(HttpRequest<?>) which acts as a request timeout. Maybe the intention was to replace the formerly existing socket timeout, but this is HttpClient's request timeout.

BTW, thank you for the great Unirest library!

[ryber]

hrm, I think you are right, the timeout on the client and the timeout on the request are different. I'm up for splitting the two but its a breaking change and so will need to be eased into

[ryber]

this is complete in complete in 4.4.0. This splits connection timeout from request timeout. The request no longer has a connection timeout setting, and instead has a request timeout setting as a replacement. Previously these two settings had been conflated. The overall config also has a default request timeout that will be applied to all requests if the request setting is not set. The default setting is null which indicates a infinite timeout.

[grillbaer]

@ryber Thank you very much for fixing this so quickly!

For your information on how the story went on on our side, just in case somebody else runs into the same problems and blames Unirest 4:

Large downloads may still cause problems even without a request timeout. This is due a bug in the JRE itself in java.net.http.HttpClient leading to excessive heap memory consumption for large TLS/SSL downloads: Uncontrolled memory consumption in SSLFlowDelegate.Reader. I our case it consumed hundreds of MB heap for a 2 GB download.

This bug is still present in all JREs from 11 to 21 as of today. We just asked for a backport of the fix into older JRE LTS versions.