Skip to content

Commit

Permalink
Fix readSizeLimit being ignored in a specific case
Browse files Browse the repository at this point in the history
When reading data up to a certain delimiter with the delimiter already present in the buffer (for a GenericStreamReader), the readSizeLimit was ignored.
  • Loading branch information
Frizlab committed Jan 31, 2024
1 parent 55e69c4 commit 1c6d95b
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 8 deletions.
29 changes: 22 additions & 7 deletions Sources/StreamReader/Implementations/GenericStreamReader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,26 @@ public final class GenericStreamReader : StreamReader {
public private(set) var currentReadPosition = 0

/**
The maximum number of bytes that can be returned by the read methods (when updating read position),
The maximum total number of bytes that can be returned by the read methods (when updating read position),
and also the number of bytes that can be read from the underlying stream.

Changing this to a greater value will force ``streamHasReachedEOF`` to `false`,
but next read might reach `EOF` directly regardless.

Changing this to a lower value will not change ``streamHasReachedEOF`` at all. */
Changing this to a lower value will not change ``streamHasReachedEOF`` at all.

The value sets the _total_ number of bytes allowed to be read.
Let’s see an example, reading a stream whose content is `11 22 33 44 55 66 77 88 99 00` (10 bytes):
- Set the limit to `nil` (no limit);
- Read 5 bytes from the stream -> ok, return `11 22 33 44 55`;
- Set the limit to 7;
- Read 3 bytes from the stream (w/o allowing reading less) -> throws notEnoughData;
- Set the limit to 8;
- Read 3 bytes from the stream (w/o allowing reading less) -> ok, returns `66 77 88`;
- Read data up to delimiter `99`, excluding the delimiter, w/o failing if not found
-> ok, returns an empty Data, _and an empty delimiter_ (the delimiter was not read as it’s past the read limit);
- Set the limit 9;
- Read data up to delimiter `99`, excluding the delimiter, w/o failing if not found -> ok, returns empty Data and `99` in the delimiter. */
public var readSizeLimit: Int? {
didSet {
if readSizeLimit ?? Int.max > oldValue ?? Int.max {
Expand Down Expand Up @@ -162,11 +175,13 @@ public final class GenericStreamReader : StreamReader {
var unmatchedDelimiters = Array(delimiters.filter{ !$0.isEmpty }.enumerated())
let (minDelimiterLength, maxDelimiterLength) = (unmatchedDelimiters.map(\.element.count).min() ?? 0, unmatchedDelimiters.map(\.element.count).max() ?? 0)

let allowedToBeRead = readSizeLimit.flatMap{ $0 - currentReadPosition } ?? .max

var searchOffset = 0
repeat {
assert(bufferValidLength - searchOffset >= 0, "INTERNAL LOGIC ERROR")
let bufferStart = buffer + bufferStartPos
let bufferSearchData = UnsafeRawBufferPointer(start: bufferStart + searchOffset, count: bufferValidLength - searchOffset)
let bufferSearchData = UnsafeRawBufferPointer(start: bufferStart + searchOffset, count: min(bufferValidLength - searchOffset, allowedToBeRead))
if let match = matchDelimiters(inData: bufferSearchData, dataStartOffset: searchOffset, usingMatchingMode: matchingMode, includeDelimiter: includeDelimiter, minDelimiterLength: minDelimiterLength, withUnmatchedDelimiters: &unmatchedDelimiters, bestMatch: &bestMatch) {
if updateReadPosition {
bufferStartPos += match.length
Expand Down Expand Up @@ -202,13 +217,13 @@ public final class GenericStreamReader : StreamReader {
}

/* No match, no error on no match, we return the whole data. */
let returnedLength = bufferValidLength
let returnedLength = min(bufferValidLength, allowedToBeRead)
let bufferStart = buffer + bufferStartPos

if updateReadPosition {
currentReadPosition += bufferValidLength
bufferStartPos += bufferValidLength
bufferValidLength = 0
bufferStartPos += returnedLength
bufferValidLength -= returnedLength
currentReadPosition += returnedLength
}

return try handler(UnsafeRawBufferPointer(start: bufferStart, count: returnedLength), Data())
Expand Down
20 changes: 19 additions & 1 deletion Tests/StreamReaderTests/StreamReaderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -415,11 +415,29 @@ class StreamReaderTests : XCTestCase {
XCTAssertGreaterThan(try reader.readStreamInBuffer(size: 4, allowMoreThanOneRead: false, bypassUnderlyingStreamReadSizeLimit: true), 0)
reader.readSizeLimit = 1
XCTAssertThrowsError(try reader.readData(size: 2))
let (checkedData, checkedDelimiter) = try reader.readData(upTo: [Data(hexEncoded: "45")!], matchingMode: .anyMatchWins, includeDelimiter: false)
let (checkedData, checkedDelimiter) = try reader.readData(upTo: [Data(hexEncoded: "45")!], matchingMode: .anyMatchWins, failIfNotFound: false, includeDelimiter: false)
XCTAssertEqual(checkedData, Data(hexEncoded: "01")!)
XCTAssertEqual(checkedDelimiter, Data())
}

func testReadLimitFromDoc() throws {
try runTest(hexDataString: "11 22 33 44 55 66 77 88 99 00", bufferSizes: [1, 42], bufferSizeIncrements: Array(1...5), underlyingStreamReadSizeLimits: [nil]){ reader, data, limit, bufferSize, bufferSizeIncrement, underlyingStreamReadSizeLimit in
try (reader as? GenericStreamReader)?.readStreamInBuffer(size: 42) /* <- This is the clever bit. Setting the buffer size to the same size does the same but does not guarantee it. */
XCTAssertEqual(try reader.readData(size: 5, allowReadingLess: false), Data(hexEncoded: "11 22 33 44 55")!)
reader.readSizeLimit = 7
XCTAssertThrowsError(try reader.readData(size: 3, allowReadingLess: false))
reader.readSizeLimit = 8
XCTAssertEqual(try reader.readData(size: 3, allowReadingLess: false), Data(hexEncoded: "66 77 88")!)
let (checkedData1, checkedDelimiter1) = try reader.readData(upTo: [Data(hexEncoded: "99")!], matchingMode: .anyMatchWins, failIfNotFound: false, includeDelimiter: false)
XCTAssertEqual(checkedData1, Data())
XCTAssertEqual(checkedDelimiter1, Data())
reader.readSizeLimit = 9
let (checkedData2, checkedDelimiter2) = try reader.readData(upTo: [Data(hexEncoded: "99")!], matchingMode: .anyMatchWins, failIfNotFound: false, includeDelimiter: false)
XCTAssertEqual(checkedData2, Data())
XCTAssertEqual(checkedDelimiter2, Data(hexEncoded: "99")!)
}
}

@available(macOS 10.15.4, iOS 13.4, tvOS 13.4, *)
func testReadErrorFromFileHandle() throws {
let tmpFileURL = URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true).appendingPathComponent("StreamReaderTest_\(Int.random(in: 0..<4242))")
Expand Down

0 comments on commit 1c6d95b

Please sign in to comment.