Skip to content

Fix: OpenSSL sockets shouldn't flush on read#16650

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
ysbaddaden:fix/openssl-bio-dont-flush-on-read
Feb 12, 2026
Merged

Fix: OpenSSL sockets shouldn't flush on read#16650
straight-shoota merged 2 commits intocrystal-lang:masterfrom
ysbaddaden:fix/openssl-bio-dont-flush-on-read

Conversation

@ysbaddaden
Copy link
Collaborator

@ysbaddaden ysbaddaden commented Feb 10, 2026

See #16640 (comment)

There's no reason for reading from the socket to autoflush the write buffer of the underlying IO object (e.g. TCP socket), especially since we already disable the buffers in OpenSSL::SSL::Socket#initialize, so it ends up being a NOOP in practice.

Crystal::BIO#initialize now always disables the buffers, so the logic is grouped together, instead of having OpenSSL::SSL::Socket do it.

NOTE: this is potentially a breaking change if someone re-enabled buffers or expects the buffers to be buffered (in non SSL socket situations).

@ysbaddaden ysbaddaden self-assigned this Feb 10, 2026
@ysbaddaden ysbaddaden added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:crypto breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. labels Feb 10, 2026
@straight-shoota straight-shoota added this to the 1.20.0 milestone Feb 10, 2026
ysbaddaden added a commit to ysbaddaden/crystal that referenced this pull request Feb 10, 2026
@straight-shoota straight-shoota merged commit 37d25b0 into crystal-lang:master Feb 12, 2026
55 checks passed
@ysbaddaden ysbaddaden deleted the fix/openssl-bio-dont-flush-on-read branch February 12, 2026 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:crypto

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants