Skip to content

Commit

Permalink
[NIOFileSystem] Avoid crash when reading more than ByteBuffer capacity
Browse files Browse the repository at this point in the history
Motivation:

As described in issue (apple#2878)[apple#2878], NIOFileSystem crashes when reading more than `ByteBuffer` capacity.

Modifications:

- Add a new static property, `byteBufferCapacity`, to `ByteCount` representing the maximum amount of bytes that can be written to `ByteBuffer`.
- Throw a `FileSystemError` in `ReadableFileHandleProtocol.readToEnd(fromAbsoluteOffset:maximumSizeAllowed:)` when `maximumSizeAllowed` is more than `ByteCount.byteBufferCapacity`.

Result:

NIOFileSystem will `throw` instead of crashing.
  • Loading branch information
clintonpi committed Oct 9, 2024
1 parent 9ff5fdd commit d003730
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 1 deletion.
15 changes: 15 additions & 0 deletions Sources/NIOFileSystem/ByteCount.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ public struct ByteCount: Hashable, Sendable {
}
}

extension ByteCount {
#if arch(arm) || arch(i386) || arch(arm64_32)
// on 32-bit platforms we can't make use of a whole UInt32.max (as it doesn't fit in an Int)
private static let byteBufferMaxIndex = UInt32(Int.max)
#else
// on 64-bit platforms we're good
private static let byteBufferMaxIndex = UInt32.max
#endif

/// A ``ByteCount`` for the maximum amount of bytes that can be written to `ByteBuffer`.
internal static var byteBufferCapacity: ByteCount {
ByteCount(bytes: Int64(byteBufferMaxIndex))
}
}

extension ByteCount: AdditiveArithmetic {
public static var zero: ByteCount { ByteCount(bytes: 0) }

Expand Down
17 changes: 16 additions & 1 deletion Sources/NIOFileSystem/FileHandleProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,9 @@ extension ReadableFileHandleProtocol {
///
/// - Important: This method checks whether the file is seekable or not (i.e., whether it's a socket,
/// pipe or FIFO), and will throw ``FileSystemError/Code-swift.struct/unsupported`` if
/// an offset other than zero is passed.
/// an offset other than zero is passed. Also, it will throw
/// ``FileSystemError/Code-swift.struct/resourceExhausted`` if `maximumSizeAllowed` is more than can be
/// written to `ByteBuffer`.
///
/// - Parameters:
/// - offset: The absolute offset into the file to read from. Defaults to zero.
Expand All @@ -340,6 +342,19 @@ extension ReadableFileHandleProtocol {
let fileSize = Int64(info.size)
let readSize = max(Int(fileSize - offset), 0)

if maximumSizeAllowed > .byteBufferCapacity {
throw FileSystemError(
code: .resourceExhausted,
message: """
The maximum size allowed (\(maximumSizeAllowed)) is more than the maximum \
amount of bytes that can be written to ByteBuffer \
(\(ByteCount.byteBufferCapacity)).
""",
cause: nil,
location: .here()
)
}

if readSize > maximumSizeAllowed.bytes {
throw FileSystemError(
code: .resourceExhausted,
Expand Down
15 changes: 15 additions & 0 deletions Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1803,6 +1803,21 @@ extension FileSystemTests {
XCTAssertEqual(byteCount, Int(size))
}
}

func testReadMoreThanByteBufferCapacity() async throws {
let path = try await self.fs.temporaryFilePath()

try await self.fs.withFileHandle(forReadingAndWritingAt: path) { fileHandle in
await XCTAssertThrowsFileSystemErrorAsync {
// Set `maximumSizeAllowed` to 1 byte more than can be written to `ByteBuffer`.
try await fileHandle.readToEnd(
maximumSizeAllowed: .byteBufferCapacity + .bytes(1)
)
} onError: { error in
XCTAssertEqual(error.code, .resourceExhausted)
}
}
}
}

#if !canImport(Darwin) && swift(<5.9.2)
Expand Down

0 comments on commit d003730

Please sign in to comment.