You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2022/12/01 00:48:43 UTC

[GitHub] [trafficserver] serrislew opened a new pull request, #9225: Verbose SSL error logs

serrislew opened a new pull request, #9225:
URL: https://github.com/apache/trafficserver/pull/9225

   Adding more verbose information to SSL error and debug logs
   - Explicitly stating (i.e. "[empty file]") for empty files to avoid confusion 
   
   Including #9222 with slight modifications 


-- 
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

[GitHub] [trafficserver] maskit commented on pull request #9225: Verbose SSL error logs

Posted by GitBox <gi...@apache.org>.
maskit commented on PR #9225:
URL: https://github.com/apache/trafficserver/pull/9225#issuecomment-1334064362

   Just rerunning the test on Fedora worked.


-- 
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

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

Posted by 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

[GitHub] [trafficserver] zwoop commented on pull request #9225: Verbose SSL error logs

Posted by GitBox <gi...@apache.org>.
zwoop commented on PR #9225:
URL: https://github.com/apache/trafficserver/pull/9225#issuecomment-1341793429

   Cherry-picked to v9.2.x


-- 
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

[GitHub] [trafficserver] maskit commented on pull request #9225: Verbose SSL error logs

Posted by GitBox <gi...@apache.org>.
maskit commented on PR #9225:
URL: https://github.com/apache/trafficserver/pull/9225#issuecomment-1334030295

   ```
   ../../../build/_aux/test-driver: line 112: 56759 Segmentation fault      (core dumped) "$@" >> "$log_file" 2>&1
   FAIL: test_QUICLossDetector
   ```
   
   I ran `make check` on my Fedora 36 VM (arm64) but the test passed successfully... I have no idea how this change could affect the test.


-- 
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

[GitHub] [trafficserver] serrislew merged pull request #9225: Verbose SSL error logs

Posted by GitBox <gi...@apache.org>.
serrislew merged PR #9225:
URL: https://github.com/apache/trafficserver/pull/9225


-- 
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

[GitHub] [trafficserver] maskit commented on pull request #9225: Verbose SSL error logs

Posted by GitBox <gi...@apache.org>.
maskit commented on PR #9225:
URL: https://github.com/apache/trafficserver/pull/9225#issuecomment-1334028395

   [approve ci fedora]


-- 
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