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

Potential infinite timeout caused by not setting HttpClientBuilder.setDefaultSocketConfig? #663

Closed
dpoineau opened this issue Dec 13, 2021 · 0 comments · Fixed by #670
Closed

Comments

@dpoineau
Copy link

Issue Summary

Currently in NetworkHttpClient, HttpClientBuilder.setDefaultRequestConfig is used to set socket and connect timeouts for http requests performed by the library. However, per this issue, those settings are not applied to initial SSL handshake or CONNECT requests, which could lead to requests hanging indefinitely: https://issues.apache.org/jira/browse/HTTPCLIENT-1892

From the comments on the issue:

You are using HTTP request configuration, are you not? I do not know what your expectation is but request level configuration applies only once the connection route has been fully established. It does not apply to SSL handshakes or CONNECT requests. You need to configure socket properties applied by the connection manager upon connection creation.

For more context: with the issues that Twilio had on Thursday, Dec 10, we noticed that we were not receiving errors, but that message send requests were hanging indefinitely and never returning. I believe that this was potentially caused by not using setDefaultSocketConfig, which does seem to have an infinite timeout set by default: https://www.javadoc.io/doc/org.apache.httpcomponents/httpcore/latest/org/apache/http/config/SocketConfig.html

Since a pooling connection manager is used, this infinite timeout from setDefaultSocketConfig could have been hit if the manager was needing to create new connections to handle new requests (as opposed to using existing connections that would have had the setDefaultRequestConfig timeouts applied). I can't guarantee that this is what was happening, but I suspect that it may have been in our case.

Requested change

Can the library set a default timeout for this value, by using HttpClientBuilder.setDefaultSocketConfig? I think setting this to the same SOCKET_TIMEOUT that is used for setDefaultRequestConfig would make sense.

Current library timeout setting code

In https://github.com/twilio/twilio-java/blob/main/src/main/java/com/twilio/http/NetworkHttpClient.java#L67

        client = clientBuilder
            .setConnectionManager(connectionManager)
            .setDefaultRequestConfig(config)
            .setDefaultHeaders(headers)
            .setRedirectStrategy(this.getRedirectStrategy())
            .build();

Technical details:

  • twilio-java version: 8.22.1
  • java version: 1.8
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

Successfully merging a pull request may close this issue.

1 participant