Check for errors without object allocation
Motivation: At the moment we use SSL.getLastError() in unwrap(...) to check for error. This is very inefficient as it creates a new String for each check and we also use a String.startsWith(...) to detect if there was an error we need to handle. Modifications: Use SSL.getLastErrorNumber() to detect if we need to handle an error, as this only returns a long and so no String creation happens. Also the detection is much cheaper as we can now only compare longs. Once an error is detected the lately SSL.getErrorString(long) is used to conver the error number to a String and include it in log and exception message. Result: Performance improvements in OpenSslEngine.unwrap(...) due less object allocation and also faster comparations.
This commit is contained in:
parent
f13a3fbefb
commit
eeb5e2c0a9
@ -30,7 +30,6 @@ public final class OpenSsl {
|
|||||||
|
|
||||||
private static final InternalLogger logger = InternalLoggerFactory.getInstance(OpenSsl.class);
|
private static final InternalLogger logger = InternalLoggerFactory.getInstance(OpenSsl.class);
|
||||||
private static final Throwable UNAVAILABILITY_CAUSE;
|
private static final Throwable UNAVAILABILITY_CAUSE;
|
||||||
private static final String IGNORABLE_ERROR_PREFIX = "error:00000000:";
|
|
||||||
|
|
||||||
static {
|
static {
|
||||||
Throwable cause = null;
|
Throwable cause = null;
|
||||||
@ -79,8 +78,8 @@ public final class OpenSsl {
|
|||||||
return UNAVAILABILITY_CAUSE;
|
return UNAVAILABILITY_CAUSE;
|
||||||
}
|
}
|
||||||
|
|
||||||
static boolean isError(String error) {
|
static boolean isError(long errorCode) {
|
||||||
return error != null && !error.startsWith(IGNORABLE_ERROR_PREFIX);
|
return errorCode != SSL.SSL_ERROR_NONE;
|
||||||
}
|
}
|
||||||
|
|
||||||
private OpenSsl() { }
|
private OpenSsl() { }
|
||||||
|
@ -115,11 +115,11 @@ public final class OpenSslClientContext extends OpenSslContext {
|
|||||||
if (certChainFile != null) {
|
if (certChainFile != null) {
|
||||||
/* Load the certificate chain. We must skip the first cert when server mode */
|
/* Load the certificate chain. We must skip the first cert when server mode */
|
||||||
if (!SSLContext.setCertificateChainFile(ctx, certChainFile.getPath(), true)) {
|
if (!SSLContext.setCertificateChainFile(ctx, certChainFile.getPath(), true)) {
|
||||||
String error = SSL.getLastError();
|
long error = SSL.getLastErrorNumber();
|
||||||
if (OpenSsl.isError(error)) {
|
if (OpenSsl.isError(error)) {
|
||||||
throw new SSLException(
|
throw new SSLException(
|
||||||
"failed to set certificate chain: "
|
"failed to set certificate chain: "
|
||||||
+ certChainFile + " (" + SSL.getLastError() + ')');
|
+ certChainFile + " (" + SSL.getErrorString(error) + ')');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -504,17 +504,18 @@ public final class OpenSslEngine extends SSLEngine {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Check for OpenSSL errors caused by the priming read
|
// Check for OpenSSL errors caused by the priming read
|
||||||
String error = SSL.getLastError();
|
long error = SSL.getLastErrorNumber();
|
||||||
if (OpenSsl.isError(error)) {
|
if (OpenSsl.isError(error)) {
|
||||||
|
String err = SSL.getErrorString(error);
|
||||||
if (logger.isInfoEnabled()) {
|
if (logger.isInfoEnabled()) {
|
||||||
logger.info(
|
logger.info(
|
||||||
"SSL_read failed: primingReadResult: " + lastPrimingReadResult +
|
"SSL_read failed: primingReadResult: " + lastPrimingReadResult +
|
||||||
"; OpenSSL error: '" + error + '\'');
|
"; OpenSSL error: '" + err + '\'');
|
||||||
}
|
}
|
||||||
|
|
||||||
// There was an internal error -- shutdown
|
// There was an internal error -- shutdown
|
||||||
shutdown();
|
shutdown();
|
||||||
throw new SSLException(error);
|
throw new SSLException(err);
|
||||||
}
|
}
|
||||||
|
|
||||||
// There won't be any application data until we're done handshaking
|
// There won't be any application data until we're done handshaking
|
||||||
@ -953,16 +954,17 @@ public final class OpenSslEngine extends SSLEngine {
|
|||||||
int code = SSL.doHandshake(ssl);
|
int code = SSL.doHandshake(ssl);
|
||||||
if (code <= 0) {
|
if (code <= 0) {
|
||||||
// Check for OpenSSL errors caused by the handshake
|
// Check for OpenSSL errors caused by the handshake
|
||||||
String error = SSL.getLastError();
|
long error = SSL.getLastErrorNumber();
|
||||||
if (OpenSsl.isError(error)) {
|
if (OpenSsl.isError(error)) {
|
||||||
|
String err = SSL.getErrorString(error);
|
||||||
if (logger.isInfoEnabled()) {
|
if (logger.isInfoEnabled()) {
|
||||||
logger.info(
|
logger.info(
|
||||||
"SSL_do_handshake failed: OpenSSL error: '" + error + '\'');
|
"SSL_do_handshake failed: OpenSSL error: '" + err + '\'');
|
||||||
}
|
}
|
||||||
|
|
||||||
// There was an internal error -- shutdown
|
// There was an internal error -- shutdown
|
||||||
shutdown();
|
shutdown();
|
||||||
throw new SSLException(error);
|
throw new SSLException(err);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -189,10 +189,11 @@ public final class OpenSslServerContext extends OpenSslContext {
|
|||||||
|
|
||||||
/* Load the certificate chain. We must skip the first cert when server mode */
|
/* Load the certificate chain. We must skip the first cert when server mode */
|
||||||
if (!SSLContext.setCertificateChainFile(ctx, certChainFile.getPath(), true)) {
|
if (!SSLContext.setCertificateChainFile(ctx, certChainFile.getPath(), true)) {
|
||||||
String error = SSL.getLastError();
|
long error = SSL.getLastErrorNumber();
|
||||||
if (OpenSsl.isError(error)) {
|
if (OpenSsl.isError(error)) {
|
||||||
|
String err = SSL.getErrorString(error);
|
||||||
throw new SSLException(
|
throw new SSLException(
|
||||||
"failed to set certificate chain: " + certChainFile + " (" + SSL.getLastError() + ')');
|
"failed to set certificate chain: " + certChainFile + " (" + err + ')');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -200,8 +201,12 @@ public final class OpenSslServerContext extends OpenSslContext {
|
|||||||
try {
|
try {
|
||||||
if (!SSLContext.setCertificate(
|
if (!SSLContext.setCertificate(
|
||||||
ctx, certChainFile.getPath(), keyFile.getPath(), keyPassword, SSL.SSL_AIDX_RSA)) {
|
ctx, certChainFile.getPath(), keyFile.getPath(), keyPassword, SSL.SSL_AIDX_RSA)) {
|
||||||
throw new SSLException("failed to set certificate: " +
|
long error = SSL.getLastErrorNumber();
|
||||||
certChainFile + " and " + keyFile + " (" + SSL.getLastError() + ')');
|
if (OpenSsl.isError(error)) {
|
||||||
|
String err = SSL.getErrorString(error);
|
||||||
|
throw new SSLException("failed to set certificate: " +
|
||||||
|
certChainFile + " and " + keyFile + " (" + err + ')');
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} catch (SSLException e) {
|
} catch (SSLException e) {
|
||||||
throw e;
|
throw e;
|
||||||
|
Loading…
Reference in New Issue
Block a user