Fix some decoder defects

preserves.nimble

@@ -1,6 +1,6 @@
 # Package
 
-version = "20230517"
+version = "20230520"
 author        = "Emery Hemingway"
 description   = "data model and serialization format"
 license       = "Unlicense"

src/preserves.nim

@@ -29,8 +29,8 @@ type Symbol* = distinct string
 proc `$`*(s: Symbol): string {.borrow.}
 proc `<`*(x, y: Symbol): bool {.borrow.}
 proc `==`*(x, y: Symbol): bool {.borrow.}
-proc len*(s: Symbol): int {.borrow.}
 proc hash*(s: Symbol): Hash {.borrow.}
+proc len*(s: Symbol): int {.borrow.}
 
 type
   Preserve*[E = void] = object
@@ -474,13 +474,13 @@ proc writeVarint(s: Stream; n: int) =
       s.write((char)c or 0x80)
 
 proc readVarint(s: Stream): int =
-  var shift: int
+  var shift = 0
   while shift < (9*8):
-    let c = s.readChar.int
+    let c = s.readInt8
     result = result or ((c and 0x7f) shl shift)
     if (c and 0x80) == 0:
       break
-    shift.inc 7
+    inc(shift, 7)
 
 proc write*[E](str: Stream; pr: Preserve[E]) =
   ## Write the binary-encoding of a Preserves value to a stream.
@@ -603,16 +603,28 @@ proc decodePreserves*(s: Stream; E = void): Preserve[E] =
     result = decodePreserves(s, E)
     result.embedded = true
   of 0xb1:
-    result = Preserve[E](kind: pkString)
-    let len = s.readVarint()
-    result.string = s.readStr(len)
+    var data = newString(s.readVarint())
+    if data.len > 0:
+      let n = s.readData(unsafeAddr data[0], data.len)
+      if n != data.len:
+        raise newException(IOError, "short read")
+    result = Preserve[E](kind: pkString, string: data)
   of 0xb2:
-    result = Preserve[E](kind: pkByteString)
-    let len = s.readVarint()
-    result.bytes = cast[seq[byte]](s.readStr(len))
+    var data = newSeq[byte](s.readVarint())
+    if data.len > 0:
+      let n = s.readData(addr data[0], data.len)
+      if n != data.len:
+        raise newException(IOError, "short read")
+    else:
+      raiseAssert "readVarint returned zero"
+    result = Preserve[E](kind: pkByteString, bytes: data)
   of 0xb3:
-    let len = s.readVarint()
-    result = Preserve[E](kind: pkSymbol, symbol: Symbol s.readStr(len))
+    var data = newString(s.readVarint())
+    if data.len > 0:
+      let n = s.readData(addr data[0], data.len)
+      if n != data.len:
+        raise newException(IOError, "short read")
+    result = Preserve[E](kind: pkSymbol, symbol: Symbol data)
   of 0xb4:
     result = Preserve[E](kind: pkRecord)
     var label = decodePreserves(s, E)
@@ -670,7 +682,7 @@ proc decodePreserves*(s: seq[byte]; E = void): Preserve[E] =
 type BufferedDecoder* = object
   ## Type for buffering binary Preserves before decoding.
   stream: StringStream
-  decodePosition, maxSize: int
+  appendPosition, decodePosition, maxSize: int
 
 proc newBufferedDecoder*(maxSize = 4096): BufferedDecoder =
   ## Create a new `newBufferedDecoder`.
@@ -683,32 +695,41 @@ proc newBufferedDecoder*(maxSize = 4096): BufferedDecoder =
     var (success, pr) = decode(buf)
     assert success
     assert $pr == "<foobar>"
-  BufferedDecoder(stream: newStringStream(), maxSize: maxSize)
+  BufferedDecoder(
+      stream: newStringStream(newStringOfCap(maxSize)),
+      maxSize: maxSize,
+    )
 
 proc feed*(dec: var BufferedDecoder; buf: pointer; len: int) =
-  if dec.maxSize > 0 and dec.maxSize < (dec.stream.getPosition + len):
+  assert len > 0
+  if dec.maxSize > 0 and dec.maxSize < (dec.appendPosition + len):
     raise newException(IOError, "BufferedDecoder at maximum buffer size")
+  dec.stream.setPosition(dec.appendPosition)
   dec.stream.writeData(buf, len)
+  inc(dec.appendPosition, len)
+  assert dec.appendPosition == dec.stream.getPosition()
 
 proc feed*[T: byte|char](dec: var BufferedDecoder; data: openarray[T]) =
-  dec.feed(unsafeAddr data[0], data.len)
+  if data.len > 0:
+    dec.feed(unsafeAddr data[0], data.len)
 
 proc decode*(dec: var BufferedDecoder; E = void): (bool, Preserve[E]) =
   ## Decode from `dec`. If decoding fails the internal position of the
-  ## decode does not advance.
-  var appendPos = dec.stream.getPosition
-  dec.stream.setPosition(dec.decodePosition)
-  try:
-    result[1] = decodePreserves(dec.stream, E)
-    result[0] = true
-    if dec.stream.getPosition == appendPos:
-      dec.stream.setPosition(0)
-      dec.decodePosition = 0
-    else:
-      dec.decodePosition = dec.stream.getPosition
-      dec.stream.setPosition(appendPos)
-  except IOError, ValueError:
-    dec.stream.setPosition(appendPos)
+  ## decoder does not advance.
+  if dec.appendPosition > 0:
+    assert(dec.decodePosition < dec.appendPosition)
+    dec.stream.setPosition(dec.decodePosition)
+    try:
+      result[1] = decodePreserves(dec.stream, E)
+      result[0] = true
+      dec.decodePosition = dec.stream.getPosition()
+      if dec.decodePosition == dec.appendPosition:
+        dec.stream.setPosition(0)
+        dec.stream.data.setLen(0)
+        dec.appendPosition = 0
+        dec.decodePosition = 0
+    except IOError, ValueError:
+      discard
 
 template preservesRecord*(label: string) {.pragma.}
   ## Serialize this object or tuple as a record.

tests/test_buffereddecoder.nim

@@ -0,0 +1,23 @@
+# SPDX-FileCopyrightText: 2021 ☭ Emery Hemingway
+# SPDX-License-Identifier: Unlicense
+
+import std/[strutils, unittest]
+import preserves
+
+suite "BufferedDecoder":
+
+  test "half-string":
+    var
+      buf = newBufferedDecoder()
+      pr = Preserve[void](kind: pkByteString, bytes: newSeq[byte](23))
+      ok: bool
+    for i, _ in pr.bytes:
+      pr.bytes[i] = byte(i)
+    let bin = encode(pr)
+    for i in 0..32:
+      checkpoint $i
+      let j = (i+2) and 0xf
+      feed(buf, bin[0..<j])
+      feed(buf, bin[j..bin.high])
+      (ok, pr) = decode(buf)
+      assert ok