Re: [PR] HTTPCLIENT-2386: Fix TLS handshake timeout handling [httpcomponents-core]
arturobernalg closed pull request #541: HTTPCLIENT-2386: Fix TLS handshake timeout handling URL: https://github.com/apache/httpcomponents-core/pull/541 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HTTPCLIENT-2386: Fix TLS handshake timeout handling [httpcomponents-core]
arturobernalg commented on PR #541: URL: https://github.com/apache/httpcomponents-core/pull/541#issuecomment-3145262000 > > > @arturobernalg I do not think we need this. The root cause of the problem is HTTPCLIENT-2386. > > > > > > @ok2c This change is inspired by the discussion in mailing list thread: https://lists.apache.org/thread/11bzsrztjm9y2862wo3x5ysoo9s1f5f7. > > @arturobernalg I am aware. I wrote it. It would be an option if we had not found the cause of the issue. @ok2c 100% agree. Trying to fix the issues on the client meanwhile. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HTTPCLIENT-2386: Fix TLS handshake timeout handling [httpcomponents-core]
ok2c commented on PR #541: URL: https://github.com/apache/httpcomponents-core/pull/541#issuecomment-3145251494 > > @arturobernalg I do not think we need this. The root cause of the problem is HTTPCLIENT-2386. > > @ok2c This change is inspired by the discussion in mailing list thread: https://lists.apache.org/thread/11bzsrztjm9y2862wo3x5ysoo9s1f5f7. @arturobernalg I am aware. I wrote it. It would be an option if we had not found the cause of the issue. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HTTPCLIENT-2386: Fix TLS handshake timeout handling [httpcomponents-core]
arturobernalg commented on PR #541: URL: https://github.com/apache/httpcomponents-core/pull/541#issuecomment-3145218820 > @arturobernalg I do not think we need this. The root cause of the problem is HTTPCLIENT-2386. @ok2c This change is inspired by the discussion in mailing list thread: https://lists.apache.org/thread/11bzsrztjm9y2862wo3x5ysoo9s1f5f7. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HTTPCLIENT-2386: Fix TLS handshake timeout handling [httpcomponents-core]
ok2c commented on PR #541: URL: https://github.com/apache/httpcomponents-core/pull/541#issuecomment-3140966578 @arturobernalg I do not think we need this. The root cause of the problem is HTTPCLIENT-2386. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HTTPCLIENT-2386: Fix TLS handshake timeout handling [httpcomponents-core]
garydgregory commented on code in PR #541:
URL:
https://github.com/apache/httpcomponents-core/pull/541#discussion_r2246081591
##
httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java:
##
@@ -166,10 +167,17 @@ void onTimeout(final Timeout timeout) throws IOException {
if (sessionListener != null) {
sessionListener.timeout(currentSession);
}
+
+final SSLIOSession tlsSession = tlsSessionRef.get();
+if (tlsSession != null && !tlsSession.isHandshakeComplete()) {
+throw new TlsHandshakeTimeoutException("TLS handshake timed out
after " + timeout);
+}
+
final IOEventHandler handler = ensureHandler(currentSession);
handler.timeout(currentSession, timeout);
}
+
Review Comment:
Extra line?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HTTPCLIENT-2386: Fix TLS handshake timeout handling [httpcomponents-core]
garydgregory commented on code in PR #541:
URL:
https://github.com/apache/httpcomponents-core/pull/541#discussion_r2246080997
##
httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java:
##
@@ -166,10 +167,17 @@ void onTimeout(final Timeout timeout) throws IOException {
if (sessionListener != null) {
sessionListener.timeout(currentSession);
}
+
+final SSLIOSession tlsSession = tlsSessionRef.get();
+if (tlsSession != null && !tlsSession.isHandshakeComplete()) {
+throw new TlsHandshakeTimeoutException("TLS handshake timed out
after " + timeout);
Review Comment:
@arturobernalg
I think we should add a timeout only constructor and have the message in
that constructor so it's normalized and not duplicated in each call site.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
