Fix autobahn tls tests (#57)

* split out message and control frames sending

* fix premature closure under TLS

* dissable hints noise

ws.nimble

@@ -15,11 +15,11 @@ requires "nimcrypto"
 requires "bearssl"
 
 task test, "run tests":
-  exec "nim c -r --opt:speed -d:debug --verbosity:0 --hints:off -d:chronicles_log_level=info ./tests/testcommon.nim"
+  exec "nim --hints:off c -r --opt:speed -d:debug --verbosity:0 --hints:off -d:chronicles_log_level=info ./tests/testcommon.nim"
   rmFile "./tests/testcommon"
 
-  exec "nim c -r --opt:speed -d:debug --verbosity:0 --hints:off -d:chronicles_log_level=info ./tests/testwebsockets.nim"
+  exec "nim --hints:off c -r --opt:speed -d:debug --verbosity:0 --hints:off -d:chronicles_log_level=info ./tests/testwebsockets.nim"
   rmFile "./tests/testwebsockets"
 
-  exec "nim -d:secure c -r --opt:speed -d:debug --verbosity:0 --hints:off -d:chronicles_log_level=info ./tests/testwebsockets.nim"
+  exec "nim --hints:off -d:secure c -r --opt:speed -d:debug --verbosity:0 --hints:off -d:chronicles_log_level=info ./tests/testwebsockets.nim"
   rmFile "./tests/testwebsockets"

ws/http/common.nim

@@ -51,9 +51,9 @@ proc closeStream*(stream: AsyncStreamRW) {.async.} =
 
 proc closeWait*(stream: AsyncStream) {.async.} =
   await allFutures(
-    stream.reader.tsource.closeTransp(),
     stream.reader.closeStream(),
-    stream.writer.closeStream())
+    stream.writer.closeStream(),
+    stream.reader.tsource.closeTransp())
 
 proc sendResponse*(
   request: HttpRequest,

ws/session.nim

@@ -23,47 +23,16 @@ proc prepareCloseBody(code: StatusCodes, reason: string): seq[byte] =
   if ord(code) > 999:
     result = @(ord(code).uint16.toBytesBE()) & result
 
-proc writeMessage*(
+proc writeMessage*(ws: WSSession,
-  ws: WSSession,
   data: seq[byte] = @[],
   opcode: Opcode,
+  maskKey: MaskKey,
   extensions: seq[Ext]) {.async.} =
-  ## Send a frame applying the supplied
-  ## extensions
-  ##
-
-  if ws.readyState == ReadyState.Closed:
-    raise newException(WSClosedError, "Socket is closed!")
-
-  logScope:
-    opcode = opcode
-    dataSize = data.len
-    masked = ws.masked
-
-  trace "Sending data to remote"
-
-  var maskKey: array[4, char]
-  if ws.masked:
-    maskKey = genMaskKey(ws.rng)
 
   if opcode notin {Opcode.Text, Opcode.Cont, Opcode.Binary}:
-
+    warn "Attempting to send a data frame with an invalid opcode!"
-    if ws.readyState in {ReadyState.Closing} and opcode notin {Opcode.Close}:
+    raise newException(WSInvalidOpcodeError,
-      return
+      &"Attempting to send a data frame with an invalid opcode {opcode}!")
-
-    await ws.stream.writer.write(
-      (await Frame(
-        fin: true,
-        rsv1: false,
-        rsv2: false,
-        rsv3: false,
-        opcode: opcode,
-        mask: ws.masked,
-        data: data, # allow sending data with close messages
-        maskKey: maskKey)
-        .encode()))
-
-    return
 
   let maxSize = ws.frameSize
   var i = 0
@@ -86,16 +55,76 @@ proc writeMessage*(
     if i >= data.len:
       break
 
+proc writeControl*(
+  ws: WSSession,
+  data: seq[byte] = @[],
+  opcode: Opcode,
+  maskKey: MaskKey) {.async.} =
+  ## Send a frame applying the supplied
+  ## extensions
+  ##
+
+  logScope:
+    opcode = opcode
+    dataSize = data.len
+    masked = ws.masked
+
+  if opcode in {Opcode.Text, Opcode.Cont, Opcode.Binary}:
+    warn "Attempting to send a control frame with an invalid opcode!"
+    raise newException(WSInvalidOpcodeError,
+      &"Attempting to send a control frame with an invalid opcode {opcode}!")
+
+  let frame = Frame(
+      fin: true,
+      rsv1: false,
+      rsv2: false,
+      rsv3: false,
+      opcode: opcode,
+      mask: ws.masked,
+      data: data,
+      maskKey: maskKey)
+
+  let encoded = await frame.encode()
+  await ws.stream.writer.write(encoded)
+
+  trace "Wrote control frame"
+
 proc send*(
   ws: WSSession,
   data: seq[byte] = @[],
-  opcode: Opcode): Future[void] =
+  opcode: Opcode): Future[void]
+  {.raises: [Defect, WSClosedError].} =
   ## Send a frame
   ##
 
-  return ws.writeMessage(data, opcode, ws.extensions)
+  if ws.readyState == ReadyState.Closed:
+    raise newException(WSClosedError, "WebSocket is closed!")
 
-proc send*(ws: WSSession, data: string): Future[void] =
+  if ws.readyState in {ReadyState.Closing} and opcode notin {Opcode.Close}:
+    trace "Can only respond with Close opcode to a closing connection"
+    return
+
+  logScope:
+    opcode = opcode
+    dataSize = data.len
+    masked = ws.masked
+
+  trace "Sending data to remote"
+
+  let maskKey = if ws.masked:
+      genMaskKey(ws.rng)
+    else:
+      default(MaskKey)
+
+  if opcode in {Opcode.Text, Opcode.Cont, Opcode.Binary}:
+    return ws.writeMessage(data, opcode, maskKey, ws.extensions)
+
+  return ws.writeControl(data, opcode, maskKey)
+
+proc send*(
+  ws: WSSession,
+  data: string): Future[void]
+  {.raises: [Defect, WSClosedError].} =
   send(ws, data.toBytes(), Opcode.Text)
 
 proc handleClose*(
@@ -169,6 +198,13 @@ proc handleClose*(
     await ws.send(prepareCloseBody(code, reason), Opcode.Close)
 
     ws.readyState = ReadyState.Closed
+
+  # TODO: Under TLS, the response takes longer
+  # to depart and fails to write the resp code
+  # and cleanly close the connection. Definitely
+  # looks like a bug, but not sure if it's chronos
+  # or us?
+  await sleepAsync(10.millis)
   await ws.stream.closeWait()
 
 proc handleControl*(ws: WSSession, frame: Frame) {.async.} =
@@ -248,7 +284,10 @@ proc readFrame*(ws: WSSession, extensions: seq[Ext] = @[]): Future[Frame] {.asyn
 
     return frame
 
-proc ping*(ws: WSSession, data: seq[byte] = @[]): Future[void] =
+proc ping*(
+  ws: WSSession,
+  data: seq[byte] = @[]): Future[void]
+  {.raises: [Defect, WSClosedError].} =
   ws.send(data, opcode = Opcode.Ping)
 
 proc recv*(
@@ -346,9 +385,10 @@ proc recv*(
 
     return consumed
   except CatchableError as exc:
+    trace "Exception reading frames", exc = exc.msg
+
     ws.readyState = ReadyState.Closed
     await ws.stream.closeWait()
-    trace "Exception reading frames", exc = exc.msg
     raise exc
   finally:
     if not isNil(ws.frame) and (ws.frame.fin and ws.frame.remainder <= 0):