CryptoStream.Flush contradicts Stream.Flush

[paulomorgado]

Stream.Flush remarks state that:

Override Flush on streams that implement a buffer. Use this method to move any information from an underlying buffer to its destination, clear the buffer, or both.

However, CryptoStream.Flush remarks state that:

Flushing the stream will not flush its underlying encoder unless you explicitly call Flush or Close. Setting AutoFlush to true means that data will be flushed from the buffer to the stream, but the encoder state will not be flushed. This allows the encoder to keep its state (partial characters) so that it can encode the next block of characters correctly. This scenario affects UTF8 and UTF7 where certain characters can only be encoded after the encoder receives the adjacent character or characters.

Note You should call either the Close method or the FlushFinalBlock method to complete flushing the buffer.

My understanding is that Flush flushes whatever it can and there's no guarantee that all data will be flushed.

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones

[bartonjs]

I'm not sure how it contradicts Stream.Flush since they both say that exact same paragraph. I'm not sure what it means for Stream, and I'm double not sure what it means for CryptoStream... it seems to be meant for StreamWriter (or TextWriter), not Stream, and not all the derived types of Stream that it got copied to.

That said, CryptoStream.Flush only flushes the stream it wraps. If the ICryptoTransform object has any notion of "flushing", Flush doesn't impact that at all (only TransformFinalBlock would have such an effect).

The best fix here is probably just to delete the paragraph in remarks, since it seems to have been some bad copy/paste.

[paulomorgado]

@bartonjs,

I think this needs clarification.

I don't see there's nothing wrong with CryptoStream. It's Stream stated assumptions about all implementations that is wrong. CryptoStream just proves them wrong.

Close and Dispose are the only operations that guarantee all work has been done for any Stream implementation.

[bartonjs]

I guess I'm not understanding what change you're hoping for. A change in just CryptoStream docs? A change in Stream docs? A change in the docs for every type that inherits Stream and says this same thing?

Based on the title I assumed "just CryptoStream", which is what my earlier response is based on.

[paulomorgado]

I guess I'm not understanding what change you're hoping for. A change in just CryptoStream docs? A change in Stream docs? A change in the docs for every type that inherits Stream and says this same thing?

Based on the title I assumed "just CryptoStream", which is what my earlier response is based on.

A change in Stream docs. CryptoStream is just one case where what the Stream docs state is not always true. There might be others.

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @dotnet/area-system-io

[jozkee]

move any information from an underlying buffer to its destination, clear the buffer, or both.

I still think this is a valid guideline for Stream.Flush. I would rather amend the documentation to state that some implementations are unable to flush the buffer since they need to maintain a state (due to an underlying encoder for example).

[paulomorgado]

move any information from an underlying buffer to its destination, clear the buffer, or both.

I still think this is a valid guideline for Stream.Flush. I would rather amend the documentation to state that some implementations are unable to flush the buffer since they need to maintain a state (due to an underlying encoder for example).

I'm sorry if I wasn't clear that that was my point.