@abh3 requested changes on this pull request.

Let's skip the changes to XrdHttpProtocol.cc as that code has been working just fine for eons and I don't think there is a problem with the way the code is now. In XrdTlsSocket, let's restrict the changes to the client as the server does not have any issues with the code being the way it is (see comments).

In src/XrdTls/XrdTlsSocket.cc:

> @@ -352,6 +353,14 @@ int XrdTlsSocket::Diagnose(const char *what, int sslrc, int tcode)
                           "TLS error rc=%d ec=%d (%s) errno=%d.",
                           sslrc, eCode, XrdTls::ssl2Text(eCode), eNO);
                  XrdTls::Emsg(pImpl->traceID, eBuff, true);
+                 // Report any additional queued errors
+                 char eBuffSSL[120]; int eQueue;
+                 while ((eQueue = ERR_get_error())) {
+                     ERR_error_string_n(eQueue, eBuffSSL, sizeof(eBuffSSL));
+                     snprintf(eBuff, sizeof(eBuff),
+                              "TLS error queue: ec=%d, %s", eQueue, eBuffSSL);
+                     XrdTls::Emsg(pImpl->traceID, eBuff, true);
+                 }

This whole section is not needed. The call to XrdTls::Emsg(0 does this alreay.

In src/XrdTls/XrdTlsSocket.cc:

> @@ -640,7 +649,8 @@ XrdTls::RC XrdTlsSocket::Read( char *buffer, size_t size, int &bytesRead )
     // have to explicitly call SSL_connect or SSL_do_handshake.
     //------------------------------------------------------------------------
 
- do{int rc = SSL_read( pImpl->ssl, buffer, size );
+ do{ERR_clear_error();

Frankly, the server does not have this issue as it seems to do quite well with post cleanup. I am still rather suprised it doesn't work for the client but then again the threading model is different. So, let's start with...

if (pImpl->isClient) ERR_clear_error();

In src/XrdTls/XrdTlsSocket.cc:

> @@ -785,7 +795,8 @@ XrdTls::RC XrdTlsSocket::Write( const char *buffer, size_t size,
     // have to explicitly call SSL_connect or SSL_do_handshake.
     //------------------------------------------------------------------------
 
- do{int rc = SSL_write( pImpl->ssl, buffer, size );
+ do{ERR_clear_error();

if (pImpl->isClient) ERR_clear_error(); // See above

In src/XrdTls/XrdTlsSocket.cc:

> @@ -178,7 +178,8 @@ XrdTls::RC XrdTlsSocket::Accept(std::string *eWhy)
 
 // An accept may require several tries, so we do that here.
 //
-do{if ((rc = SSL_accept( pImpl->ssl )) > 0)
+do{ERR_clear_error();

I would remove this. The server has never had an issue with Accept(). I suppose the overhead here is miniscule but I'm just trying to be consistent. Oddly, why wasn't the error queue cleared for connect()?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1