Skip to content

Commit

Permalink
Use distinct errors for errors during delimited parsing.
Browse files Browse the repository at this point in the history
This allows the caller to tell the difference between when a message *within*
the delimited stream was bad vs. when the framing on stream itself was bad.
  • Loading branch information
thomasvl committed Aug 22, 2023
1 parent e5244e8 commit 778ec5c
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 33 deletions.
14 changes: 7 additions & 7 deletions Sources/SwiftProtobuf/AsyncMessageSequence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ extension AsyncSequence where Element == UInt8 {
/// - 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
/// `BinaryDelimited.Error.truncated` if the stream ends before fully decoding a
/// message or a delimiter,
/// `BinaryDecodingError.malformedProtobuf`if a delimiter could not be read and
/// `BinaryDelimited.Error.malformedLength`if a delimiter could not be read and
/// `BinaryDecodingError.tooLarge` if a size delimiter of 2GB or greater is found.
@inlinable
public func binaryProtobufDelimitedMessages<M: Message>(
Expand Down Expand Up @@ -80,9 +80,9 @@ public struct AsyncMessageSequence<
/// - 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
/// `BinaryDelimited.Error.truncated` if the stream ends before fully decoding a
/// message or a delimiter,
/// `BinaryDecodingError.malformedProtobuf`if a delimiter could not be read and
/// `BinaryDelimited.Error.malformedLength`if a delimiter could not be read and
/// `BinaryDecodingError.tooLarge` if a size delimiter of 2GB or greater is found.
public init(
base: Base,
Expand Down Expand Up @@ -130,7 +130,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
Expand All @@ -139,7 +139,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.
}
Expand Down Expand Up @@ -182,7 +182,7 @@ public struct AsyncMessageSequence<
)
}
}
throw BinaryDecodingError.truncated // The buffer was not filled.
throw BinaryDelimited.Error.truncated // The buffer was not filled.
}
}

Expand Down
6 changes: 5 additions & 1 deletion Sources/SwiftProtobuf/BinaryDelimited.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ 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
}

/// Serialize a single size-delimited message to the given stream. Delimited
Expand Down Expand Up @@ -246,7 +250,7 @@ internal func decodeVarint(_ stream: InputStream) throws -> UInt64 {
}
shift += 7
if shift > 63 {
throw BinaryDecodingError.malformedProtobuf
throw BinaryDelimited.Error.malformedLength
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions Tests/SwiftProtobufTests/Test_AsyncMessageSequence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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> {
Expand Down
112 changes: 93 additions & 19 deletions Tests/SwiftProtobufTests/Test_BinaryDelimited.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.truncated)
}
}

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()

Expand Down Expand Up @@ -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)
}

}

0 comments on commit 778ec5c

Please sign in to comment.