From 28ddfaeda7ab25e9083b87893a631b23854889b1 Mon Sep 17 00:00:00 2001 From: mustiikhalil <26250654+mustiikhalil@users.noreply.github.com> Date: Tue, 24 Dec 2024 07:29:09 +0100 Subject: [PATCH] Fixes a bug that made a copy of the changing vars within the verifier leading to an incorrect count (#8451) Removes all the unneeded keyword (mutating) from verifier Adds tests to verify depth --- swift/Sources/FlatBuffers/Verifier.swift | 83 ++++++++++++------- .../FlatBuffersVectorsTests.swift | 3 - .../FlatbuffersVerifierTests.swift | 55 +++++++++++- 3 files changed, 104 insertions(+), 37 deletions(-) diff --git a/swift/Sources/FlatBuffers/Verifier.swift b/swift/Sources/FlatBuffers/Verifier.swift index 7b3170426f2..0d52ccd8a8d 100644 --- a/swift/Sources/FlatBuffers/Verifier.swift +++ b/swift/Sources/FlatBuffers/Verifier.swift @@ -22,22 +22,29 @@ public struct Verifier { /// Flag to check for alignment if true fileprivate let _checkAlignment: Bool - /// Capacity of the current buffer - fileprivate var _capacity: Int - /// Current ApparentSize - fileprivate var _apparentSize: UOffset = 0 - /// Amount of tables present within a buffer - fileprivate var _tableCount = 0 - - /// Capacity of the buffer - internal var capacity: Int { _capacity } - /// Current reached depth within the buffer - internal var _depth = 0 + /// Storage for all changing values within the verifier + private let storage: Storage /// Current verifiable ByteBuffer internal var _buffer: ByteBuffer /// Options for verification internal let _options: VerifierOptions + /// Current stored capacity within the verifier + var capacity: Int { + storage.capacity + } + + /// Current depth of verifier + var depth: Int { + storage.depth + } + + /// Current table count + var tableCount: Int { + storage.tableCount + } + + /// Initializer for the verifier /// - Parameters: /// - buffer: Bytebuffer that is required to be verified @@ -54,15 +61,15 @@ public struct Verifier { } _buffer = buffer - _capacity = buffer.capacity _checkAlignment = checkAlignment _options = options + storage = Storage(capacity: buffer.capacity) } /// Resets the verifier to initial state - public mutating func reset() { - _depth = 0 - _tableCount = 0 + public func reset() { + storage.depth = 0 + storage.tableCount = 0 } /// Checks if the value of type `T` is aligned properly in the buffer @@ -70,7 +77,7 @@ public struct Verifier { /// - position: Current position /// - type: Type of value to check /// - Throws: `missAlignedPointer` if the pointer is not aligned properly - public mutating func isAligned(position: Int, type: T.Type) throws { + public func isAligned(position: Int, type: T.Type) throws { /// If check alignment is false this mutating function doesnt continue if !_checkAlignment { return } @@ -94,13 +101,13 @@ public struct Verifier { /// - Throws: `outOfBounds` if the value is out of the bounds of the buffer /// and `apparentSizeTooLarge` if the apparent size is bigger than the one specified /// in `VerifierOptions` - public mutating func rangeInBuffer(position: Int, size: Int) throws { + public func rangeInBuffer(position: Int, size: Int) throws { let end = UInt(clamping: (position &+ size).magnitude) if end > _buffer.capacity { - throw FlatbuffersErrors.outOfBounds(position: end, end: capacity) + throw FlatbuffersErrors.outOfBounds(position: end, end: storage.capacity) } - _apparentSize = _apparentSize &+ UInt32(size) - if _apparentSize > _options._maxApparentSize { + storage.apparentSize = storage.apparentSize &+ UInt32(size) + if storage.apparentSize > _options._maxApparentSize { throw FlatbuffersErrors.apparentSizeTooLarge } } @@ -111,7 +118,7 @@ public struct Verifier { /// - position: Current readable position /// - type: Type of value to check /// - Throws: FlatbuffersErrors - public mutating func inBuffer(position: Int, of type: T.Type) throws { + public func inBuffer(position: Int, of type: T.Type) throws { try isAligned(position: position, type: type) try rangeInBuffer(position: position, size: MemoryLayout.size) } @@ -131,15 +138,15 @@ public struct Verifier { type: VOffset.self) try rangeInBuffer(position: vtablePosition, size: length) - _tableCount += 1 + storage.tableCount += 1 - if _tableCount > _options._maxTableCount { + if storage.tableCount > _options._maxTableCount { throw FlatbuffersErrors.maximumTables } - _depth += 1 + storage.depth += 1 - if _depth > _options._maxDepth { + if storage.depth > _options._maxDepth { throw FlatbuffersErrors.maximumDepth } @@ -154,7 +161,7 @@ public struct Verifier { /// - Parameter position: Current position to be read /// - Throws: `inBuffer` errors /// - Returns: a value of type `T` usually a `VTable` or a table offset - internal mutating func getValue(at position: Int) throws -> T { + internal func getValue(at position: Int) throws -> T { try inBuffer(position: position, of: T.self) return _buffer.read(def: T.self, position: position) } @@ -165,7 +172,7 @@ public struct Verifier { /// - Throws: `inBuffer` errors & `signedOffsetOutOfBounds` /// - Returns: Current readable position for a field @inline(__always) - internal mutating func derefOffset(position: Int) throws -> Int { + internal func derefOffset(position: Int) throws -> Int { try inBuffer(position: position, of: Int32.self) let offset = _buffer.read(def: Int32.self, position: position) @@ -197,14 +204,14 @@ public struct Verifier { } /// finishes the current iteration of verification on an object - internal mutating func finish() { - _depth -= 1 + internal func finish() { + storage.depth -= 1 } @inline(__always) - mutating func verify(id: String) throws { + func verify(id: String) throws { let size = MemoryLayout.size - guard _capacity >= (size * 2) else { + guard storage.capacity >= (size * 2) else { throw FlatbuffersErrors.bufferDoesntContainID } let str = _buffer.readString(at: size, count: size) @@ -214,4 +221,18 @@ public struct Verifier { throw FlatbuffersErrors.bufferIdDidntMatchPassedId } + final private class Storage { + /// Current ApparentSize + fileprivate var apparentSize: UOffset = 0 + /// Amount of tables present within a buffer + fileprivate var tableCount = 0 + /// Capacity of the current buffer + fileprivate let capacity: Int + /// Current reached depth within the buffer + fileprivate var depth = 0 + + init(capacity: Int) { + self.capacity = capacity + } + } } diff --git a/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/FlatBuffersVectorsTests.swift b/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/FlatBuffersVectorsTests.swift index ab9d2fea9a4..f0dedfeb3f8 100644 --- a/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/FlatBuffersVectorsTests.swift +++ b/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/FlatBuffersVectorsTests.swift @@ -65,9 +65,6 @@ final class FlatBuffersVectors: XCTestCase { var b = FlatBufferBuilder(initialSize: 100) let o = b.createVector(ofStructs: vector) b.finish(offset: o) - vector.withUnsafeBytes { pointer in - print(Array(pointer)) - } // swiftformat:disable all XCTAssertEqual(b.sizedByteArray, [4, 0, 0, 0, 3, 0, 0, 0, 0, 0, 128, 63, 0, 0, 0, 64, 0, 0, 64, 64, 0, 0, 128, 64, 0, 0, 160, 64, 0, 0, 192, 64, 0, 0, 224, 64, 0, 0, 0, 65, 0, 0, 16, 65]) // swiftformat:enable all diff --git a/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/FlatbuffersVerifierTests.swift b/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/FlatbuffersVerifierTests.swift index 71a00789073..53c40278762 100644 --- a/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/FlatbuffersVerifierTests.swift +++ b/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/FlatbuffersVerifierTests.swift @@ -75,7 +75,7 @@ final class FlatbuffersVerifierTests: XCTestCase { } func testVerifierCheckAlignment() { - var verifier = try! Verifier(buffer: &buffer) + let verifier = try! Verifier(buffer: &buffer) do { try verifier.isAligned(position: 20, type: Int.self) } catch { @@ -85,7 +85,7 @@ final class FlatbuffersVerifierTests: XCTestCase { } XCTAssertNoThrow(try verifier.isAligned(position: 16, type: Int.self)) - var newVerifer = try! Verifier(buffer: &buffer, checkAlignment: false) + let newVerifer = try! Verifier(buffer: &buffer, checkAlignment: false) XCTAssertNoThrow(try newVerifer.isAligned(position: 16, type: Int.self)) } @@ -112,7 +112,7 @@ final class FlatbuffersVerifierTests: XCTestCase { } func testPositionInBuffer() { - var verifier = try! Verifier(buffer: &buffer) + let verifier = try! Verifier(buffer: &buffer) XCTAssertNoThrow(try verifier.inBuffer(position: 0, of: Int64.self)) XCTAssertNoThrow(try verifier.inBuffer(position: 24, of: Int64.self)) XCTAssertThrowsError(try verifier.inBuffer(position: -9, of: Int64.self)) @@ -134,6 +134,9 @@ final class FlatbuffersVerifierTests: XCTestCase { var verifier = try! Verifier(buffer: &validFlatbuffersObject) var tableVerifer = try! verifier.visitTable(at: 48) + XCTAssertEqual(verifier.depth, 1) + XCTAssertEqual(verifier.tableCount, 1) + XCTAssertNoThrow(try tableVerifer.visit( field: 4, fieldName: "Vec", @@ -210,6 +213,8 @@ final class FlatbuffersVerifierTests: XCTestCase { error as! FlatbuffersErrors, .missAlignedPointer(position: 25, type: "UInt16")) } + tableVerifer.finish() + XCTAssertEqual(verifier.depth, 0) } func testVerifyUnionVectors() { @@ -291,7 +296,51 @@ final class FlatbuffersVerifierTests: XCTestCase { XCTAssertNoThrow(try getCheckedRoot(byteBuffer: &buf) as Movie) } + func testNestedTables() throws { + var builder = FlatBufferBuilder() + let name = builder.create(string: "Monster") + + let enemy = MyGame_Example_Monster.createMonster( + &builder, + nameOffset: name) + let currentName = builder.create(string: "Main name") + let monster = MyGame_Example_Monster.createMonster( + &builder, + nameOffset: currentName, + enemyOffset: enemy) + builder.finish(offset: monster) + + var sizedBuffer = builder.sizedBuffer + var verifier = try! Verifier(buffer: &sizedBuffer) + var tableVerifer = try! verifier.visitTable( + at: try getOffset(at: 0, within: verifier)) + XCTAssertEqual(verifier.depth, 1) + XCTAssertEqual(verifier.tableCount, 1) + + let position = try tableVerifer.dereference(28)! + + var nestedTable = try verifier.visitTable( + at: try getOffset(at: position, within: verifier)) + + XCTAssertEqual(verifier.depth, 2) + XCTAssertEqual(verifier.tableCount, 2) + nestedTable.finish() + XCTAssertEqual(verifier.depth, 1) + XCTAssertEqual(verifier.tableCount, 2) + tableVerifer.finish() + XCTAssertEqual(verifier.depth, 0) + XCTAssertEqual(verifier.tableCount, 2) + } + func add(buffer: inout ByteBuffer, v: Int32, p: Int) { buffer.write(value: v, index: p) } + + private func getOffset( + at value: Int, + within verifier: Verifier) throws -> Int + { + let offset: UOffset = try verifier.getValue(at: value) + return Int(clamping: (Int(offset) &+ 0).magnitude) + } }