Skip to content

Fix stream_socket_get_crypto_status() after supplemental read#22332

Open
iluuu1994 wants to merge 2 commits into
php:masterfrom
iluuu1994:stream_socket_get_crypto_status-after-supplemental-read
Open

Fix stream_socket_get_crypto_status() after supplemental read#22332
iluuu1994 wants to merge 2 commits into
php:masterfrom
iluuu1994:stream_socket_get_crypto_status-after-supplemental-read

Conversation

@iluuu1994

@iluuu1994 iluuu1994 commented Jun 16, 2026

Copy link
Copy Markdown
Member

Let's assume we have a stream with 5 queued bytes. When calling fread($s, 1), the underlying buffered stream will request a chunk of up to 8192 bytes immediately, but only return the requested 1 byte back to the reader. If the reader then requests fread($s, 10), _php_stream_read() will first return anything that was buffered but not yet read.

If the requested length exceeds the number of buffered bytes (as is the case above), another read call is issued. This call will return nothing, because the stream only provides 4 more readable bytes, all of which are buffered. php_openssl_handle_ssl_error() (called by php_openssl_sockop_io()) will then incorrectly set last_status to WANT_READ, even though we've already read the remaining data.

Furthermore, stream_select() can cause the same issue via php_openssl_sockop_cast(castas: PHP_STREAM_AS_FD_FOR_SELECT), which pre-fills the read buffer on SSL_pending() > 0. The subsequent fread() will lead to the same condition as above.

There's a second issue here. If the stream is blocking, the supplement read will block for the duration of the timeout. This will be addressed in a second PR.

For context: https://github.com/php/php-src/actions/runs/27519531617/job/81334718266

Let's assume we have a stream with 5 queued bytes. When calling fread($s, 1),
the underlying buffered stream will request a chunk of up to 8192 bytes
immediately, but only return the requested 1 byte back to the reader. If the
reader then requests fread($s, 10), _php_stream_read() will first return
anything that was buffered but not yet read.

If the requested length exceeds the number of buffered bytes (as is the case
above), another read call is issued. This call will return nothing, because the
stream only provides 4 more readable bytes, all of which are buffered.
php_openssl_handle_ssl_error() (called by php_openssl_sockop_io()) will then
incorrectly set last_status to WANT_READ, even though we've already read the
remaining data.

Furthermore, stream_select() can cause the same issue via
php_openssl_sockop_cast(castas: PHP_STREAM_AS_FD_FOR_SELECT), which pre-fills
the read buffer on SSL_pending() > 0. The subsequent fread() will lead to the
same condition as above.

There's a second issue here. If the stream is blocking, the supplement read will
block for the duration of the timeout. This will be addressed in a second PR.
@iluuu1994 iluuu1994 requested a review from bukka as a code owner June 16, 2026 13:19
iluuu1994 added a commit to iluuu1994/php-src that referenced this pull request Jun 17, 2026
This issue is related to phpGH-22332:

> Let's assume we have a stream with 5 queued bytes. When calling fread($s, 1),
> the underlying buffered stream will request a chunk of up to 8192 bytes
> immediately, but only return the requested 1 byte back to the reader. If the
> reader then requests fread($s, 10), _php_stream_read() will first return
> anything that was buffered but not yet read.
>
> If the requested length exceeds the number of buffered bytes (as is the case
> above), another read call is issued. This call will return nothing, because
> the stream only provides 4 more readable bytes, all of which are buffered.
> php_openssl_handle_ssl_error() (called by php_openssl_sockop_io()) will then
> incorrectly set last_status to WANT_READ, even though we've already read the
> remaining data.
>
> Furthermore, stream_select() can cause the same issue via
> php_openssl_sockop_cast(castas: PHP_STREAM_AS_FD_FOR_SELECT), which pre-fills
> the read buffer on SSL_pending() > 0. The subsequent fread() will lead to the
> same condition as above.
>
> There's a second issue here. If the stream is blocking, the supplement read
> will block for the duration of the timeout. This will be addressed in a second
> PR.

This addresses the last paragraph. Avoid a blocking read when we already have
buffered data, as we may have reached the end of the stream and will wait in
vain.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant