This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 720e613 Try out more consistent error handling
720e613 is described below
commit 720e613bbe529682d725de7c4c1b89a9a1450cad
Author: remm <r...@apache.org>
AuthorDate: Wed Mar 10 16:07:05 2021 +0100
Try out more consistent error handling
Any meaningful SSL call should clear the error stack, then follow up
with checking for errors on <= 0 results. All errors are logged as
debug.
---
.../tomcat/util/net/openssl/OpenSSLEngine.java | 33 +++++++++++++++-------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
index f17ca3a..99720f3 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
@@ -308,7 +308,7 @@ public final class OpenSSLEngine extends SSLEngine
implements SSLUtil.ProtocolIn
}
}
- return -1;
+ return 0;
}
/**
@@ -430,12 +430,16 @@ public final class OpenSSLEngine extends SSLEngine
implements SSLUtil.ProtocolIn
return new
SSLEngineResult(SSLEngineResult.Status.BUFFER_OVERFLOW, handshakeStatus, 0, 0);
}
+ clearLastError();
// Write the pending data from the network BIO into the dst buffer
try {
bytesProduced = readEncryptedData(networkBIO, dst, pendingNet);
} catch (Exception e) {
throw new SSLException(e);
}
+ if (bytesProduced == 0) {
+ checkLastError();
+ }
// If isOutboundDone is set, then the data from the network BIO
// was the close_notify message -- we are not required to wait
@@ -457,12 +461,16 @@ public final class OpenSSLEngine extends SSLEngine
implements SSLUtil.ProtocolIn
}
while (src.hasRemaining()) {
+ clearLastError();
// Write plain text application data to the SSL engine
try {
bytesConsumed += writePlaintextData(ssl, src);
} catch (Exception e) {
throw new SSLException(e);
}
+ if (bytesConsumed == 0) {
+ checkLastError();
+ }
// Check to see if the engine wrote data into the network BIO
pendingNet = SSL.pendingWrittenBytesInBIO(networkBIO);
@@ -474,12 +482,16 @@ public final class OpenSSLEngine extends SSLEngine
implements SSLUtil.ProtocolIn
SSLEngineResult.Status.BUFFER_OVERFLOW,
getHandshakeStatus(), bytesConsumed, bytesProduced);
}
+ clearLastError();
// Write the pending data from the network BIO into the
dst buffer
try {
bytesProduced += readEncryptedData(networkBIO, dst,
pendingNet);
} catch (Exception e) {
throw new SSLException(e);
}
+ if (bytesProduced == 0) {
+ checkLastError();
+ }
return new SSLEngineResult(getEngineStatus(),
getHandshakeStatus(), bytesConsumed, bytesProduced);
}
@@ -541,15 +553,16 @@ public final class OpenSSLEngine extends SSLEngine
implements SSLUtil.ProtocolIn
}
// Write encrypted data to network BIO
- int written = -1;
+ clearLastError();
+ int written = 0;
try {
written = writeEncryptedData(networkBIO, src);
} catch (Exception e) {
throw new SSLException(e);
}
// OpenSSL can return 0 or -1 to these calls if nothing was written
- if (written < 0) {
- written = 0;
+ if (written == 0) {
+ checkLastError();
}
// There won't be any application data until we're done handshaking
@@ -584,6 +597,7 @@ public final class OpenSSLEngine extends SSLEngine
implements SSLUtil.ProtocolIn
break;
}
+ clearLastError();
int bytesRead;
try {
bytesRead = readPlaintextData(ssl, dst);
@@ -591,7 +605,8 @@ public final class OpenSSLEngine extends SSLEngine
implements SSLUtil.ProtocolIn
throw new SSLException(e);
}
- if (bytesRead <= 0) {
+ if (bytesRead == 0) {
+ checkLastError();
// This should not be possible. pendingApp is positive
// therefore the read should have read at least one byte.
throw new
IllegalStateException(sm.getString("engine.failedToReadAvailableBytes"));
@@ -963,12 +978,10 @@ public final class OpenSSLEngine extends SSLEngine
implements SSLUtil.ProtocolIn
* Many calls to SSL methods do not check the last error. Those that do
* check the last error need to ensure that any previously ignored error is
* cleared prior to the method call else errors may be falsely reported.
+ * Ideally, before any SSL_read, SSL_write, clearLastError should always
+ * be called, and getLastError should be called after on any negative or
+ * zero result.
* @return the first error in the stack
- *
- * TODO: Improve error handling. Ideally, before any SSL_read, SSL_write,
- * clearLastError should always be called, and getLastError should be
called
- * after on any negative result.
- *
*/
private static String getLastError() {
String sslError = null;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
