Skip to content

Commit

Permalink
Unbuffer reads after handshake (#379)
Browse files Browse the repository at this point in the history
# Motivation
We missed unbuffering reads after the handshake completed. This could result in reads that were buffered while the handshake was in progress to not unbuffer.

# Modification
This PR unbuffers any reads after the handshake is completed.

# Result
We are now unbuffering reads correctly.
  • Loading branch information
FranzBusch authored Jul 13, 2022
1 parent fd5f38b commit c30c680
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 0 deletions.
3 changes: 3 additions & 0 deletions Sources/NIOSSL/NIOSSLHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,9 @@ public class NIOSSLHandler : ChannelInboundHandler, ChannelOutboundHandler, Remo
// write before we completed the handshake. We may also have pending reads if the user sent data immediately
// after their FINISHED record. We decode the reads first, as those reads may trigger writes.
self.doDecodeData(context: context)
if let receiveBuffer = self.plaintextReadBuffer {
self.doFlushReadData(context: context, receiveBuffer: receiveBuffer, readOnEmptyBuffer: false)
}
self.doUnbufferWrites(context: context)
}

Expand Down
1 change: 1 addition & 0 deletions Tests/NIOSSLTests/NIOSSLIntegrationTest+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ extension NIOSSLIntegrationTest {
("testExtractingCertificates", testExtractingCertificates),
("testForcingVerificationFailureNewCallback", testForcingVerificationFailureNewCallback),
("testErroringNewVerificationCallback", testErroringNewVerificationCallback),
("testReadsAreUnbufferedAfterHandshake", testReadsAreUnbufferedAfterHandshake),
("testNewCallbackCanDelayHandshake", testNewCallbackCanDelayHandshake),
("testExtractingCertificatesNewCallback", testExtractingCertificatesNewCallback),
("testNewCallbackCombinedWithDefaultTrustStore", testNewCallbackCombinedWithDefaultTrustStore),
Expand Down
70 changes: 70 additions & 0 deletions Tests/NIOSSLTests/NIOSSLIntegrationTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,76 @@ class NIOSSLIntegrationTest: XCTestCase {
XCTAssertThrowsError(try handshakeResultPromise.futureResult.wait())
}

func testReadsAreUnbufferedAfterHandshake() throws {
// This is a regression test for rdar://96850712
var config = TLSConfiguration.makeServerConfiguration(
certificateChain: [.certificate(NIOSSLIntegrationTest.cert)],
privateKey: .privateKey(NIOSSLIntegrationTest.key)
)
config.certificateVerification = .noHostnameVerification
let context = try assertNoThrowWithValue(NIOSSLContext(configuration: config))

let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
defer {
XCTAssertNoThrow(try group.syncShutdownGracefully())
}

var completionPromiseFired: Bool = false
let completionPromiseFiredLock = Lock()

let completionPromise: EventLoopPromise<ByteBuffer> = group.next().makePromise()
completionPromise.futureResult.whenComplete { _ in
completionPromiseFiredLock.withLock {
completionPromiseFired = true
}
}

var handshakeCompletePromise: EventLoopPromise<NIOSSLVerificationResult>? = nil
let handshakeFiredPromise: EventLoopPromise<Void> = group.next().makePromise()

let serverChannel: Channel = try serverTLSChannel(
context: context,
preHandlers: [],
postHandlers: [PromiseOnReadHandler(promise: completionPromise)],
group: group,
customVerificationCallback: { innerCertificates, promise in
handshakeCompletePromise = promise
handshakeFiredPromise.succeed(())
})
defer {
XCTAssertNoThrow(try serverChannel.close().wait())
}

let clientChannel = try clientTLSChannel(
context: try configuredSSLContext(),
preHandlers: [],
postHandlers: [],
group: group,
connectingTo: serverChannel.localAddress!
)
defer {
XCTAssertNoThrow(try clientChannel.close().wait())
}

var originalBuffer = clientChannel.allocator.buffer(capacity: 5)
originalBuffer.writeString("Hello")
clientChannel.writeAndFlush(originalBuffer, promise: nil)

// This has driven the handshake to begin, so we can wait for that.
XCTAssertNoThrow(try handshakeFiredPromise.futureResult.wait())

// We can now check whether the completion promise has fired: it should not have.
completionPromiseFiredLock.withLock {
XCTAssertFalse(completionPromiseFired)
}

// Ok, allow the handshake to run.
handshakeCompletePromise!.succeed(.certificateVerified)

let newBuffer = try completionPromise.futureResult.wait()
XCTAssertEqual(newBuffer, originalBuffer)
}

func testNewCallbackCanDelayHandshake() throws {
let context = try configuredSSLContext()

Expand Down

0 comments on commit c30c680

Please sign in to comment.