dart:io | Change names for SecureSocket exceptions.

dart:io | Change names for SecureSocket exceptions.

BUG=
R=ajohnsen@google.com, sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=24405

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

lgtm

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc
File runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc...
runtime/bin/secure_socket.cc:450: if (buffer_size <= 0 || buffer_size > 1024 *
1024) {
I think these numbers deserve a constant.

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc...
runtime/bin/secure_socket.cc:572: "Called SSLFilter::InitializeLibrary more than
once",
:: -> ., we don't have :: in Dart.

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc...
runtime/bin/secure_socket.cc:692: ThrowPRException("TlsException",
Here and below, it looks like it should be CertificateException.

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dart
File sdk/lib/io/secure_socket.dart (right):

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dar...
sdk/lib/io/secure_socket.dart:1189: final String type;
Move to top.

https://codereview.chromium.org/17589007/diff/11/tests/standalone/io/raw_secu...
File tests/standalone/io/raw_secure_server_socket_argument_test.dart (right):

https://codereview.chromium.org/17589007/diff/11/tests/standalone/io/raw_secu...
tests/standalone/io/raw_secure_server_socket_argument_test.dart:26:
Expect.throws(() => RawSecureSocket.connect(SERVER_ADDRESS, 3456,
maybe add a validator for the right error?

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

LGTM

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc
File runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc...
runtime/bin/secure_socket.cc:692: ThrowPRException("TlsException",
On 2013/06/25 05:55:03, Anders Johnsen wrote:
> Here and below, it looks like it should be CertificateException.

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

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc...
runtime/bin/secure_socket.cc:778: ThrowPRException("HandshakeException",
We could consider dropping HandshakeException and just use TlsException here.

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dart
File sdk/lib/io/secure_socket.dart (right):

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dar...
sdk/lib/io/secure_socket.dart:544: static void _verifyFields(host,
Indentation.

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dar...
sdk/lib/io/secure_socket.dart:1198: */
As mentioned before maybe just drop HandshakeException. I am not sure whether
failure in the handshake phase needs a separate exception.

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dar...
sdk/lib/io/secure_socket.dart:1210: */
Ca we add some more information to this exception about the certificate causing
the problem in addition to the error text and code from NSS?

[Bill Hesse, 2013-06-25 12:41:13]

Please take a look at ThrowCertificateException in secure_socket.cc - the other
changes are addressing comments.

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc
File runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc...
runtime/bin/secure_socket.cc:450: if (buffer_size <= 0 || buffer_size > 1024 *
1024) {
On 2013/06/25 05:55:03, Anders Johnsen wrote:
> I think these numbers deserve a constant.

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.

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc...
runtime/bin/secure_socket.cc:572: "Called SSLFilter::InitializeLibrary more than
once",
On 2013/06/25 05:55:03, Anders Johnsen wrote:
> :: -> ., we don't have :: in Dart.

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.

https://codereview.chromium.org/17589007/diff/11/runtime/bin/secure_socket.cc...
runtime/bin/secure_socket.cc:778: ThrowPRException("HandshakeException",
On 2013/06/25 06:35:49, Søren Gjesse wrote:
> We could consider dropping HandshakeException and just use TlsException here.

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.

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dart
File sdk/lib/io/secure_socket.dart (right):

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dar...
sdk/lib/io/secure_socket.dart:544: static void _verifyFields(host,
On 2013/06/25 06:35:49, Søren Gjesse wrote:
> Indentation.

Done.

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dar...
sdk/lib/io/secure_socket.dart:1189: final String type;
On 2013/06/25 05:55:03, Anders Johnsen wrote:
> Move to top.

Done.

https://codereview.chromium.org/17589007/diff/11/sdk/lib/io/secure_socket.dar...
sdk/lib/io/secure_socket.dart:1210: */
On 2013/06/25 06:35:49, Søren Gjesse wrote:
> Ca we add some more information to this exception about the certificate
causing
> the problem in addition to the error text and code from NSS?

Done.

https://codereview.chromium.org/17589007/diff/11/tests/standalone/io/raw_secu...
File tests/standalone/io/raw_secure_server_socket_argument_test.dart (right):

https://codereview.chromium.org/17589007/diff/11/tests/standalone/io/raw_secu...
tests/standalone/io/raw_secure_server_socket_argument_test.dart:26:
Expect.throws(() => RawSecureSocket.connect(SERVER_ADDRESS, 3456,
On 2013/06/25 05:55:03, Anders Johnsen wrote:
> maybe add a validator for the right error?

Done.

[Søren Gjesse, 2013-06-25 13:17:38]

lgtm

https://codereview.chromium.org/17589007/diff/19001/runtime/bin/secure_socket.cc
File runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/17589007/diff/19001/runtime/bin/secure_socket...
runtime/bin/secure_socket.cc:71: static void ThrowCertificateException(const
char* format,
When passing a format string you should consider adding full argv support.

[Bill Hesse, 2013-06-25 13:54:54]

Committed patchset #5 manually as r24405 (presubmit successful).