Make HTTPFileStreamer testable

This also improves ThumbnailEntryPointTest by allowing it
to assert which headers got sent.

Change-Id: I33911775bce1b3cc7a53a03c2be50d53a28fabd1

includes/Output/StreamFile.php

@@ -28,6 +28,7 @@ use HTTPFileStreamer;
 use InvalidArgumentException;
 use MediaWiki\MainConfigNames;
 use MediaWiki\MediaWikiServices;
+use RequestContext;
 use UploadBase;
 
 /**
@@ -64,13 +65,23 @@ class StreamFile {
 			$fname,
 			[
 				'obResetFunc' => 'wfResetOutputBuffers',
-				'streamMimeFunc' => [ __CLASS__, 'contentTypeFromPath' ]
+				'streamMimeFunc' => [ __CLASS__, 'contentTypeFromPath' ],
+				'headerFunc' => [ __CLASS__, 'setHeader' ],
 			]
 		);
 
 		return $streamer->stream( $headers, $sendErrors, $optHeaders, $flags );
 	}
 
+	/**
+	 * @param string $header
+	 *
+	 * @internal
+	 */
+	public static function setHeader( $header ) {
+		RequestContext::getMain()->getRequest()->response()->header( $header );
+	}
+
 	/**
 	 * Determine the file type of a file based on the path
 	 *

includes/libs/filebackend/FileBackend.php

@@ -123,9 +123,11 @@ abstract class FileBackend implements LoggerAwareInterface {
 	protected $profiler;
 
 	/** @var callable */
-	protected $obResetFunc;
-	/** @var callable|null */
-	protected $streamMimeFunc;
+	private $obResetFunc;
+	/** @var callable */
+	private $headerFunc;
+	/** @var array Option map for use with HTTPFileStreamer */
+	protected $streamerOptions;
 	/** @var callable|null */
 	protected $statusWrapper;
 
