Bug 1340854 - Properly report TLS handshake telemetry for 0 length reads. r=mt
authorEKR <ekr@rtfm.com>
Sat, 18 Feb 2017 11:27:21 -0800
changeset 486819 356449a93a4c2f3262b982d3fd621d880c919ac9
parent 486818 12fae55fe512dfd640b09279821d49b5f9e03a8a
child 486820 b0cbe51520865de381809966b309ecefeba78f22
push id46066
push userbmo:gasolin@mozilla.com
push dateMon, 20 Feb 2017 01:56:10 +0000
reviewersmt
bugs1340854
milestone54.0a1
Bug 1340854 - Properly report TLS handshake telemetry for 0 length reads. r=mt
security/manager/ssl/nsNSSIOLayer.cpp
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -1153,22 +1153,28 @@ static_assert((SSL_ERROR_END_OF_LIST - S
 static_assert((SEC_ERROR_END_OF_LIST - SEC_ERROR_BASE) <= 256,
               "too many SEC errors");
 static_assert((PR_MAX_ERROR - PR_NSPR_ERROR_BASE) <= 128,
               "too many NSPR errors");
 static_assert((mozilla::pkix::ERROR_BASE - mozilla::pkix::END_OF_LIST) < 31,
               "too many moz::pkix errors");
 
 static void
-reportHandshakeResult(int32_t bytesTransferred, PRErrorCode err)
+reportHandshakeResult(int32_t bytesTransferred, bool wasReading, PRErrorCode err)
 {
   uint32_t bucket;
 
-  if (bytesTransferred >= 0) {
+  // A negative bytesTransferred or a 0 read are errors.
+  if (bytesTransferred > 0) {
     bucket = 0;
+  } else if ((bytesTransferred == 0) && !wasReading) {
+    // PR_Write() is defined to never return 0, but let's make sure.
+    // https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSPR/Reference/PR_Write.
+    MOZ_ASSERT(false);
+    bucket = 671;
   } else if (IS_SSL_ERROR(err)) {
     bucket = err - SSL_ERROR_BASE;
     MOZ_ASSERT(bucket > 0);   // SSL_ERROR_EXPORT_ONLY_SERVER isn't used.
   } else if (IS_SEC_ERROR(err)) {
     bucket = (err - SEC_ERROR_BASE) + 256;
   } else if ((err >= PR_NSPR_ERROR_BASE) && (err < PR_MAX_ERROR)) {
     bucket = (err - PR_NSPR_ERROR_BASE) + 512;
   } else if ((err >= mozilla::pkix::ERROR_BASE) &&
@@ -1256,17 +1262,17 @@ checkHandshake(int32_t bytesTransfered, 
 
   // TLS intolerant servers only cause the first transfer to fail, so let's
   // set the HandshakePending attribute to false so that we don't try the logic
   // above again in a subsequent transfer.
   if (handleHandshakeResultNow) {
     // Report the result once for each handshake. Note that this does not
     // get handshakes which are cancelled before any reads or writes
     // happen.
-    reportHandshakeResult(bytesTransfered, originalError);
+    reportHandshakeResult(bytesTransfered, wasReading, originalError);
     socketInfo->SetHandshakeNotPending();
   }
 
   if (bytesTransfered < 0) {
     // Remember that we encountered an error so that getSocketInfoIfRunning
     // will correctly cause us to fail if another part of Gecko
     // (erroneously) calls an I/O function (PR_Send/PR_Recv/etc.) again on
     // this socket. Note that we use the original error because if we use