Skip to content

Commit 07c60ba

Browse files
authored
Clear Output Buffering Only When It's Been Started (#3741)
Fix #3739. User is seeing an intermittent notice when running samples using a web browser. The notice is comming from ob_clean, complaining that there is no buffer to delete. I frankly do not understand what the ob_clean is supposed to be paired with, nor why I cannot duplicate this result. Possibly, a better solution would be to eliminate the ob_clean; but using ob_get_length beforehand to see if there is anything to clean up seems safer, and I can't think of a downside. The code in question is never executed when a sample is run from the command line. Consequently, no formal unit test is possible. The user reporting the problem was asked to test the change, and confirmed that the problem went away; no new problem arose on my system.
1 parent 23b95e2 commit 07c60ba

File tree

3 files changed

+23
-6
lines changed

3 files changed

+23
-6
lines changed

samples/bootstrap/css/bootstrap.min.css

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/PhpSpreadsheet/Helper/Downloader.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44

55
use PhpOffice\PhpSpreadsheet\Exception;
66

7+
/**
8+
* Assist downloading files when samples are run in browser.
9+
* Never run as part of unit tests, which are command line.
10+
*
11+
* @codeCoverageIgnore
12+
*/
713
class Downloader
814
{
915
protected string $filepath;
@@ -49,7 +55,13 @@ public function download(): void
4955

5056
public function headers(): void
5157
{
52-
ob_clean();
58+
// I cannot tell what this ob_clean is paired with.
59+
// I have never seen a problem with it, but someone has - issue 3739.
60+
// Perhaps it should be removed altogether,
61+
// but making it conditional seems harmless, and safer.
62+
if ((int) ob_get_length() > 0) {
63+
ob_clean();
64+
}
5365

5466
$this->contentType();
5567
$this->contentDisposition();

src/PhpSpreadsheet/Helper/Sample.php

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ public function write(Spreadsheet $spreadsheet, $filename, array $writers = ['Xl
131131
$writer->save($path);
132132
$this->logWrite($writer, $path, /** @scrutinizer ignore-type */ $callStartTime);
133133
if ($this->isCli() === false) {
134+
// @codeCoverageIgnoreStart
134135
echo '<a href="/download.php?type=' . pathinfo($path, PATHINFO_EXTENSION) . '&name=' . basename($path) . '">Download ' . basename($path) . '</a><br />';
136+
// @codeCoverageIgnoreEnd
135137
}
136138
}
137139

@@ -190,12 +192,17 @@ public function log(string $message): void
190192
echo($this->isCli() ? date('H:i:s ') : '') . $message . $eol;
191193
}
192194

195+
/**
196+
* Render chart as part of running chart samples in browser.
197+
* Charts are not rendered in unit tests, which are command line.
198+
*
199+
* @codeCoverageIgnore
200+
*/
193201
public function renderChart(Chart $chart, string $fileName): void
194202
{
195203
if ($this->isCli() === true) {
196204
return;
197205
}
198-
199206
Settings::setChartRenderer(MtJpGraphRenderer::class);
200207

201208
$fileName = $this->getFilename($fileName, 'png');
@@ -261,9 +268,8 @@ public function logWrite(IWriter $writer, $path, $callStartTime): void
261268
$reflection = new ReflectionClass($writer);
262269
$format = $reflection->getShortName();
263270

264-
$message = ($this->isCli() === true)
265-
? "Write {$format} format to {$path} in " . sprintf('%.4f', $callTime) . ' seconds'
266-
: "Write {$format} format to <code>{$path}</code> in " . sprintf('%.4f', $callTime) . ' seconds';
271+
$codePath = $this->isCli() ? $path : "<code>$path</code>";
272+
$message = "Write {$format} format to {$codePath} in " . sprintf('%.4f', $callTime) . ' seconds';
267273

268274
$this->log($message);
269275
}

0 commit comments

Comments
 (0)