@@ -184,6 +186,7 @@ abstract class FileBackend implements LoggerAwareInterface {
 	 *      try to discover a usable temporary directory.
 	 *   - obResetFunc : alternative callback to clear the output buffer
 	 *   - streamMimeFunc : alternative method to determine the content type from the path
+	 *   - headerFunc : alternative callback for sending response headers
 	 *   - logger : Optional PSR logger object.
 	 *   - profiler : Optional callback that takes a section name argument and returns
 	 *      a ScopedCallback instance that ends the profile section in its destructor.
@@ -215,8 +218,14 @@ abstract class FileBackend implements LoggerAwareInterface {
 		$this->concurrency = isset( $config['concurrency'] )
 			? (int)$config['concurrency']
 			: 50;
-		$this->obResetFunc = $config['obResetFunc'] ?? [ $this, 'resetOutputBuffer' ];
-		$this->streamMimeFunc = $config['streamMimeFunc'] ?? null;
+		$this->obResetFunc = $config['obResetFunc']
+			?? [ self::class, 'resetOutputBufferTheDefaultWay' ];
+		$this->headerFunc = $config['headerFunc'] ?? 'header';
+		$this->streamerOptions = [
+			'obResetFunc' => $this->obResetFunc,
+			'headerFunc' => $this->headerFunc,
+			'streamMimeFunc' => $config['streamMimeFunc'] ?? null,
+		];
 
 		$this->profiler = $config['profiler'] ?? null;
 		if ( !is_callable( $this->profiler ) ) {
@@ -232,6 +241,15 @@ abstract class FileBackend implements LoggerAwareInterface {
 		}
 	}
 
+	protected function header( $header ) {
+		( $this->headerFunc )( $header );
+	}
+
+	protected function resetOutputBuffer() {
+		// By default, this ends up calling $this->defaultOutputBufferReset
+		( $this->obResetFunc )();
+	}
+
 	public function setLogger( LoggerInterface $logger ) {
 		$this->logger = $logger;
 	}
@@ -1702,9 +1720,11 @@ abstract class FileBackend implements LoggerAwareInterface {
 	}
 
 	/**
-	 * @codeCoverageIgnore Let's not reset output buffering during tests
+	 * Default behavior of resetOutputBuffer().
+	 * @codeCoverageIgnore
+	 * @internal
 	 */
-	protected function resetOutputBuffer() {
+	public static function resetOutputBufferTheDefaultWay() {
 		// XXX According to documentation, ob_get_status() always returns a non-empty array and this
 		// condition will always be true
 		while ( ob_get_status() ) {
@@ -1715,4 +1735,13 @@ abstract class FileBackend implements LoggerAwareInterface {
 			}
 		}
 	}
+
+	/**
+	 * Return options for use with HTTPFileStreamer.
+	 *
+	 * @internal
+	 */
+	public function getStreamerOptions(): array {
+		return $this->streamerOptions;
+	}
 }

includes/libs/filebackend/FileBackendStore.php

@@ -1065,10 +1065,7 @@ abstract class FileBackendStore extends FileBackend {
 		if ( $fsFile ) {
 			$streamer = new HTTPFileStreamer(
 				$fsFile->getPath(),
-				[
-					'obResetFunc' => $this->obResetFunc,
-					'streamMimeFunc' => $this->streamMimeFunc
-				]
+				$this->getStreamerOptions()
 			);
 			$res = $streamer->stream( $params['headers'], true, $params['options'], $flags );
 		} else {

includes/libs/filebackend/HTTPFileStreamer.php

@@ -35,8 +35,10 @@ class HTTPFileStreamer {
 	protected $obResetFunc;
 	/** @var callable */
 	protected $streamMimeFunc;
+	/** @var callable */
+	protected $headerFunc;
 
-	// Do not send any HTTP headers (e.g. body only)
+	// Do not send any HTTP headers (i.e. body only)
 	public const STREAM_HEADLESS = 1;
 	// Do not try to tear down any PHP output buffers
 	public const STREAM_ALLOW_OB = 2;
@@ -67,11 +69,18 @@ class HTTPFileStreamer {
 	 * @param array $params Options map, which includes:
 	 *   - obResetFunc : alternative callback to clear the output buffer
 	 *   - streamMimeFunc : alternative method to determine the content type from the path
+	 *   - headerFunc : alternative method for sending response headers
 	 */
 	public function __construct( $path, array $params = [] ) {
 		$this->path = $path;
-		$this->obResetFunc = $params['obResetFunc'] ?? [ __CLASS__, 'resetOutputBuffers' ];
-		$this->streamMimeFunc = $params['streamMimeFunc'] ?? [ __CLASS__, 'contentTypeFromPath' ];
+
+		$this->obResetFunc = $params['obResetFunc'] ??
+			[ __CLASS__, 'resetOutputBuffers' ];
+
+		$this->streamMimeFunc = $params['streamMimeFunc'] ??
+			[ __CLASS__, 'contentTypeFromPath' ];
+
+		$this->headerFunc = $params['headerFunc'] ?? 'header';
 	}
 
 	/**
@@ -88,19 +97,19 @@ class HTTPFileStreamer {
 	public function stream(
 		$headers = [], $sendErrors = true, $optHeaders = [], $flags = 0
 	) {
+		$headless = ( $flags & self::STREAM_HEADLESS );
+
 		// Don't stream it out as text/html if there was a PHP error
-		if ( ( ( $flags & self::STREAM_HEADLESS ) == 0 || $headers ) && headers_sent() ) {
+		if ( $headers && headers_sent() ) {
 			echo "Headers already sent, terminating.\n";
 			return false;
 		}
 
-		$headerFunc = ( $flags & self::STREAM_HEADLESS )
+		$headerFunc = $headless
 			? static function ( $header ) {
 				// no-op
 			}
-			: static function ( $header ) {
-				is_int( $header ) ? HttpStatus::header( $header ) : header( $header );
-			};
+			: [ $this, 'header' ];
 
 		AtEase::suppressWarnings();
 		$info = stat( $this->path );
@@ -284,4 +293,12 @@ class HTTPFileStreamer {
 
 		return 'unknown/unknown';
 	}
+
+	private function header( $header ) {
+		if ( is_int( $header ) ) {
+			$header = HttpStatus::getHeader( $header );
+		}
+
+		( $this->headerFunc )( $header );
+	}
 }

includes/libs/filebackend/SwiftFileBackend.php

@@ -1165,13 +1165,13 @@ class SwiftFileBackend extends FileBackendStore {
 		// Send the requested additional headers
 		if ( empty( $params['headless'] ) ) {
 			foreach ( $params['headers'] as $header ) {
-				header( $header );
+				$this->header( $header );
 			}
 		}
 
 		if ( empty( $params['allowOB'] ) ) {
 			// Cancel output buffering and gzipping if set
-			( $this->obResetFunc )();
+			$this->resetOutputBuffer();
 		}
 
 		$handle = fopen( 'php://output', 'wb' );

includes/media/MediaTransformOutput.php

@@ -24,7 +24,6 @@
 
 use MediaWiki\MainConfigNames;
 use MediaWiki\MediaWikiServices;
-use MediaWiki\Output\StreamFile;
 use MediaWiki\Status\Status;
 
 /**
@@ -218,14 +217,25 @@ abstract class MediaTransformOutput {
 		if ( !$this->path ) {
 			return Status::newFatal( 'backend-fail-stream', '<no path>' );
 		}
-		if ( FileBackend::isStoragePath( $this->path ) ) {
-			$be = $this->file->getRepo()->getBackend();
+
+		$repo = $this->file->getRepo();
+
+		if ( $repo && FileBackend::isStoragePath( $this->path ) ) {
 			return Status::wrap(
-				$be->streamFile( [ 'src' => $this->path, 'headers' => $headers ] ) );
+				$repo->getBackend()->streamFile(
+					[ 'src' => $this->path, 'headers' => $headers, ]
+				)
+			);
+		} else {
+			$streamer = new HTTPFileStreamer(
+				$this->getLocalCopyPath(),
+				$repo ? $repo->getBackend()->getStreamerOptions() : []
+			);
+
+			$success = $streamer->stream( $headers );
+			return $success ? Status::newGood()
+				: Status::newFatal( 'backend-fail-stream', $this->path );
 		}
-		// FS-file
-		$success = StreamFile::stream( $this->getLocalCopyPath(), $headers );
-		return $success ? Status::newGood() : Status::newFatal( 'backend-fail-stream', $this->path );
 	}
 
 	/**

tests/phpunit/includes/filerepo/ThumbnailEntryPointTest.php

@@ -25,6 +25,8 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 	// Counter for getting unique width values
 	private static $uniqueWidth = 20;
 
+	private ?MockEnvironment $environment = null;
+
 	/**
 	 * will be called only once per test class
 	 */
@@ -68,6 +70,10 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 		$this->installTestRepoGroup();
 	}
 
+	private function recordHeader( string $header ) {
+		$this->environment->getFauxResponse()->header( $header );
+	}
+
 	/**
 	 * @param FauxRequest|string|array|null $request
 	 *
@@ -87,7 +93,8 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 			$request->setRequestURL( '/w/img.php' );
 		}
 
-		return new MockEnvironment( $request );
+		$this->environment = new MockEnvironment( $request );
+		return $this->environment;
 	}
 
 	/**
@@ -161,8 +168,9 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 		$entryPoint->run();
 		$output = $entryPoint->getCapturedOutput();
 
-		// TODO: Assert Content-Type and Content-Length headers.
-		//       Needs FileStreamer to use WebResponse.
+		$response = $env->getFauxResponse();
+		$this->assertSame( 'image/png', $response->getHeader( 'Content-Type' ) );
+		$this->assertGreaterThan( 500, (int)$response->getHeader( 'Content-Length' ) );
 
 		$env->assertStatusCode( 200, $output );
 
@@ -257,9 +265,6 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 	}
 
 	public function testContentDisposition() {
-		// TODO...
-		$this->markTestSkipped( 'Needs refactoring of HTTPFileStreamer to capture headers' );
-
 		$env = $this->makeEnvironment(
 			[
 				'f' => 'Test.png',
@@ -275,7 +280,10 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 		$env->assertStatusCode( 200 );
 		$this->assertThumbnail( [ 'magic' => self::PNG_MAGIC, ], $output );
 
-		$env->assertHeaderValue( 'attachment', 'Content-Disposition' );
+		$env->assertHeaderValue(
+			'attachment;filename*=UTF-8\'\'Test.png',
+			'Content-Disposition'
+		);
 	}
 
 	public static function provideThumbNameParam() {
@@ -721,13 +729,24 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 	}
 
 	public static function provideRepoCouldNotStreamFile() {
-		// The width must match the one generated by testGenerateAndStreamThumbnail
-		// This error comes from FileBackend::doStreamFile.
-		yield 'existing thumbnail' => [ 12, 'xyzzy-error' ];
+		// TODO: figure out how to provoke an error in
+		// MediaTransformOutput::streamFileWithStatus.
+		// The below causes an error to be triggered too early.
+		// Since MediaTransformOutput uses StreamFile directly, we have to also
+		// sabotage transformations in the handler to return a ThumbnailImage
+		// with no path. This is unfortunately brittle to implementation changes.
 
-		// TODO: also test the case where we fail to stream a newly created
-		// thumbnail. In that case, the expected error comes from
-		// MediaTransformOutput::streamFileWithStatus, not FileBackend::doStreamFile.
+		// The width must match the one generated by testGenerateAndStreamThumbnail
+		/*yield 'existing thumbnail' => [
+			12,
+			'Could not stream the file',
+		];*/
+
+		// The specific error message may change as the code evolves
+		yield 'non-existing thumbnail' => [
+			self::$uniqueWidth++,
+			'No path supplied in thumbnail object',
+		];
 	}
 
 	/**
@@ -735,23 +754,6 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 	 * @depends testGenerateAndStreamThumbnail
 	 */
 	public function testRepoCouldNotStreamFile( int $width, string $expectedError ) {
-		// Sabotage streaming in file backend
-		$backend = $this->createFileBackend( [
-			'overrides' => [
-				'doStreamFile' => Status::newFatal( 'xyzzy-error' )
-			]
-		] );
-
-		$this->installTestRepoGroup(
-			[ 'backend' => $backend ]
-		);
-
-		// TODO: figure out how to provoke an error in
-		// MediaTransformOutput::streamFileWithStatus.
-		// The below causes an error to be triggered too early.
-		// Since MediaTransformOutput uses StreamFile directly, we have to also
-		// sabotage transformations in the handler to return a ThumbnailImage
-		// with no path. This is unfortunately brittle to implementation changes.
 		$handler = $this->getMockBuilder( BitmapHandler::class )
 			->onlyMethods( [ 'doTransform' ] )
 			->getMock();
@@ -783,8 +785,7 @@ class ThumbnailEntryPointTest extends MediaWikiIntegrationTestCase {
 			'Content-Type'
 		);
 
-		// TODO: check the log for the specific error.
-		$this->assertStringContainsString( 'Could not stream the file', $output );
+		$this->assertStringContainsString( $expectedError, $output );
 	}
 
 	/**

tests/phpunit/includes/media/MediaWikiMediaTestCase.php

@@ -30,7 +30,10 @@ abstract class MediaWikiMediaTestCase extends MediaWikiIntegrationTestCase {
 			'name' => 'localtesting',
 			'wikiId' => WikiMap::getCurrentWikiId(),
 			'containerPaths' => $containers,
-			'tmpDirectory' => $this->getNewTempDirectory()
+			'tmpDirectory' => $this->getNewTempDirectory(),
+			'obResetFunc' => static function () {
+				// do nothing, we need the output buffer in tests
+			}
 		] );
 		$this->repo = new FileRepo( $this->getRepoOptions() );
 	}

tests/phpunit/includes/media/ThumbnailImageTest.php

@@ -0,0 +1,115 @@
+<?php
+
+/**
+ * @group Media
+ * @group medium
+ *
+ * @covers \ThumbnailImage
+ * @covers \MediaTransformOutput
+ */
+class ThumbnailImageTest extends MediaWikiMediaTestCase {
+
+	private function newFile( $name = 'Test.jpg' ) {
+		return $this->dataFile( $name );
+	}
+
+	private function newThumbnail( $file = null, $url = null, $path = false, $parameters = [] ) {
+		$file ??= $this->newFile();
+		$path ??= 'thumb/a/ab/Test.jpg/123px-Test.jpg';
+		$url ??= "https://example.com/w/images/$path";
+
+		$parameters += [
+			'width' => 200,
+			'height' => 100,
+		];
+
+		return new ThumbnailImage( $file, $url, $path, $parameters );
+	}
+
+	public function testConstructor() {
+		$file = $this->newFile();
+		$path = 'thumb/a/ab/Test.jpg/123px-Test.jpg';
+		$url = "https://example.com/w/images/$path";
+
+		$parameters = [
+			'width' => 300,
+			'height' => 200,
+		];
+
+		$thumbnail = $this->newThumbnail(
+			$file,
+			$url,
+			$path,
+			$parameters
+		);
+
+		$this->assertSame( $file, $thumbnail->getFile() );
+		$this->assertSame( $url, $thumbnail->getUrl() );
+		$this->assertSame( $parameters['width'], $thumbnail->getWidth() );
+		$this->assertSame( $parameters['height'], $thumbnail->getHeight() );
+		$this->assertFalse( $thumbnail->isError() );
+	}
+
+	/**
+	 * Check that we can stream data from a file system path
+	 */
+	public function testStreamFileWithStatus_fsPath() {
+		$fsPath = $this->getFilePath() . 'test.jpg';
+		$data = file_get_contents( $fsPath );
+		$file = $this->newFile();
+
+		// NOTE: We need the FileRepo in $file for the streamer option,
+		// to prevent a real reset of the output buffer.
+		$thumbnail = $this->newThumbnail( $file, null, $fsPath );
+
+		ob_start();
+		$thumbnail->streamFileWithStatus();
+		$output = ob_get_clean();
+
+		$this->assertSame( $data, $output );
+	}
+
+	/**
+	 * Check that we can stream using the FileBackend
+	 */
+	public function testStreamFileWithStatus_thumbStoragePath() {
+		$this->backend = $this->createNoOpMock( FileBackend::class, [ 'streamFile' ] );
+
+		$this->backend->expects( $this->once() )
+			->method( 'streamFile' )
+			->wilLreturn( StatusValue::newGood() );
+
+		$this->repo = new FileRepo( $this->getRepoOptions() );
+
+		$file = $this->newFile( 'test.jpg' );
+		$thumbnail = $this->newThumbnail(
+			$file,
+			$file->getThumbUrl(),
+			$file->getThumbPath()
+		);
+
+		$thumbnail->streamFileWithStatus();
+
+		// no assertion needed, we just expect streamFile() to be called.
+	}
+
+	/**
+	 * Check that we don't explode if no file repo is known
+	 */
+	public function testStreamFileWithStatus_UnregisteredLocalFile() {
+		// Use a non-existing file, so streaming will fail.
+		// If streaming was successful, we'd generate real output, since
+		// without a file backend, we have no way to disable a full reset
+		// of output buffers.
+		$fsPath = $this->getFilePath() . 'this does not exist';
+
+		// No file repo or backend!
+		$file = new UnregisteredLocalFile( false, false, $fsPath );
+		$thumbnail = $this->newThumbnail( $file );
+
+		// Check that streaming fails gracefully
+		$status = $thumbnail->streamFileWithStatus();
+		$this->assertStatusError( 'backend-fail-stream', $status );
+	}
+
+}

tests/phpunit/mocks/filerepo/TestRepoTrait.php

@@ -132,7 +132,7 @@ trait TestRepoTrait {
 			"deletedHashLevels" => 0,
 			"updateCompatibleMetadata" => false,
 			"reserializeMetadata" => false,
-			"backend" => 'local-backend'
+			"backend" => 'local-backend',
 		];
 
 		if ( !$info['backend'] instanceof FileBackend ) {
@@ -153,6 +153,9 @@ trait TestRepoTrait {
 			'obResetFunc' => static function () {
 				ob_end_flush();
 			},
+			'headerFunc' => function ( string $header ) {
+				$this->recordHeader( $header );
+			},
 			'containerPaths' => [
 				"$name-public" => "$dir",
 				"$name-thumb" => "$dir/thumb",
@@ -212,4 +215,8 @@ trait TestRepoTrait {
 		return $file;
 	}
 
+	private function recordHeader( string $header ) {
+		// no-op
+	}
+
 }

tests/phpunit/unit/includes/filebackend/HTTPFileStreamerTest.php

@@ -2,10 +2,29 @@
 
 use PHPUnit\Framework\TestCase;
 
+/**
+ * @covers HTTPFileStreamer
+ */
 class HTTPFileStreamerTest extends TestCase {
 
+	private const FILE = MW_INSTALL_PATH . '/tests/phpunit/data/media/test.jpg';
+
+	private $obLevel = null;
+
+	protected function setUp(): void {
+		$this->obLevel = ob_get_level();
+		ob_start();
+		parent::setUp();
+	}
+
+	protected function tearDown(): void {
+		while ( ob_get_level() > $this->obLevel ) {
+			ob_end_clean();
+		}
+		parent::tearDown();
+	}
+
 	/**
-	 * @covers \HTTPFileStreamer::preprocessHeaders
 	 * @dataProvider providePreprocessHeaders
 	 */
 	public function testPreprocessHeaders( array $input, array $expectedRaw, array $expectedOpt ) {
@@ -33,4 +52,286 @@ class HTTPFileStreamerTest extends TestCase {
 		];
 	}
 
+	private function makeStreamerParams( &$actual ) {
+		$actual = [
+			'reset' => 0,
+			'file' => null,
+			'range' => null,
+			'headers' => [],
+			'status' => 200,
+		];
+
+		return [
+			'obResetFunc' => static function () use ( &$actual ) {
+				$actual['reset']++;
+			},
+			'streamMimeFunc' => static function () {
+				return 'test/test';
+			},
+			'headerFunc' => static function ( $header ) use ( &$actual ) {
+				if ( preg_match( '/^HTTP.*? (\d+)/', $header, $m ) ) {
+					$actual['status'] = (int)$m[1];
+				}
+
+				$actual['headers'][] = $header;
+			},
+		];
+	}
+
+	public static function provideStream() {
+		$mtime = filemtime( self::FILE );
+		$size = filesize( self::FILE );
+
+		$modified = MWTimestamp::convert( TS_RFC2822, $mtime );
+
+		yield 'simple stream' => [
+			[],
+			[],
+			[
+				"Last-Modified: $modified",
+				'Content-type: test/test',
+				"Content-Length: $size"
+			]
+		];
+
+		yield 'extra header' => [
+			[ 'Extra: yes' ],
+			[],
+			[
+				'Extra: yes',
+				"Last-Modified: $modified",
+				'Content-type: test/test',
+				"Content-Length: $size"
+			]
+		];
+
+		yield 'modified' => [
+			[],
+			[
+				'if-modified-since' => MWTimestamp::convert( TS_RFC2822, $mtime - 1 )
+			],
+			[
+				"Last-Modified: $modified",
+				'Content-type: test/test',
+				"Content-Length: $size"
+			]
+		];
+
+		yield 'not modified' => [
+			[],
+			[
+				'if-modified-since' => MWTimestamp::convert( TS_RFC2822, $mtime + 1 )
+			],
+			[
+				'HTTP/1.1 304 Not Modified'
+			]
+		];
+	}
+
+	/**
+	 * @dataProvider provideStream
+	 */
+	public function testStream( $extraHeaders, $reqHeaders, $expectedHeaders ) {
+		$params = $this->makeStreamerParams( $actual );
+
+		$streamer = new HTTPFileStreamer( self::FILE, $params );
+		$ok = $streamer->stream( $extraHeaders, true, $reqHeaders );
+
+		$this->assertTrue( $ok );
+
+		if ( !isset( $actual['status'] ) ) {
+			$this->assertSame( self::FILE, $actual['file'] );
+		}
+		$this->assertSame( 1, $actual['reset'] );
+
+		foreach ( $expectedHeaders as $exp ) {
+			$this->assertContains( $exp, $actual['headers'] );
+		}
+	}
+
+	public static function provideStream_range() {
+		$filesize = filesize( self::FILE );
+		$length = $filesize;
+		$start = 0;
+		$end = $length - 1;
+
+		yield 'all' => [
+			'bytes=-',
+			206,
+			[
+				"Content-Length: $length",
+				"Content-Range: bytes $start-$end/$filesize",
+			],
+			[ $start, $end, $length ]
+		];
+
+		$length = 100;
+		$start = 0;
+		$end = $length - 1;
+		yield 'prefix' => [
+			'bytes=0-99',
+			206,
+			[
+				"Content-Length: $length",
+				"Content-Range: bytes $start-$end/$filesize",
+			],
+			[ $start, $end, $length ]
+		];
+
+		$length = 100;
+		$start = 100;
+		$end = $start + $length - 1;
+		yield 'middle' => [
+			'bytes=100-199',
+			206,
+			[
+				"Content-Length: $length",
+				"Content-Range: bytes $start-$end/$filesize",
+			],
+			[ $start, $end, $length ]
+		];
+
+		$length = 100;
+		$start = $filesize - $length;
+		$end = $filesize - 1;
+		yield 'suffix' => [
+			'bytes=-100',
+			206,
+			[
+				"Content-Length: $length",
+				"Content-Range: bytes $start-$end/$filesize",
+			],
+			[ $start, $end, $length ]
+		];
+
+		$length = $filesize - 100;
+		$start = 100;
+		$end = $start + $length - 1;
+		yield 'remaining' => [
+			'bytes=100-',
+			206,
+			[
+				"Content-Length: $length",
+				"Content-Range: bytes $start-$end/$filesize",
+			],
+			[ $start, $end, $length ]
+		];
+
+		yield 'impossible' => [
+			'bytes=1000-2000',
+			416,
+			[ 'Cache-Control: no-cache' ],
+			null
+		];
+
+		yield 'unrecognized' => [
+			'foo=1000-2000',
+			200,
+			[ "Content-Length: $filesize" ],
+			null
+		];
+	}
+
+	/**
+	 * Check parsing of the Range header and corresponding response headers.
+	 *
+	 * @dataProvider provideStream_range
+	 */
+	public function testStream_range(
+		$range,
+		$expectedStatus,
+		$expectedHeaders,
+		$expectedRange
+	) {
+		$params = $this->makeStreamerParams( $actual );
+
+		$streamer = new HTTPFileStreamer( self::FILE, $params );
+		$streamer->stream( [], true, [ 'range' => $range ] );
+
+		$this->assertSame( $expectedStatus, $actual['status'] );
+
+		foreach ( $expectedHeaders as $exp ) {
+			$this->assertContains( $exp, $actual['headers'] );
+		}
+
+		if ( $expectedStatus < 300 ) {
+			[ $start, , $length ] = $expectedRange;
+			$this->assertBufferContainsFile( $start, $length );
+		}
+	}
+
+	/**
+	 * Check we ar ereaching the correct chunks from the file
+	 */
+	public function testStream_chunks() {
+		$data = file_get_contents( self::FILE );
+
+		$params = $this->makeStreamerParams( $actual );
+
+		$streamer = new HTTPFileStreamer( self::FILE, $params );
+
+		// grab a chunk from the middle
+		ob_start();
+		$streamer->stream( [], true, [ 'range' => 'bytes=100-199' ] );
+		$chunk1 = ob_get_clean();
+
+		// get the start
+		ob_start();
+		$streamer->stream( [], true, [ 'range' => 'bytes=0-99' ] );
+		$chunk2 = ob_get_clean();
+
+		// fetch the rest
+		ob_start();
+		$streamer->stream( [], true, [ 'range' => 'bytes=200-' ] );
+		$chunk3 = ob_get_clean();
+
+		$this->assertSame( $data, $chunk2 . $chunk1 . $chunk3 );
+	}
+
+	public function testStream_404() {
+		$params = $this->makeStreamerParams( $actual );
+
+		$streamer = new HTTPFileStreamer( 'Xyzzy.jpg', $params );
+
+		ob_start();
+		$ok = $streamer->stream();
+		$data = ob_get_clean();
+
+		$this->assertFalse( $ok );
+		$this->assertStringContainsString( '<h1>File not found</h1>', $data );
+	}
+
+	public function testStream_allowOB() {
+		$params = $this->makeStreamerParams( $actual );
+
+		$streamer = new HTTPFileStreamer( self::FILE, $params );
+		$streamer->stream( [], true, [], HTTPFileStreamer::STREAM_ALLOW_OB );
+
+		// Expect no buffer reset, even though the file was sent
+		$this->assertSame( 0, $actual['reset'] );
+		$this->assertBufferContainsFile();
+	}
+
+	public function testStream_headless() {
+		$params = $this->makeStreamerParams( $actual );
+
+		$streamer = new HTTPFileStreamer( self::FILE, $params );
+		$streamer->stream( [ 'Extra: yes' ], true, [], HTTPFileStreamer::STREAM_HEADLESS );
+
+		// Expect no headers, not even "extra"
+		$this->assertSame( [], $actual['headers'] );
+		$this->assertBufferContainsFile();
+	}
+
+	private function assertBufferContainsFile( ?int $offset = null, ?int $length = null ) {
+		$actual = ob_get_clean();
+		$expected = file_get_contents( self::FILE );
+
+		if ( $offset !== null || $length !== null ) {
+			$expected = substr( $expected, $offset ?? 0, $length );
+		}
+
+		$this->assertSame( $expected, $actual );
+	}
+
 }

tests/phpunit/unit/includes/libs/filebackend/FileBackendTest.php

@@ -245,21 +245,21 @@ class FileBackendTest extends MediaWikiUnitTestCase {
 			'concurrency null' => [ 'concurrency', 50, [ 'concurrency' => null ] ],
 			'concurrency cast to int' => [ 'concurrency', 51, [ 'concurrency' => '51x' ] ],
 
-			'obResetFunc default value' => [ 'obResetFunc',
-				// I'd've thought the return type should be 'callable', but apparently protected
-				// methods aren't callable.
-				static function ( FileBackend $backend ): array {
-					return [ $backend, 'resetOutputBuffer' ];
-				} ],
-			'obResetFunc null' => [ 'obResetFunc',
-				static function ( FileBackend $backend ): array {
-					return [ $backend, 'resetOutputBuffer' ];
-				} ],
-			'obResetFunc set' => [ 'obResetFunc', 'wfSomeImaginaryFunction',
-				[ 'obResetFunc' => 'wfSomeImaginaryFunction' ] ],
+			'obResetFunc default value' =>
+				[ 'obResetFunc', [ FileBackend::class, 'resetOutputBufferTheDefaultWay' ] ],
+			'obResetFunc null' => [
+				'obResetFunc',
+				[ FileBackend::class, 'resetOutputBufferTheDefaultWay' ],
+				[ 'obResetFunc' => null ]
+			],
+			'obResetFunc set' => [
+				'obResetFunc',
+				'wfSomeImaginaryFunction',
+				[ 'obResetFunc' => 'wfSomeImaginaryFunction' ]
+			],
 
-			'streamMimeFunc default value' => [ 'streamMimeFunc', null ],
-			'streamMimeFunc set' => [ 'streamMimeFunc', 'smf', [ 'streamMimeFunc' => 'smf' ] ],
+			'headerFunc default value' => [ 'headerFunc', 'header' ],
+			'headerFunc set' => [ 'headerFunc', 'myHeaderFunc', [ 'headerFunc' => 'myHeaderFunc' ] ],
 
 			'profiler default value' => [ 'profiler', null ],
 			'profiler not callable' => [ 'profiler', null, [ 'profiler' => '!' ] ],