Add two new unique delimited error code for better caller handling

Sources/SwiftProtobuf/AsyncMessageSequence.swift

@@ -28,12 +28,9 @@ extension AsyncSequence where Element == UInt8 {
   ///     `BinaryDecodingError.missingRequiredFields` will be thrown.
   ///   - options: The BinaryDecodingOptions to use.
   /// - Returns: An asynchronous sequence of messages read from the `AsyncSequence` of bytes.
-  /// - Throws: `BinaryDecodingError` if decoding fails, throws
-  ///           `BinaryDelimited.Error` for some reading errors,
-  ///           `BinaryDecodingError.truncated` if the stream ends before fully decoding a
-  ///           message or a delimiter,
-  ///           `BinaryDecodingError.malformedProtobuf`if a delimiter could not be read and
-  ///           `BinaryDecodingError.tooLarge` if a size delimiter of 2GB or greater is found.
+  /// - Throws: `BinaryDelimited.Error` for errors in the framing of the messages
+  ///           in the sequence, `BinaryDecodingError` for errors while decoding
+  ///           messages.
   @inlinable
   public func binaryProtobufDelimitedMessages<M: Message>(
     of messageType: M.Type = M.self,
@@ -78,12 +75,9 @@ public struct AsyncMessageSequence<
   ///     `BinaryDecodingError.missingRequiredFields` will be thrown.
   ///   - options: The BinaryDecodingOptions to use.
   /// - Returns: An asynchronous sequence of messages read from the `AsyncSequence` of bytes.
-  /// - Throws: `BinaryDecodingError` if decoding fails, throws
-  ///           `BinaryDelimited.Error` for some reading errors,
-  ///           `BinaryDecodingError.truncated` if the stream ends before fully decoding a
-  ///           message or a delimiter,
-  ///           `BinaryDecodingError.malformedProtobuf`if a delimiter could not be read and
-  ///           `BinaryDecodingError.tooLarge` if a size delimiter of 2GB or greater is found.
+  /// - Throws: `BinaryDelimited.Error` for errors in the framing of the messages
+  ///           in the sequence, `BinaryDecodingError` for errors while decoding
+  ///           messages.
   public init(
     base: Base,
     extensions: ExtensionMap? = nil,
@@ -130,7 +124,7 @@ public struct AsyncMessageSequence<
         shift += UInt64(7)
         if shift > 35 {
           iterator = nil
-          throw BinaryDecodingError.malformedProtobuf
+          throw BinaryDelimited.Error.malformedLength
         }
         if (byte & 0x80 == 0) {
           return messageSize
@@ -139,7 +133,7 @@ public struct AsyncMessageSequence<
       if (shift > 0) {
         // The stream has ended inside a varint.
         iterator = nil
-        throw BinaryDecodingError.truncated
+        throw BinaryDelimited.Error.truncated
       }
       return nil // End of stream reached.
     }
@@ -161,7 +155,7 @@ public struct AsyncMessageSequence<
           guard let byte = try await iterator?.next() else {
             // The iterator hit the end, but the chunk wasn't filled, so the full
             // payload wasn't read.
-            throw BinaryDecodingError.truncated
+            throw BinaryDelimited.Error.truncated
           }
           chunk[consumedBytes] = byte
           consumedBytes += 1

Sources/SwiftProtobuf/BinaryDelimited.swift

@@ -32,6 +32,19 @@ public enum BinaryDelimited {
     /// Messages are limited by the protobuf spec to 2GB; when decoding, if the
     /// length says the payload is over 2GB, this error is raised.
     case tooLarge
+
+    /// While attempting to read the length of a message on the stream, the
+    /// bytes were malformed for the protobuf format.
+    case malformedLength
+
+    /// This isn't really an "error". `InputStream` documents that
+    /// `hasBytesAvailable` _may_ return `True` if a read is needed to
+    /// determine if there really are bytes available. So this "error" is throw
+    /// when a `parse` or `merge` fails because there were no bytes available.
+    /// If this is rasied, the callers should decides via what ever other means
+    /// are correct if the stream has completely ended or if more bytes might
+    /// eventually show up.
+    case noBytesAvailable
   }
 
   /// Serialize a single size-delimited message to the given stream. Delimited
@@ -110,7 +123,7 @@ public enum BinaryDelimited {
   /// - Returns: The message read.
   /// - Throws: `BinaryDecodingError` if decoding fails, throws
   ///           `BinaryDelimited.Error` for some reading errors, and the
-  ///           underlying InputStream.streamError for a stream error.
+  ///           underlying `InputStream.streamError` for a stream error.
   public static func parse<M: Message>(
     messageType: M.Type,
     from stream: InputStream,
@@ -151,7 +164,7 @@ public enum BinaryDelimited {
   ///   - options: The BinaryDecodingOptions to use.
   /// - Throws: `BinaryDecodingError` if decoding fails, throws
   ///           `BinaryDelimited.Error` for some reading errors, and the
-  ///           underlying InputStream.streamError for a stream error.
+  ///           underlying `InputStream.streamError` for a stream error.
   public static func merge<M: Message>(
     into message: inout M,
     from stream: InputStream,
@@ -222,31 +235,38 @@ internal func decodeVarint(_ stream: InputStream) throws -> UInt64 {
   let readBuffer = UnsafeMutablePointer<UInt8>.allocate(capacity: 1)
   defer { readBuffer.deallocate() }
 
-  func nextByte() throws -> UInt8 {
+  func nextByte() throws -> UInt8? {
     let bytesRead = stream.read(readBuffer, maxLength: 1)
-    if bytesRead != 1 {
-      if bytesRead == -1 {
-        if let streamError = stream.streamError {
-          throw streamError
-        }
-        throw BinaryDelimited.Error.unknownStreamError
+    switch bytesRead {
+    case 1:
+      return readBuffer[0]
+    case 0:
+      return nil
+    default:
+      precondition(bytesRead == -1)
+      if let streamError = stream.streamError {
+        throw streamError
       }
-      throw BinaryDelimited.Error.truncated
+      throw BinaryDelimited.Error.unknownStreamError
     }
-    return readBuffer[0]
   }
 
   var value: UInt64 = 0
   var shift: UInt64 = 0
   while true {
-    let c = try nextByte()
+    guard let c = try nextByte() else {
+      if shift == 0 {
+        throw BinaryDelimited.Error.noBytesAvailable
+      }
+      throw BinaryDelimited.Error.truncated
+    }
     value |= UInt64(c & 0x7f) << shift
     if c & 0x80 == 0 {
       return value
     }
     shift += 7
     if shift > 63 {
-      throw BinaryDecodingError.malformedProtobuf
+      throw BinaryDelimited.Error.malformedLength
     }
   }
 }

Tests/SwiftProtobufTests/Test_AsyncMessageSequence.swift

@@ -126,11 +126,11 @@ final class Test_AsyncMessageSequence: XCTestCase {
         XCTFail("Shouldn't have returned a value for an empty stream.")
       }
     } catch {
-      if error as! BinaryDecodingError == .truncated {
+      if error as! BinaryDelimited.Error == .truncated {
         truncatedThrown = true
       }
     }
-    XCTAssertTrue(truncatedThrown, "Should throw a BinaryDecodingError.truncated")
+    XCTAssertTrue(truncatedThrown, "Should throw a BinaryDelimited.Error.truncated")
   }
 
   // Single varint describing a 2GB message
@@ -161,11 +161,11 @@ final class Test_AsyncMessageSequence: XCTestCase {
         XCTFail("Shouldn't have returned a value for an empty stream.")
       }
     } catch {
-      if error as! BinaryDecodingError == .truncated {
+      if error as! BinaryDelimited.Error == .truncated {
         truncatedThrown = true
       }
     }
-    XCTAssertTrue(truncatedThrown, "Should throw a BinaryDecodingError.truncated")
+    XCTAssertTrue(truncatedThrown, "Should throw a BinaryDelimited.Error.truncated")
   }
 
   // Stream with a valid varint and message, but the following varint is truncated
@@ -193,11 +193,11 @@ final class Test_AsyncMessageSequence: XCTestCase {
       }
       XCTAssertEqual(count, 1, "One message should be deserialized")
     } catch {
-      if error as! BinaryDecodingError == .truncated {
+      if error as! BinaryDelimited.Error == .truncated {
         truncatedThrown = true
       }
     }
-    XCTAssertTrue(truncatedThrown, "Should throw a BinaryDecodingError.truncated")
+    XCTAssertTrue(truncatedThrown, "Should throw a BinaryDelimited.Error.truncated")
   }
 
   fileprivate func asyncByteStream(bytes: [UInt8]) -> AsyncStream<UInt8> {

Tests/SwiftProtobufTests/Test_BinaryDelimited.swift

@@ -12,12 +12,98 @@ import Foundation
 import XCTest
 import SwiftProtobuf
 
+fileprivate func openInputStream(_ bytes: [UInt8]) -> InputStream {
+  let istream = InputStream(data: Data(bytes))
+  istream.open()
+  return istream
+}
+
 class Test_BinaryDelimited: XCTestCase {
 
-  func testEverything() {
-    // Don't need to test encode/decode since there are plenty of tests specific to that,
-    // just test the delimited behaviors.
+  /// Helper to assert the next message read matches and expected one.
+  func assertParse<M: Message & Equatable>(expected: M, onStream istream: InputStream) {
+    do {
+      let msg = try BinaryDelimited.parse(
+        messageType: M.self,
+        from: istream)
+      XCTAssertEqual(msg, expected)
+    } catch let e {
+      XCTFail("Unexpected failure: \(e)")
+    }
+  }
+
+  /// Helper to assert we're at the end of the stream.
+  ///
+  /// `hasBytesAvailable` is documented as maybe returning True and a read
+  /// has to happen to really know if ones at the end. This is especially
+  /// true with file based streams.
+  func assertParseFails(atEndOfStream istream: InputStream) {
+    XCTAssertThrowsError(try BinaryDelimited.parse(messageType: SwiftProtoTesting_TestAllTypes.self,
+                                                   from: istream)) { error in
+      XCTAssertEqual(error as? BinaryDelimited.Error, BinaryDelimited.Error.noBytesAvailable)
+    }
+  }
+
+  func assertParsing(failsWithTruncatedStream istream: InputStream) {
+    XCTAssertThrowsError(try BinaryDelimited.parse(messageType: SwiftProtoTesting_TestAllTypes.self,
+                                                   from: istream)) { error in
+      XCTAssertEqual(error as? BinaryDelimited.Error, BinaryDelimited.Error.truncated)
+    }
+  }
+
+  func testNoData() {
+    let istream = openInputStream([])
+
+    assertParseFails(atEndOfStream: istream)
+  }
+
+  func testZeroLengthMessage() {
+    let istream = openInputStream([0])
+
+    assertParse(expected: SwiftProtoTesting_TestAllTypes(), onStream: istream)
+
+    assertParseFails(atEndOfStream: istream)
+  }
+
+  func testNoDataForMessage() {
+    let istream = openInputStream([0x96, 0x01])
+
+    // Length will be read, then the no data for the message, so .truncated.
+    assertParsing(failsWithTruncatedStream: istream)
+  }
 
+  func testNotEnoughDataForMessage() {
+    let istream = openInputStream([0x96, 0x01, 0x01, 0x02, 0x03])
+
+    // Length will be read, but not enought data, so .truncated
+    assertParsing(failsWithTruncatedStream: istream)
+  }
+
+  func testTruncatedLength() {
+    let istream = openInputStream([0x96]) // Needs something like `, 0x01`
+
+    assertParsing(failsWithTruncatedStream: istream)
+  }
+
+  func testTooLarge() {
+    let istream = openInputStream([0x80, 0x80, 0x80, 0x80, 0x08]) // 2GB
+
+    XCTAssertThrowsError(try BinaryDelimited.parse(messageType: SwiftProtoTesting_TestAllTypes.self,
+                                                   from: istream)) { error in
+      XCTAssertEqual(error as? BinaryDelimited.Error, BinaryDelimited.Error.tooLarge)
+    }
+  }
+
+  func testOverEncodedLength() {
+    let istream = openInputStream([0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80 ,0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x08])
+
+    XCTAssertThrowsError(try BinaryDelimited.parse(messageType: SwiftProtoTesting_TestAllTypes.self,
+                                                   from: istream)) { error in
+      XCTAssertEqual(error as? BinaryDelimited.Error, BinaryDelimited.Error.malformedLength)
+    }
+  }
+
+  func testTwoMessages() {
     let stream1 = OutputStream.toMemory()
     stream1.open()
 
@@ -48,27 +134,15 @@ class Test_BinaryDelimited: XCTestCase {
     let stream2 = InputStream(data: data)
     stream2.open()
 
+    // Test using `merge`
     var msg1a = SwiftProtoTesting_TestAllTypes()
     XCTAssertNoThrow(try BinaryDelimited.merge(into: &msg1a, from: stream2))
     XCTAssertEqual(msg1, msg1a)
 
-    do {
-      let msg2a = try BinaryDelimited.parse(
-        messageType: SwiftProtoTesting_TestPackedTypes.self,
-        from: stream2)
-      XCTAssertEqual(msg2, msg2a)
-    } catch let e {
-      XCTFail("Unexpected failure: \(e)")
-    }
+    // Test using `parse`
+    assertParse(expected: msg2, onStream: stream2)
 
-    do {
-      _ = try BinaryDelimited.parse(messageType: SwiftProtoTesting_TestAllTypes.self, from: stream2)
-      XCTFail("Should not have gotten here")
-    } catch BinaryDelimited.Error.truncated {
-      // Nothing, this is what we expect since there is nothing left to read.
-    } catch let e {
-      XCTFail("Unexpected failure: \(e)")
-    }
+    assertParseFails(atEndOfStream: stream2)
   }
 
 }