apple / swift-nio-ssl

TLS Support for SwiftNIO, based on BoringSSL.
https://swiftpackageindex.com/apple/swift-nio-ssl/main/documentation/niossl
Apache License 2.0
385 stars 139 forks source link

`NIOSSLHandler`: behave sensibly on `close(mode: .output)` #428

Closed felixschlegel closed 1 year ago

felixschlegel commented 1 year ago

Motivation

If NIOSSLHandler receives close(mode: .output) or close(mode: .input), it ignores them, optionally failing their promises. This is weird behaviour. It should do better.

Modifications

Warning: This PR relies on EmbeddedChannel supporting ChannelOptions.Types.AllowRemoteHalfClosureOption, which is currently not possible and proposed as a Patch to swift-nio

Note: I have removed a lot of whitespace, so consider selecting Hide Whitespace in the GitHub Files Changed tab

felixschlegel commented 1 year ago

Hey @Lukasa ,

I have implemented your requested changes and added the buffering of closeOutput when we are still in state = .handshaking / .additionalVerification.

I still have a couple of open questions for this PR:

Best regards, -Felix

Lukasa commented 1 year ago

@swift-server-bot add to allowlist

Lukasa commented 1 year ago

should we also support scheduledShutdown for close(mode: .output) (see the normal close mode for reference)

No, scheduled shutdown is waiting for us to get a CLOSE_NOTIFY from the remote peer. In this mode, we don't expect one.

what if the downstream ChannelHandler does not support close(mode: .output)?

Then the partial close fails and the user should upgrade to full close.

felixschlegel commented 1 year ago

@swift-server-bot test this please

yim-lee commented 1 year ago

@swift-server-bot test this please