Fix secure socket tests to close sockets after writing. Fix handling of close events in TlsSocket …

sdk/lib/io/tls_socket.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * TlsSocket provides a secure (SSL or TLS) client connection to a server.
  * The certificate provided by the server is checked
  * using the certificate database provided in setCertificateDatabase.
  */
 abstract class TlsSocket implements Socket {
@@ skipping 103 matching lines @@
   OutputStream get outputStream {
     // TODO(6701): Implement stream interfaces on TlsSocket.
     throw new UnimplementedError("TlsSocket.inputStream not implemented yet");
   }
 
   int available() {
     throw new UnimplementedError("TlsSocket.available not implemented yet");
   }
 
   void close([bool halfClose]) {
-    _socket.close(halfClose);
+    if (halfClose) {
+      _closedWrite = true;
+      _writeEncryptedData();
+      if (_filterWriteEmpty) {
+        _socket.close(true);
+        _socketClosedWrite = true;
+      }
+    } else {
+      _closedWrite = true;
+      _closedRead = true;
+      _socket.close(false);
+      _socketClosedWrite = true;
+      _socketClosedRead = true;
+      _tlsFilter.destroy();
+      _tlsFilter = null;
+      if (scheduledDataEvent != null) {
+        scheduledDataEvent.cancel();
+      }
+      _status = CLOSED;
+    }
   }
 
   List<int> read([int len]) {
+    if (_closedRead) {
+      throw new SocketException("Reading from a closed socket");
+    }
     var buffer = _tlsFilter.buffers[READ_PLAINTEXT];
     _readEncryptedData();
     int toRead = buffer.length;
     if (len != null) {
       if (len is! int || len < 0) {
         throw new ArgumentError(
             "Invalid len parameter in TlsSocket.read (len: $len)");
       }
       if (len < toRead) {
         toRead = len;
       }
     }
     List<int> result = buffer.data.getRange(buffer.start, toRead);
     buffer.advanceStart(toRead);
     _setHandlersAfterRead();
     return result;
   }
 
   int readList(List<int> data, int offset, int bytes) {
+    if (_closedRead) {
+      throw new SocketException("Reading from a closed socket");
+    }
     if (offset < 0 || bytes < 0 || offset + bytes > data.length) {
       throw new ArgumentError(
           "Invalid offset or bytes in TlsSocket.readList");
     }
 
     int bytesRead = 0;
     var buffer = _tlsFilter.buffers[READ_PLAINTEXT];
     // TODO(whesse): Currently this fails if the if is turned into a while loop.
     // Fix it so that it can loop and read more than one buffer's worth of data.
     if (bytes > bytesRead) {
       _readEncryptedData();
       if (buffer.length > 0) {
         int toRead = min(bytes - bytesRead, buffer.length);
         data.setRange(offset, toRead, buffer.data, buffer.start);
         buffer.advanceStart(toRead);
         bytesRead += toRead;
         offset += toRead;
       }
     }
 
     _setHandlersAfterRead();
     return bytesRead;
   }
 
   // Write the data to the socket, and flush it as much as possible
   // until it would block.  If the write would block, _writeEncryptedData sets
   // up handlers to flush the pipeline when possible.
   int writeList(List<int> data, int offset, int bytes) {
+    if (_closedWrite) {
+      throw new SocketException("Writing to a closed socket");
+    }
     var buffer = _tlsFilter.buffers[WRITE_PLAINTEXT];
     if (bytes > buffer.free) {
       bytes = buffer.free;
     }
     if (bytes > 0) {
       buffer.data.setRange(buffer.start + buffer.length, bytes, data, offset);
       buffer.length += bytes;
     }
     _writeEncryptedData();  // Tries to flush all pipeline stages.
     return bytes;
   }
 
   void _tlsConnectHandler() {
     _connectPending = true;
     _tlsFilter.connect(_host, _port, _is_server, _certificateName);
     _status = HANDSHAKE;
     _tlsHandshake();
   }
 
   void _tlsWriteHandler() {
+    _writeEncryptedData();
+    if (_filterWriteEmpty && _closedWrite && !_socketClosedWrite) {
+      _socket.close(true);
+      _sockedClosedWrite = true;
+    }
     if (_status == HANDSHAKE) {
       _tlsHandshake();
     } else if (_status == CONNECTED) {
       if (_socketWriteHandler != null) {
         _socketWriteHandler();
+        _socketWriteHandler = null;

[Mads Ager (google), 2012/11/21 16:06:31]

I don't understand this change. Why would you want to get rid of the socket
write handler here?

[Bill Hesse, 2012/11/21 16:25:25]

The onWrite handler is a one-shot handler, that is reset to null after being
called.  This is in the documentation of the Socket class.  It was a mistake
that we were potentially calling it multiple times here. 

There was an error here, in that it should be possible to reset the write
handler from the write handler.  Fixed.
There is another potential bug:
 I have now added a check here that it is possible to write to the plaintext
write buffer here.

[Mads Ager (google), 2012/11/22 10:04:16]

Thanks, that's right. I keep forgetting that the writeHandler is one-shot. With
the new event-handling system I think this should just be a stream that you need
to subscribe to and unsubscribe from like any other stream. But this is right
for this change. :)

       }
     }
   }
 
   void _tlsDataHandler() {
     if (_status == HANDSHAKE) {
       _tlsHandshake();
     } else {
       _writeEncryptedData();  // TODO(whesse): Removing this causes a failure.
       _readEncryptedData();
-      var buffer = _tlsFilter.buffers[READ_PLAINTEXT];
-      if (_filterEmpty) {
-        if (_fireCloseEventPending) {
-          _fireCloseEvent();
-        }
-      } else {  // Filter is not empty.
+      if (!_filterReadEmpty) {
+        // Call the onData event.
         if (scheduledDataEvent != null) {
           scheduledDataEvent.cancel();
           scheduledDataEvent = null;
         }
         if (_socketDataHandler != null) {
           _socketDataHandler();
         }
       }
     }
   }
 
   void _tlsCloseHandler() {
-    _socketClosed = true;
-    _status = CLOSED;
-    _socket.close();
-    if (_filterEmpty) {
+    _socketClosedRead = true;
+    if (_filterReadEmpty) {
+      _closedRead = true;
       _fireCloseEvent();
-    } else {
-      _fireCloseEventPending = true;
+      if (_socketClosedWrite) {
+        _tlsFilter.destroy();
+        _tlsFilter = null;
+        _status = CLOSED;
+      }
     }
   }
 
   void _tlsHandshake() {
-      _readEncryptedData();
-      _tlsFilter.handshake();
-      _writeEncryptedData();
-      if (_tlsFilter.buffers[WRITE_ENCRYPTED].length > 0) {
-        _socket.onWrite = _tlsWriteHandler;
-      }
+    _readEncryptedData();
+    _tlsFilter.handshake();
+    _writeEncryptedData();
+    if (_tlsFilter.buffers[WRITE_ENCRYPTED].length > 0) {
+      _socket.onWrite = _tlsWriteHandler;
+    }
   }
 
   void _tlsHandshakeCompleteHandler() {
     _status = CONNECTED;
     if (_connectPending && _socketConnectHandler != null) {
       _connectPending = false;
       _socketConnectHandler();
     }
   }
 
   void _fireCloseEvent() {
-    _fireCloseEventPending = false;
-    _tlsFilter.destroy();
-    _tlsFilter = null;
     if (scheduledDataEvent != null) {
       scheduledDataEvent.cancel();
     }
     if (_socketCloseHandler != null) {
       _socketCloseHandler();
     }
   }
 
   void _readEncryptedData() {
     // Read from the socket, and push it through the filter as far as
     // possible.
     var encrypted = _tlsFilter.buffers[READ_ENCRYPTED];
     var plaintext = _tlsFilter.buffers[READ_PLAINTEXT];
     bool progress = true;
     while (progress) {
       progress = false;
       // Do not try to read plaintext from the filter while handshaking.
-      if ((_status == CONNECTED || _status == CLOSED) && plaintext.free > 0) {
+      if ((_status == CONNECTED) && plaintext.free > 0) {
         int bytes = _tlsFilter.processBuffer(READ_PLAINTEXT);
         if (bytes > 0) {
           plaintext.length += bytes;
           progress = true;
         }
       }
       if (encrypted.length > 0) {
         int bytes = _tlsFilter.processBuffer(READ_ENCRYPTED);
         if (bytes > 0) {
           encrypted.advanceStart(bytes);
           progress = true;
         }
       }
-      if (!_socketClosed) {
+      if (!_socketClosedRead) {
         int bytes = _socket.readList(encrypted.data,
                                      encrypted.start + encrypted.length,
                                      encrypted.free);
         if (bytes > 0) {
           encrypted.length += bytes;
           progress = true;
         }
       }
     }
-    // TODO(whesse): This can be incorrect if there is a partial
-    // encrypted block stuck in the tlsFilter, and no other data.
-    // Fix this - we do need to know when the filter is empty.
-    _filterEmpty = (plaintext.length == 0);
+    // If there is any data in any stages of the filter, there should
+    // be data in the plaintext buffer after this process.
+    // TODO(whesse): Verify that this is true, and there can be no
+    // partial encrypted block stuck in the tlsFilter.
+    _filterReadEmpty = (plaintext.length == 0);
   }
 
   void _writeEncryptedData() {
-    // Write from the filter to the socket.
-    var buffer = _tlsFilter.buffers[WRITE_ENCRYPTED];
+    if (_socketClosedWrite) return;
+    var encrypted = _tlsFilter.buffers[WRITE_ENCRYPTED];
+    var plaintext = _tlsFilter.buffers[WRITE_PLAINTEXT];
     while (true) {
-      if (buffer.length > 0) {
-        int bytes = _socket.writeList(buffer.data, buffer.start, buffer.length);
+      if (encrypted.length > 0) {
+        // Write from the filter to the socket.
+        int bytes = _socket.writeList(encrypted.data,
+                                      encrypted.start,
+                                      encrypted.length);
         if (bytes == 0) {
           // The socket has blocked while we have data to write.
           // We must be notified when it becomes unblocked.
           _socket.onWrite = _tlsWriteHandler;
+          _filterWriteEmpty = false;
           break;
         }
-        buffer.advanceStart(bytes);
+        encrypted.advanceStart(bytes);
       } else {
         var plaintext = _tlsFilter.buffers[WRITE_PLAINTEXT];
         if (plaintext.length > 0) {
            int plaintext_bytes = _tlsFilter.processBuffer(WRITE_PLAINTEXT);
            plaintext.advanceStart(plaintext_bytes);
         }
         int bytes = _tlsFilter.processBuffer(WRITE_ENCRYPTED);
-        if (bytes <= 0) break;
-        buffer.length += bytes;
+        if (bytes <= 0) {
+          // We know the WRITE_ENCRYPTED buffer is empty, and the
+          // filter wrote zero bytes to it, so the filter must be empty.
+          // Also, the WRITE_PLAINTEXT buffer must have been empty, or
+          // it would have written to the filter.
+          // TODO(whesse): Verify that the filter works this way.
+          _filterWriteEmpty = true;
+          break;
+        }
+        encrypted.length += bytes;
       }
     }
   }
 
   /* After a read, the onData handler is enabled to fire again.
    * We may also have a close event waiting for the TlsFilter to empty.
    */
   void _setHandlersAfterRead() {
     // If the filter is empty, then we are guaranteed an event when it
-    // becomes unblocked.
+    // becomes unblocked.  Cancel any _tlsDataHandler call.
     // Otherwise, schedule a _tlsDataHandler call since there may data
     // available, and this read call enables the data event.
-    if (!_filterEmpty && scheduledDataEvent == null) {
-      scheduledDataEvent = new Timer(0, (_) => _tlsDataHandler());
-    } else if (_filterEmpty && scheduledDataEvent != null) {
+    if (_filterReadEmpty) {
+      if (scheduledDataEvent != null) {
         scheduledDataEvent.cancel();
         scheduledDataEvent = null;
+      }
+    } else if (scheduledDataEvent == null) {
+      scheduledDataEvent = new Timer(0, (_) => _tlsDataHandler());
     }
-    if (_filterEmpty && _fireCloseEventPending) {
-        _fireCloseEvent();
+
+    if (_socketClosedRead) {  // An onClose event is pending.
+      // _closedRead is false, since we are in a read or readList call.
+      if (!_filterReadEmpty) {
+        // _filterReadEmpty may be out of date since read and readList empty
+        // the plaintext buffer after calling _readEncryptedData.
+        // TODO(whesse): Fix this as part of fixing read and readList.

[Mads Ager (google), 2012/11/21 16:06:31]

What does _readEncryptedData() do in that case? Is it harmless?

[Bill Hesse, 2012/11/21 16:25:25]

Yes, _readEncryptedData is harmless - it advances data through the buffers of
the filter pipeline.  It checks that the socket has not been closed for reading
before attempting to read data from it.

+        _readEncryptedData();
+      }
+      if (_filterReadEmpty) {
+        // This can't be an else clause: the value of _filterReadEmpty changes.
+        // This must be asynchronous, because we are in a read or readList call.
+        new Timer(0, (_) => _fireCloseEvent());
+      }
     }
   }
 
   // _TlsSocket cannot extend _Socket and use _Socket's factory constructor.
   Socket _socket;
   String _host;
   int _port;
   bool _is_server;
   String _certificateName;
 
   var _status = NOT_CONNECTED;
-  bool _socketClosed = false;
-  bool _filterEmpty = false;
+  bool _socketClosedRead = false;  // The network socket is closed for reading.
+  bool _socketClosedWrite = false;  // The network socket is closed for writing.
+  bool _closedRead = false;  // The secure socket has fired an onClosed event.
+  bool _closedWrite = false;  // The secure socket has been closed for writing.
+  bool _filterReadEmpty = null;  // There is no buffered data to read.

[Mads Ager (google), 2012/11/21 16:06:31]

Please initialize to booleans. It would make sense that they are true on
initialization as there is nothing in the filter at that point.

+  bool _filterWriteEmpty = null;  // There is no buffered data to be written.
   bool _connectPending = false;
-  bool _fireCloseEventPending = false;
   Function _socketConnectHandler;
   Function _socketWriteHandler;
   Function _socketDataHandler;
   Function _socketCloseHandler;
   Timer scheduledDataEvent;
 
   _TlsFilter _tlsFilter;
 }
 
 
@@ skipping 26 matching lines @@
                bool is_server,
                String certificateName);
   void destroy();
   void handshake();
   void init();
   int processBuffer(int bufferIndex);
   void registerHandshakeCompleteCallback(Function handshakeCompleteHandler);
 
   List<_TlsExternalBuffer> get buffers;
 }