buffer: fix cancel()

* fix mismatch between return value & read value that would either crash
  or freeze the browser depending on its mood
* add an assertion to detect the above footgun
* fix some resource leaks
* fix iteration over a table that called a function which altered the
  table in buffer's cancel()
* if user cancels before anything is loaded, destroy the container too

4 files changed, 54 insertions, 23 deletions

diff --git a/src/loader/loader.nim b/src/loader/loader.nim
index 5aed021e..295062b5 100644
--- a/src/loader/loader.nim
+++ b/src/loader/loader.nim
@@ -987,6 +987,7 @@ proc onConnected*(loader: FileLoader, fd: int) =
     r.sread(msg) # packet 1
     loader.unregisterFun(fd)
     loader.unregistered.add(fd)
+    stream.sclose()
     let err = newTypeError("NetworkError when attempting to fetch resource")
     promise.resolve(JSResult[Response].err(err))
   loader.connecting.del(fd)
diff --git a/src/local/container.nim b/src/local/container.nim
index 49526a2a..43686b6f 100644
--- a/src/local/container.nim
+++ b/src/local/container.nim
@@ -44,7 +44,7 @@ type
 
   ContainerEventType* = enum
     cetAnchor, cetNoAnchor, cetUpdate, cetReadLine, cetReadArea, cetOpen,
-    cetSetLoadInfo, cetStatus, cetAlert, cetLoaded, cetTitle
+    cetSetLoadInfo, cetStatus, cetAlert, cetLoaded, cetTitle, cetCancel
 
   ContainerEvent* = object
     case t*: ContainerEventType
@@ -89,7 +89,7 @@ type
   BufferFilter* = ref object
     cmd*: string
 
-  LoadState = enum
+  LoadState* = enum
     lsLoading, lsCanceled, lsLoaded
 
   ContainerFlag* = enum
@@ -136,7 +136,7 @@ type
     hlon*: bool # highlight on?
     sourcepair*: Container # pointer to buffer with a source view (may be nil)
     needslines*: bool
-    loadState: LoadState
+    loadState*: LoadState
     events*: Deque[ContainerEvent]
     startpos: Option[CursorPosition]
     redirectDepth*: int
@@ -1359,13 +1359,10 @@ proc setLoadInfo(container: Container, msg: string) =
   container.triggerEvent(cetSetLoadInfo)
 
 #TODO this should be called with a timeout.
-proc onload*(container: Container, res: int) =
+proc onload(container: Container; res: int) =
   if container.loadState == lsCanceled:
-    container.setLoadInfo("")
-    container.iface.cancel().then(proc() =
-      container.needslines = true
-    )
-  elif res == -1:
+    return
+  if res == -1:
     container.loadState = lsLoaded
     container.setLoadInfo("")
     container.triggerEvent(cetStatus)
