Skip to content

Commit

Permalink
Correctly resize ByteBuffers (#150)
Browse files Browse the repository at this point in the history
Motivation:

In two different places in this module we "reserve" space by
moving the writer index forward. This isn't the correct way to
do that, and it's a bit of a needless performance
optimization. Instead, just write a zero in that location and
come back to it.

Modifications:

- Replace unmotivated writer index move with a write.
- Add tests.

Result:

Fewer crashes
  • Loading branch information
Lukasa committed Jun 29, 2023
1 parent dcc089f commit db57f32
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Sources/NIOSSH/ByteBuffer+SSH.swift
Expand Up @@ -205,7 +205,7 @@ extension ByteBuffer {
mutating func writeCompositeSSHString(_ compositeFunction: (inout ByteBuffer) throws -> Int) rethrows -> Int {
// Reserve 4 bytes for the length.
let originalWriterIndex = self.writerIndex
self.moveWriterIndex(forwardBy: 4)
self.writeInteger(UInt32(0))

var writtenLength: Int
do {
Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOSSH/SSHPacketSerializer.swift
Expand Up @@ -73,7 +73,7 @@ extension ByteBuffer {
/// byte[m] mac (Message Authentication Code - MAC); m = mac_length

/// payload
self.moveWriterIndex(forwardBy: 5)
self.writeMultipleIntegers(UInt32(0), UInt8(0))
let messageLength = self.writeSSHMessage(message)

// Depending on on whether packet length is encrypted, padding should reflect that
Expand Down
27 changes: 27 additions & 0 deletions Tests/NIOSSHTests/ByteBuffer+SSHTests.swift
Expand Up @@ -361,6 +361,33 @@ final class ByteBufferSSHTests: XCTestCase {

XCTAssertNoThrow(XCTAssertNotNil(try buffer.readSSHHostKey()))
}

func testCompositeStringDoesTheRightThingWithBB() throws {
var buffer = ByteBuffer()
XCTAssertEqual(buffer.capacity, 0)

buffer.writeCompositeSSHString {
$0.writeInteger(UInt64(9))
}
let writtenBytes = buffer.readBytes(length: buffer.readableBytes)
XCTAssertEqual(
writtenBytes,
[0, 0, 0, 8, 0, 0, 0, 0, 0, 0, 0, 9]
)
}

func testWriteSSHPacketDoesTheRightThingWithBB() throws {
var buffer = ByteBuffer()
XCTAssertEqual(buffer.capacity, 0)

buffer.writeSSHPacket(message: .version("Tests_v1.0"), lengthEncrypted: false, blockSize: 8)

let writtenBytes = buffer.readBytes(length: 5)
XCTAssertEqual(
writtenBytes,
[0, 0, 0, 24, 11]
)
}
}

extension ByteBuffer {
Expand Down

0 comments on commit db57f32

Please sign in to comment.