[GitHub] [trafficserver] mlibbey commented on a diff in pull request #9225: Verbose SSL error logs

[GitBox <gi...@apache.org>]

mlibbey commented on code in PR #9225:
URL: https://github.com/apache/trafficserver/pull/9225#discussion_r1037340519


##########
iocore/net/SSLUtils.cc:
##########
@@ -2222,16 +2233,19 @@ SSLMultiCertConfigLoader::load_certs_and_cross_reference_names(
     key_tok.setString("");
   }
 
-  if (sslMultCertSettings && sslMultCertSettings->key && cert_tok.getNumTokensRemaining() != key_tok.getNumTokensRemaining()) {
-    Error("the number of certificates in ssl_cert_name and ssl_key_name doesn't match");
+  size_t cert_tok_num = cert_tok.getNumTokensRemaining();
+  if (sslMultCertSettings && sslMultCertSettings->key && cert_tok_num != key_tok.getNumTokensRemaining()) {
+    Error("the number of certificates in ssl_cert_name (%zu) and ssl_key_name (%zu) doesn't match", cert_tok_num,

Review Comment:
   Can we change "doesn't" to "do not" (think the latter is better grammar).
   



##########
iocore/net/SSLUtils.cc:
##########
@@ -2222,16 +2233,19 @@ SSLMultiCertConfigLoader::load_certs_and_cross_reference_names(
     key_tok.setString("");
   }
 
-  if (sslMultCertSettings && sslMultCertSettings->key && cert_tok.getNumTokensRemaining() != key_tok.getNumTokensRemaining()) {
-    Error("the number of certificates in ssl_cert_name and ssl_key_name doesn't match");
+  size_t cert_tok_num = cert_tok.getNumTokensRemaining();
+  if (sslMultCertSettings && sslMultCertSettings->key && cert_tok_num != key_tok.getNumTokensRemaining()) {
+    Error("the number of certificates in ssl_cert_name (%zu) and ssl_key_name (%zu) doesn't match", cert_tok_num,
+          key_tok.getNumTokensRemaining());
     return false;
   }
 
   SimpleTokenizer ca_tok("", SSL_CERT_SEPARATE_DELIM);
   if (sslMultCertSettings && sslMultCertSettings->ca) {
     ca_tok.setString(sslMultCertSettings->ca);
     if (cert_tok.getNumTokensRemaining() != ca_tok.getNumTokensRemaining()) {
-      Error("the number of certificates in ssl_cert_name and ssl_ca_name doesn't match");
+      Error("the number of certificates in ssl_cert_name (%zu) and ssl_ca_name (%zu) doesn't match", cert_tok_num,

Review Comment:
   doesn't --> do not



##########
iocore/net/SSLSecret.cc:
##########
@@ -55,12 +55,14 @@ SSLSecret::loadFile(const std::string &name, std::string &data_item)
   struct stat statdata;
   // Load the secret and add it to the map
   if (stat(name.c_str(), &statdata) < 0) {
+    Debug("ssl_secret", "File does not exist or cannot be found: %s", name.c_str());

Review Comment:
   Perhaps a bike shed comment... I'm struggling to think of the difference between doesn't exist and "cannot be found". Would "cannot be read" (for instance a file ownership or permissions issue) describe an issue caught in this if statement?



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org