dart:io | Change names for SecureSocket exceptions.

runtime/bin/secure_socket.cc

Index: runtime/bin/secure_socket.cc
diff --git a/runtime/bin/secure_socket.cc b/runtime/bin/secure_socket.cc
index 718b04f480c9348434822242bfef79cd655af33d..7b35f9d2999c951cd78a7973244034572a9a6df6 100644
--- a/runtime/bin/secure_socket.cc
+++ b/runtime/bin/secure_socket.cc
@@ -50,6 +50,19 @@ NativeService SSLFilter::filter_service_("FilterService", ProcessFilter, 16);
 
 static const int kSSLFilterNativeFieldIndex = 0;
 
+
+/* Handle an error reported from the NSS library. */
+static void ThrowPRException(const char* exception_type, const char* message) {
+  PRErrorCode error_code = PR_GetError();
+  const char* error_message = PR_ErrorToString(error_code, PR_LANGUAGE_EN);
+  OSError os_error_struct(error_code, error_message, OSError::kNSS);
+  Dart_Handle os_error = DartUtils::NewDartOSError(&os_error_struct);
+  Dart_Handle exception =
+      DartUtils::NewDartIOException(exception_type, message, os_error);
+  Dart_ThrowException(exception);
+}
+
+
 static SSLFilter* GetFilter(Dart_NativeArguments args) {
   SSLFilter* filter;
   Dart_Handle dart_this = ThrowIfError(Dart_GetNativeArgument(args, 0));
@@ -369,7 +382,8 @@ static Dart_Handle X509FromCertificate(CERTCertificate* certificate) {
   SECStatus status =
       CERT_GetCertTimes(certificate, &start_validity, &end_validity);
   if (status != SECSuccess) {
-    ThrowPRException("Cannot get validity times from certificate");
+    ThrowPRException("CertificateException",
+                     "Cannot get validity times from certificate");
   }
   int64_t start_epoch_ms = start_validity / PR_USEC_PER_MSEC;
   int64_t end_epoch_ms = end_validity / PR_USEC_PER_MSEC;
@@ -434,12 +448,10 @@ void SSLFilter::InitializeBuffers(Dart_Handle dart_this) {
   int64_t encrypted_buffer_size =
       DartUtils::GetIntegerValue(dart_encrypted_buffer_size);
   if (buffer_size <= 0 || buffer_size > 1024 * 1024) {

[Anders Johnsen, 2013/06/25 05:55:03]

I think these numbers deserve a constant.

[Bill Hesse, 2013/06/25 12:41:14]

The upper one is just a sanity check - there is no real justification for this
value.  Changed it to 1 MB.

Also added check against out-of-range access to these buffers.  buffer_size_ is
in C++, and immune to Dart after this point, but start and end positions are now
range checked, in line 316.

-    Dart_ThrowException(
-        DartUtils::NewString("Invalid buffer size in _ExternalBuffer"));
+    FATAL("Invalid buffer size in _ExternalBuffer");
   }
   if (encrypted_buffer_size <= 0 || encrypted_buffer_size > 1024 * 1024) {
-    Dart_ThrowException(DartUtils::NewString(
-        "Invalid encrypted buffer size in _ExternalBuffer"));
+    FATAL("Invalid encrypted buffer size in _ExternalBuffer");
   }
   buffer_size_ = static_cast<int>(buffer_size);
   encrypted_buffer_size_ = static_cast<int>(encrypted_buffer_size);
@@ -503,14 +515,16 @@ void SSLFilter::InitializeLibrary(const char* certificate_database,
       status = NSS_NoDB_Init(NULL);
       if (status != SECSuccess) {
         mutex_.Unlock();  // MutexLocker destructor not called when throwing.
-        ThrowPRException("Failed NSS_NoDB_Init call.");
+        ThrowPRException("TlsException",
+                         "Failed NSS_NoDB_Init call.");
       }
       if (use_builtin_root_certificates) {
         SECMODModule* module = SECMOD_LoadUserModule(
             const_cast<char*>(builtin_roots_module), NULL, PR_FALSE);
         if (!module) {
           mutex_.Unlock();  // MutexLocker destructor not called when throwing.
-          ThrowPRException("Failed to load builtin root certificates.");
+          ThrowPRException("TlsException",
+                           "Failed to load builtin root certificates.");
         }
       }
     } else {
@@ -525,7 +539,8 @@ void SSLFilter::InitializeLibrary(const char* certificate_database,
                               init_flags);
       if (status != SECSuccess) {
         mutex_.Unlock();  // MutexLocker destructor not called when throwing.
-        ThrowPRException("Failed NSS_Init call.");
+        ThrowPRException("TlsException",
+                         "Failed NSS_Init call.");
       }
     }
     library_initialized_ = true;
@@ -533,23 +548,29 @@ void SSLFilter::InitializeLibrary(const char* certificate_database,
     status = NSS_SetDomesticPolicy();
     if (status != SECSuccess) {
       mutex_.Unlock();  // MutexLocker destructor not called when throwing.
-      ThrowPRException("Failed NSS_SetDomesticPolicy call.");
+      ThrowPRException("TlsException",
+                       "Failed NSS_SetDomesticPolicy call.");
     }
     // Enable TLS, as well as SSL3 and SSL2.
     status = SSL_OptionSetDefault(SSL_ENABLE_TLS, PR_TRUE);
     if (status != SECSuccess) {
       mutex_.Unlock();  // MutexLocker destructor not called when throwing.
-      ThrowPRException("Failed SSL_OptionSetDefault enable TLS call.");
+      ThrowPRException("TlsException",
+                       "Failed SSL_OptionSetDefault enable TLS call.");
     }
     status = SSL_ConfigServerSessionIDCache(0, 0, 0, NULL);
     if (status != SECSuccess) {
       mutex_.Unlock();  // MutexLocker destructor not called when throwing.
-      ThrowPRException("Failed SSL_ConfigServerSessionIDCache call.");
+      ThrowPRException("TlsException",
+                       "Failed SSL_ConfigServerSessionIDCache call.");
     }
 
   } else if (report_duplicate_initialization) {
     mutex_.Unlock();  // MutexLocker destructor not called when throwing.
-    ThrowException("Called SSLFilter::InitializeLibrary more than once");
+    // Like ThrowPRException, without adding an OSError.
+    Dart_ThrowException(DartUtils::NewDartIOException("TlsException",
+        "Called SSLFilter::InitializeLibrary more than once",

[Anders Johnsen, 2013/06/25 05:55:03]

:: -> ., we don't have :: in Dart.

[Bill Hesse, 2013/06/25 12:41:14]

Changed message, and added documentation to SecureSocket.initialize that this
exception is thrown if it is called twice.  This is a useful error for
developers, because they need to know if they are trying to initialize twice,
perhaps with different settings.

+        Dart_Null()));
   }
 }
 
@@ -596,7 +617,7 @@ void SSLFilter::Connect(const char* host_name,
                         bool send_client_certificate) {
   is_server_ = is_server;
   if (in_handshake_) {
-    ThrowException("Connect called while already in handshake state.");
+    FATAL("Connect called twice on the same _SecureFilter.");
   }
 
   if (!is_server && certificate_name != NULL) {
@@ -605,7 +626,7 @@ void SSLFilter::Connect(const char* host_name,
 
   filter_ = SSL_ImportFD(NULL, filter_);
   if (filter_ == NULL) {
-    ThrowPRException("Failed SSL_ImportFD call");
+    ThrowPRException("TlsException", "Failed SSL_ImportFD call");
   }
 
   SSLVersionRange vrange;
@@ -622,12 +643,13 @@ void SSLFilter::Connect(const char* host_name,
       // Look up certificate using the distinguished name (DN) certificate_name.
       CERTCertDBHandle* certificate_database = CERT_GetDefaultCertDB();
       if (certificate_database == NULL) {
-        ThrowPRException("Certificate database cannot be loaded");
+        ThrowPRException("CertificateException",
+                         "Certificate database cannot be loaded");
       }
       certificate = CERT_FindCertByNameString(certificate_database,
           const_cast<char*>(certificate_name));
       if (certificate == NULL) {
-        ThrowPRException(
+        ThrowPRException("CertificateException",
             "Cannot find server certificate by distinguished name");
       }
     } else {
@@ -636,7 +658,8 @@ void SSLFilter::Connect(const char* host_name,
           const_cast<char*>(certificate_name),
           static_cast<void*>(const_cast<char*>(password_)));
       if (certificate == NULL) {
-        ThrowPRException("Cannot find server certificate by nickname");
+        ThrowPRException("CertificateException",
+                         "Cannot find server certificate by nickname");
       }
     }
     SECKEYPrivateKey* key = PK11_FindKeyByAnyCert(
@@ -645,9 +668,11 @@ void SSLFilter::Connect(const char* host_name,
     if (key == NULL) {
       CERT_DestroyCertificate(certificate);
       if (PR_GetError() == -8177) {
-        ThrowPRException("Certificate database password incorrect");
+        ThrowPRException("CertificateException",
+                         "Certificate database password incorrect");
       } else {
-        ThrowPRException("Failed PK11_FindKeyByAnyCert call."
+        ThrowPRException("CertificateException",
+                         "Failed PK11_FindKeyByAnyCert call."
                          " Cannot find private key for certificate");
       }
     }
@@ -657,23 +682,27 @@ void SSLFilter::Connect(const char* host_name,
     CERT_DestroyCertificate(certificate);
     SECKEY_DestroyPrivateKey(key);
     if (status != SECSuccess) {
-      ThrowPRException("Failed SSL_ConfigSecureServer call");
+      ThrowPRException("CertificateException",
+                       "Failed SSL_ConfigSecureServer call");
     }
 
     if (request_client_certificate) {
       status = SSL_OptionSet(filter_, SSL_REQUEST_CERTIFICATE, PR_TRUE);
       if (status != SECSuccess) {
-        ThrowPRException("Failed SSL_OptionSet(REQUEST_CERTIFICATE) call");
+        ThrowPRException("TlsException",

[Anders Johnsen, 2013/06/25 05:55:03]

Here and below, it looks like it should be CertificateException.

[Søren Gjesse, 2013/06/25 06:35:49]

I think TlsException is fine here. I see CertificateException as related to the
validity/validation of a certificate whereas TlsException is releated to issues
with the TLS (NSS/SSL) "subsystem".

+                         "Failed SSL_OptionSet(REQUEST_CERTIFICATE) call");
       }
       PRBool require_cert = require_client_certificate ? PR_TRUE : PR_FALSE;
       status = SSL_OptionSet(filter_, SSL_REQUIRE_CERTIFICATE, require_cert);
       if (status != SECSuccess) {
-        ThrowPRException("Failed SSL_OptionSet(REQUIRE_CERTIFICATE) call");
+        ThrowPRException("TlsException",
+                         "Failed SSL_OptionSet(REQUIRE_CERTIFICATE) call");
       }
     }
   } else {  // Client.
     if (SSL_SetURL(filter_, host_name) == -1) {
-      ThrowPRException("Failed SetURL call");
+      ThrowPRException("TlsException",
+                       "Failed SetURL call");
     }
 
     // This disables the SSL session cache for client connections.
@@ -681,7 +710,8 @@ void SSLFilter::Connect(const char* host_name,
     // TODO(7230): Reenable session cache, without breaking client connections.
     status = SSL_OptionSet(filter_, SSL_NO_CACHE, PR_TRUE);
     if (status != SECSuccess) {
-      ThrowPRException("Failed SSL_OptionSet(NO_CACHE) call");
+      ThrowPRException("TlsException",
+                       "Failed SSL_OptionSet(NO_CACHE) call");
     }
 
     if (send_client_certificate) {
@@ -690,7 +720,8 @@ void SSLFilter::Connect(const char* host_name,
           NSS_GetClientAuthData,
           static_cast<void*>(client_certificate_name_));
       if (status != SECSuccess) {
-        ThrowPRException("Failed SSL_GetClientAuthDataHook call");
+        ThrowPRException("TlsException",
+                         "Failed SSL_GetClientAuthDataHook call");
       }
     }
   }
@@ -703,7 +734,8 @@ void SSLFilter::Connect(const char* host_name,
   PRBool as_server = is_server ? PR_TRUE : PR_FALSE;
   status = SSL_ResetHandshake(filter_, as_server);
   if (status != SECSuccess) {
-    ThrowPRException("Failed SSL_ResetHandshake call");
+    ThrowPRException("TlsException",
+                     "Failed SSL_ResetHandshake call");
   }
 
   // Set the peer address from the address passed. The DNS has already
@@ -743,9 +775,11 @@ void SSLFilter::Handshake() {
       }
     } else {
       if (is_server_) {
-        ThrowPRException("Unexpected handshake error in server");
+        ThrowPRException("HandshakeException",

[Søren Gjesse, 2013/06/25 06:35:49]

We could consider dropping HandshakeException and just use TlsException here.

[Bill Hesse, 2013/06/25 12:41:14]

Let's see how it goes.  I think we may also get exceptions after handshake,
during the connection, and it would be nice to distinguish them.

+                         "Handshake error in server");
       } else {
-        ThrowPRException("Unexpected handshake error in client");
+        ThrowPRException("HandshakeException",
+                         "Handshake error in client");
       }
     }
   }