Commit 19bbac58ffc for woocommerce
commit 19bbac58ffc65a4836a0f635074cabc646d63465
Author: Darren Ethier <darren@roughsmootheng.in>
Date: Sun Aug 23 11:43:09 2026 -0400
Handle download stream read failures (#67945)
Detect and handle stream read failures during force downloads. Prevents corrupted partial responses and clears stale download headers before returning an error or redirect.
diff --git a/plugins/woocommerce/changelog/fix-woo6-122-download-read-failure b/plugins/woocommerce/changelog/fix-woo6-122-download-read-failure
new file mode 100644
index 00000000000..153ddf86e76
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-woo6-122-download-read-failure
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Handle read errors while streaming product downloads.
diff --git a/plugins/woocommerce/includes/class-wc-download-handler.php b/plugins/woocommerce/includes/class-wc-download-handler.php
index 74ec30eefdf..61e3c67d2ef 100644
--- a/plugins/woocommerce/includes/class-wc-download-handler.php
+++ b/plugins/woocommerce/includes/class-wc-download-handler.php
@@ -21,6 +21,15 @@ class WC_Download_Handler {
*/
public const TRACK_DOWNLOAD_CALLBACK = 'track_partial_download';
+ /** Successful completion of a streamed file read. */
+ private const READ_RESULT_SUCCESS = 'success';
+
+ /** A streamed file read failed before emitting any file data. */
+ private const READ_RESULT_FAILURE_BEFORE_OUTPUT = 'failure_before_output';
+
+ /** A streamed file read failed after emitting file data. */
+ private const READ_RESULT_FAILURE_AFTER_OUTPUT = 'failure_after_output';
+
/**
* Hook in methods.
*/
@@ -506,7 +515,7 @@ class WC_Download_Handler {
// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_operations_fopen -- Streaming a remote file needs fopen (WP_Filesystem cannot stream); a false return is handled below.
$handle = @fopen( $parsed_file_path['file_path'], 'r' );
- $served = false;
+ $read_result = self::READ_RESULT_FAILURE_BEFORE_OUTPUT;
if ( false !== $handle ) {
$response_headers = stream_get_meta_data( $handle )['wrapper_data'] ?? array();
$filename = self::resolve_filename_from_response_headers(
@@ -518,15 +527,20 @@ class WC_Download_Handler {
);
self::download_headers( $parsed_file_path['file_path'], $filename, $download_range, true );
- $served = self::readfile_from_handle( $handle, $start, $length );
+ $read_result = self::readfile_from_handle( $handle, $start, $length );
}
} else {
self::download_headers( $parsed_file_path['file_path'], $filename, $download_range );
- $served = self::readfile_chunked( $parsed_file_path['file_path'], $start, $length );
+ $read_result = self::readfile_chunked_with_result( $parsed_file_path['file_path'], $start, $length );
}
- if ( ! $served ) {
- if ( $parsed_file_path['remote_file'] && 'yes' === get_option( 'woocommerce_downloads_redirect_fallback_allowed' ) ) {
+ if ( self::READ_RESULT_SUCCESS !== $read_result ) {
+ if ( self::READ_RESULT_FAILURE_AFTER_OUTPUT === $read_result ) {
+ wc_get_logger()->warning(
+ __( 'A file could not be completely served using the Force Download method because the response had already started.', 'woocommerce' )
+ );
+ } elseif ( $parsed_file_path['remote_file'] && 'yes' === get_option( 'woocommerce_downloads_redirect_fallback_allowed' ) ) {
+ self::remove_download_headers();
wc_get_logger()->warning(
sprintf(
/* translators: %1$s contains the filepath of the digital asset. */
@@ -817,6 +831,18 @@ class WC_Download_Handler {
* @return bool Success or fail
*/
public static function readfile_chunked( $file, $start = 0, $length = 0 ) {
+ return self::READ_RESULT_SUCCESS === self::readfile_chunked_with_result( $file, $start, $length );
+ }
+
+ /**
+ * Read a file in chunks and report when a failure follows partial output.
+ *
+ * @param string $file File.
+ * @param int $start Byte offset/position of the beginning from which to read from the file.
+ * @param int $length Length of the chunk to be read from the file in bytes, 0 means full file.
+ * @return string One of the READ_RESULT_* constants.
+ */
+ private static function readfile_chunked_with_result( $file, $start, $length ) {
// Define before attempting to open the file: the constant has always been defined even
// when the open fails, and external code may rely on that side effect.
if ( ! defined( 'WC_CHUNK_SIZE' ) ) {
@@ -826,7 +852,7 @@ class WC_Download_Handler {
$handle = @fopen( $file, 'r' ); // phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_read_fopen
if ( false === $handle ) {
- return false;
+ return self::READ_RESULT_FAILURE_BEFORE_OUTPUT;
}
if ( ! $length ) {
@@ -842,7 +868,7 @@ class WC_Download_Handler {
* @param resource $handle Open file handle, e.g. from `fopen()`.
* @param int $start Byte offset/position of the beginning from which to read from the file.
* @param int $length Length of the chunk to be read from the file in bytes, 0 means until the end of file.
- * @return bool Success or fail
+ * @return string One of the READ_RESULT_* constants.
*/
private static function readfile_from_handle( $handle, $start = 0, $length = 0 ) {
if ( ! defined( 'WC_CHUNK_SIZE' ) ) {
@@ -850,6 +876,7 @@ class WC_Download_Handler {
}
$read_length = (int) WC_CHUNK_SIZE;
+ $output_sent = false;
if ( $length ) {
$end = $start + $length - 1;
@@ -863,7 +890,15 @@ class WC_Download_Handler {
$read_length = $end - $p + 1;
}
- echo @fread( $handle, $read_length ); // phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged, WordPress.XSS.EscapeOutput.OutputNotEscaped, WordPress.WP.AlternativeFunctions.file_system_read_fread
+ $chunk = @fread( $handle, $read_length ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_operations_fread -- Streamed downloads require fread(); suppress warnings so they do not corrupt the binary response, and handle false below.
+
+ if ( false === $chunk ) {
+ @fclose( $handle ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_operations_fclose -- The read has already failed; suppress close warnings and report failure below.
+ return $output_sent ? self::READ_RESULT_FAILURE_AFTER_OUTPUT : self::READ_RESULT_FAILURE_BEFORE_OUTPUT;
+ }
+
+ echo $chunk; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Download chunks are raw binary data and must not be HTML-escaped.
+ $output_sent = $output_sent || '' !== $chunk;
$p = @ftell( $handle ); // phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged
if ( ob_get_length() ) {
@@ -873,7 +908,15 @@ class WC_Download_Handler {
}
} else {
while ( ! @feof( $handle ) ) { // @codingStandardsIgnoreLine.
- echo @fread( $handle, $read_length ); // @codingStandardsIgnoreLine.
+ $chunk = @fread( $handle, $read_length ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_operations_fread -- Streamed downloads require fread(); suppress warnings so they do not corrupt the binary response, and handle false below.
+
+ if ( false === $chunk ) {
+ @fclose( $handle ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_operations_fclose -- The read has already failed; suppress close warnings and report failure below.
+ return $output_sent ? self::READ_RESULT_FAILURE_AFTER_OUTPUT : self::READ_RESULT_FAILURE_BEFORE_OUTPUT;
+ }
+
+ echo $chunk; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Download chunks are raw binary data and must not be HTML-escaped.
+ $output_sent = $output_sent || '' !== $chunk;
if ( ob_get_length() ) {
ob_flush();
flush();
@@ -881,7 +924,12 @@ class WC_Download_Handler {
}
}
- return @fclose( $handle ); // phpcs:ignore Generic.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_read_fclose
+ // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_operations_fclose -- Suppress close warnings so they do not corrupt the binary response, and report failure below.
+ if ( @fclose( $handle ) ) {
+ return self::READ_RESULT_SUCCESS;
+ }
+
+ return $output_sent ? self::READ_RESULT_FAILURE_AFTER_OUTPUT : self::READ_RESULT_FAILURE_BEFORE_OUTPUT;
}
/**
@@ -900,6 +948,15 @@ class WC_Download_Handler {
return $headers;
}
+ /**
+ * Remove headers that describe a streamed download before sending another response type.
+ */
+ private static function remove_download_headers(): void {
+ foreach ( array( 'Content-Type', 'Content-Description', 'Content-Disposition', 'Content-Transfer-Encoding', 'Content-Length', 'Content-Range', 'Accept-Ranges' ) as $header ) {
+ header_remove( $header );
+ }
+ }
+
/**
* Die with an error message if the download fails.
*
@@ -915,10 +972,8 @@ class WC_Download_Handler {
if ( headers_sent() ) {
wc_get_logger()->log( 'warning', __( 'Headers already sent when generating download error message.', 'woocommerce' ) );
} else {
+ self::remove_download_headers();
header( 'Content-Type: ' . get_option( 'html_type' ) . '; charset=' . get_option( 'blog_charset' ) );
- header_remove( 'Content-Description;' );
- header_remove( 'Content-Disposition' );
- header_remove( 'Content-Transfer-Encoding' );
}
if ( ! strstr( $message, '<a ' ) ) {
diff --git a/plugins/woocommerce/tests/php/helpers/FakeRemoteStreamWrapper.php b/plugins/woocommerce/tests/php/helpers/FakeRemoteStreamWrapper.php
index b5cbd52d312..f6e20c43b5e 100644
--- a/plugins/woocommerce/tests/php/helpers/FakeRemoteStreamWrapper.php
+++ b/plugins/woocommerce/tests/php/helpers/FakeRemoteStreamWrapper.php
@@ -16,7 +16,8 @@ declare( strict_types = 1 );
* // ...
* stream_wrapper_restore( 'http' );
*
- * Only the operations the download handler performs are implemented; the stream is always empty.
+ * Only the operations the download handler performs are implemented; the stream is empty unless
+ * configured to fail reads.
* Parameter names and signatures are fixed by PHP's streamWrapper prototype, so unused ones are
* expected: https://www.php.net/manual/en/class.streamwrapper.php
*
@@ -29,6 +30,20 @@ declare( strict_types = 1 );
*/
class FakeRemoteStreamWrapper {
+ /**
+ * Whether reads should fail.
+ *
+ * @var bool
+ */
+ public static $fail_reads = false;
+
+ /**
+ * Whether the stream was closed.
+ *
+ * @var bool
+ */
+ public static $closed = false;
+
/**
* Stream context, assigned by PHP when the wrapper is instantiated.
*
@@ -46,6 +61,7 @@ class FakeRemoteStreamWrapper {
* @return bool
*/
public function stream_open( $path, $mode, $options, &$opened_path ) {
+ self::$closed = false;
return true;
}
@@ -53,9 +69,13 @@ class FakeRemoteStreamWrapper {
* Read from the stream.
*
* @param int $count Bytes to read.
- * @return string
+ * @return string|false
*/
public function stream_read( $count ) {
+ if ( self::$fail_reads ) {
+ return false;
+ }
+
return '';
}
@@ -65,9 +85,29 @@ class FakeRemoteStreamWrapper {
* @return bool
*/
public function stream_eof() {
+ return ! self::$fail_reads;
+ }
+
+ /**
+ * Seek within the stream.
+ *
+ * @param int $offset Seek offset.
+ * @param int $whence Seek origin.
+ * @return bool
+ */
+ public function stream_seek( $offset, $whence ) {
return true;
}
+ /**
+ * Get the current stream position.
+ *
+ * @return int
+ */
+ public function stream_tell() {
+ return 0;
+ }
+
/**
* Stat the open stream.
*
@@ -83,6 +123,7 @@ class FakeRemoteStreamWrapper {
* @return bool
*/
public function stream_close() {
+ self::$closed = true;
return true;
}
diff --git a/plugins/woocommerce/tests/php/includes/class-wc-download-handler-tests.php b/plugins/woocommerce/tests/php/includes/class-wc-download-handler-tests.php
index 8dab580f0fc..5b0f11384af 100644
--- a/plugins/woocommerce/tests/php/includes/class-wc-download-handler-tests.php
+++ b/plugins/woocommerce/tests/php/includes/class-wc-download-handler-tests.php
@@ -547,6 +547,74 @@ class WC_Download_Handler_Tests extends \WC_Unit_Test_Case {
WC_Download_Handler::download_file_force( 'http://127.0.0.1:1/missing-file', 'missing-file' );
}
+ /**
+ * @testdox readfile_chunked() should emit binary download bytes unchanged.
+ */
+ public function test_readfile_chunked_emits_binary_data_unchanged(): void {
+ $binary_content = "\x00\xFF\xFE<script>&\x80";
+ $temp_file = wp_tempnam( 'wc-download-handler-streaming' );
+
+ file_put_contents( $temp_file, $binary_content ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_file_put_contents -- Test fixture written to the temp directory.
+
+ $output = '';
+ ob_start(
+ function ( $chunk ) use ( &$output ) {
+ $output .= $chunk;
+ return '';
+ }
+ );
+
+ try {
+ $served = WC_Download_Handler::readfile_chunked( $temp_file, 0, strlen( $binary_content ) );
+ } finally {
+ ob_end_clean();
+ wp_delete_file( $temp_file );
+ }
+
+ $this->assertTrue( $served, 'A complete binary stream should be reported as served.' );
+ $this->assertSame( $binary_content, $output, 'Binary download bytes must not be escaped or otherwise transformed.' );
+ }
+
+ /**
+ * @testdox readfile_chunked() should stop and report failure when a stream read fails.
+ *
+ * @dataProvider provider_stream_lengths
+ *
+ * @param int $length Requested download length, where zero means until EOF.
+ */
+ public function test_readfile_chunked_reports_read_failure( int $length ): void {
+ $scheme = 'wc-failing-download';
+
+ FakeRemoteStreamWrapper::$fail_reads = true;
+ stream_wrapper_register( $scheme, FakeRemoteStreamWrapper::class );
+
+ ob_start();
+
+ try {
+ $served = WC_Download_Handler::readfile_chunked( $scheme . '://fixture', 0, $length );
+ } finally {
+ $output = ob_get_clean();
+ stream_wrapper_unregister( $scheme );
+ FakeRemoteStreamWrapper::$fail_reads = false;
+ }
+
+ $this->assertFalse( $served, 'A failed fread() call should make the download fail.' );
+ $this->assertSame( '', $output, 'A failed read should not append anything to the download response.' );
+ $this->assertTrue( FakeRemoteStreamWrapper::$closed, 'The failed stream should be closed immediately.' );
+ }
+
+ /**
+ * Download lengths for failed-stream coverage.
+ *
+ * @return array<string, array<int>>
+ */
+ public function provider_stream_lengths(): array {
+ return array(
+ 'requested range' => array( 4 ),
+ 'read until stream EOF' => array( 0 ),
+ );
+ }
+
/**
* @testdox The Content-Type fallback to the resolved filename should apply to remote files only.
*/