Skip to content

Commit

Permalink
Add a new "error" for no bytes available.
Browse files Browse the repository at this point in the history
`InputStream` documents that `hasBytesAvailable` might return `True` when a
`read` is needed to determine the real status. This case caused some attempts to
use the delimited api then fall into seeing "end" as an error because it caused
folks to see `.truncated` without really knowing nothing was there to read. This
provide a distinct case for this condition so callers then have the option to
gracefully handle reaching the end.

Closed #1410
  • Loading branch information
thomasvl committed Aug 22, 2023
1 parent 778ec5c commit 618433a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
36 changes: 26 additions & 10 deletions Sources/SwiftProtobuf/BinaryDelimited.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ public enum BinaryDelimited {
/// 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
Expand Down Expand Up @@ -226,24 +235,31 @@ 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
Expand Down
2 changes: 1 addition & 1 deletion Tests/SwiftProtobufTests/Test_BinaryDelimited.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class Test_BinaryDelimited: XCTestCase {
func assertParseFails(atEndOfStream istream: InputStream) {
XCTAssertThrowsError(try BinaryDelimited.parse(messageType: SwiftProtoTesting_TestAllTypes.self,
from: istream)) { error in
XCTAssertEqual(error as? BinaryDelimited.Error, BinaryDelimited.Error.truncated)
XCTAssertEqual(error as? BinaryDelimited.Error, BinaryDelimited.Error.noBytesAvailable)
}
}

Expand Down

0 comments on commit 618433a

Please sign in to comment.