@@ -1441,12 +1438,22 @@ proc applyResponse*(container: Container; response: Response;
       container.charsetStack.add(DefaultCharset)
   container.charset = container.charsetStack[^1]
 
+proc remoteCancel*(container: Container) =
+  container.iface.cancel().then(proc() =
+    container.needslines = true
+  )
+  container.setLoadInfo("")
+  container.alert("Canceled loading")
+
 proc cancel*(container: Container) {.jsfunc.} =
   if container.select.open:
     container.select.cancel()
   elif container.loadState == lsLoading:
     container.loadState = lsCanceled
-    container.alert("Canceled loading")
+    if container.iface != nil:
+      container.remoteCancel()
+    else:
+      container.triggerEvent(cetCancel)
 
 proc findAnchor*(container: Container; anchor: string) =
   container.iface.findAnchor(anchor).then(proc(found: bool) =
diff --git a/src/local/pager.nim b/src/local/pager.nim
index c734b936..29e817f9 100644
--- a/src/local/pager.nim
+++ b/src/local/pager.nim
@@ -674,7 +674,8 @@ proc replace*(pager: Pager; target, container: Container) =
     pager.setContainer(container)
 
 proc deleteContainer(pager: Pager; container: Container) =
-  container.cancel()
+  if container.loadState == lsLoading:
+    container.cancel()
   if container.sourcepair != nil:
     container.sourcepair.sourcepair = nil
     container.sourcepair = nil
@@ -1759,6 +1760,20 @@ proc handleEvent0(pager: Pager; container: Container; event: ContainerEvent):
   of cetAlert:
     if pager.container == container:
       pager.alert(event.msg)
+  of cetCancel:
+    let i = pager.findConnectingContainer(container)
+    if i == -1:
+      # whoops. we tried to cancel, but the event loop did not favor us...
+      # at least cancel it in the buffer
+      container.remoteCancel()
+    else:
+      let item = pager.connectingContainers[i]
+      dec pager.numload
+      pager.deleteContainer(container)
+      pager.connectingContainers.del(i)
+      pager.selector.unregister(item.stream.fd)
+      pager.loader.unregistered.add(item.stream.fd)
+      item.stream.sclose()
   return true
 
 proc handleEvents*(pager: Pager, container: Container) =
diff --git a/src/server/buffer.nim b/src/server/buffer.nim
index ddd85420..0df80567 100644
--- a/src/server/buffer.nim
+++ b/src/server/buffer.nim
@@ -147,6 +147,7 @@ proc getFromOpaque[T](opaque: pointer, res: var T) =
   if opaque.len != 0:
     var r = opaque.stream.initReader(opaque.len)
     r.sread(res)
+    opaque.len = 0
 
 proc newBufferInterface*(stream: SocketStream, registerFun: proc(fd: int)):
     BufferInterface =
@@ -176,6 +177,11 @@ proc cloneInterface*(stream: SocketStream, registerFun: proc(fd: int)):
 proc resolve*(iface: BufferInterface, packetid, len: int) =
   iface.opaque.len = len
   iface.map.resolve(packetid)
+  # Protection against accidentally not exhausting data available to read,
+  # by setting opaque len to 0 in getFromOpaque.
+  # (If this assertion is failing, then it means you then()'ed a promise which
+  # should read something from the stream with an empty function.)
+  assert iface.opaque.len == 0
 
 proc hasPromises*(iface: BufferInterface): bool =
   return not iface.map.empty()
@@ -1168,7 +1174,7 @@ proc forceRender*(buffer: Buffer) {.proxy.} =
   buffer.prevStyled = nil
   buffer.do_reshape()
 
-proc cancel*(buffer: Buffer): int {.proxy.} =
+proc cancel*(buffer: Buffer) {.proxy.} =
   if buffer.state == bsLoaded:
     return
   for fd, data in buffer.loader.connecting:
@@ -1177,21 +1183,23 @@ proc cancel*(buffer: Buffer): int {.proxy.} =
     data.stream.sclose()
   buffer.loader.connecting.clear()
   for fd, data in buffer.loader.ongoing:
-    data.response.unregisterFun()
+    buffer.selector.unregister(fd)
+    buffer.loader.unregistered.add(fd)
+    data.response.body.sclose()
   buffer.loader.ongoing.clear()
-  buffer.selector.unregister(buffer.fd)
-  buffer.loader.unregistered.add(buffer.fd)
-  buffer.loader.removeCachedItem(buffer.cacheId)
-  buffer.fd = -1
-  buffer.cacheId = -1
-  buffer.outputId = -1
-  buffer.istream.sclose()
-  buffer.istream = nil
-  buffer.htmlParser.finish()
+  if buffer.istream != nil:
+    buffer.selector.unregister(buffer.fd)
+    buffer.loader.unregistered.add(buffer.fd)
+    buffer.loader.removeCachedItem(buffer.cacheId)
+    buffer.fd = -1
+    buffer.cacheId = -1
+    buffer.outputId = -1
+    buffer.istream.sclose()
+    buffer.istream = nil
+    buffer.htmlParser.finish()
   buffer.document.readyState = rsInteractive
   buffer.state = bsLoaded
   buffer.do_reshape()
-  return buffer.lines.len
 
 #https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart/form-data-encoding-algorithm
 proc serializeMultipartFormData(entries: seq[FormDataEntry]): FormData =