unbuffered: After `Closed` no `WriteTraffic` state arrives

[uazu]

Checklist

• I've searched the issue tracker for similar bugs.

Describe the bug
I've updated my pipebuf crate Rustls wrapper in crate pipebuf_rustls to add unbuffered Rustls support. However my tests which succeed in the buffered mode fail for the unbuffered mode. The failing test does the following:

• Client sends one byte
• Client closes connection
• Server sends one byte
• Server closes connection

The byte sent by the server cannot be processed through the unbuffered interface because the process_tls_records call does not give a WriteTraffic state again after Closed. Without access to WriteTraffic, I can't send the data. The buffered interface does not have this limitation.

To Reproduce
Check out pipebuf_rustls from github (https://github.com/uazu/pipebuf_rustls). Run a single test using cargo test --no-default-features --features unbuffered byte_each_way. There is an assertion failure due to 1 byte sent but 0 bytes received.

Applicable Version(s)
0.23.4 on Linux

Expected behavior
I'd expect this case to be supported. I don't know how you'd want to support it with your unbuffered API though.

Additional context
The reason I added unbuffered support to pipebuf_rustls was that this is a much better fit to the pipebuf model. I added PBufRd::data_mut to pipebuf 0.3.1 to support Rustls unbuffered mode (which requires &mut [u8] for incoming data). So I support the unbuffered effort. Just it doesn't cover all necessary cases yet.

[ctz]

Thanks for the report. Yes, the current API treats a half-close as a full-close. I think we should add additional ConnectionStates so the server in your example can finish its connection with:

• ReadTraffic reads the client's close_notify
• PeerClosed informs the application that no further data will be forthcoming from peer
• WriteTraffic for additional server-sent traffic, followed by queue_close_notify
• EncodeTlsData for outputting that data
• TransmitTlsData for confirming the data was sent
• Closed nothing further can happen with this connection

(This arrangement retains Closed as a terminal, full-close state.)

[djc]

Will this need semver-incompatible changes? If so, it might take us a while to get this released.

[cpu]

The ConnectionState enum is non_exhaustive so adding new states there isn't semver breaking in itself. I think it would be more a question of how the semantics of the overall API are updated and whether it affects existing usages.

[uazu]

Thanks for the quick response. There is no rush from my point of view as I also have it working with the buffered interface, and I can continue to use that. So if it's a breaking change for 0.24 that's not a problem. The API changes you suggested sound fine, but I'll code against whatever you come up with. If you want me to test against a pre-release version of Rustls I can do that.

Also it would be helpful to have some guidance about buffer-sizing requirements, either in the docs or as API calls. For example I'm allowing 18KiB for EncodeTlsData because that appears to be the maximum required by protocol, but typically it will be much less. I can add another issue for this if you wish.