geckon01/PhpSpreadsheet
1
0
Fork 0
mirror of https://github.com/PHPOffice/PhpSpreadsheet.git synced 2026-09-12 19:16:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Html Writer Handle Text Colors a Bit Better

Browse Source 
Researching a security advisory, the code for handling text colors in Html Writer when the cell is non-numeric and the color is hard-coded in the Number Format looked a bit suspicious. As it turns out, the code was *not* subject to a security exploit. However, the color for the Number Format was ignored. This PR corrects that omission.
This commit is contained in:
oleibman
2026-04-05 22:50:02 -07:00
parent 4a476071b3
commit fc6e7e6a66
7 changed files with 152 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/PhpSpreadsheet/Reader/BaseReader.php
+4 -5
View File
@@ -2,7 +2,6 @@
namespace PhpOffice\PhpSpreadsheet\Reader;
use Closure;
use PhpOffice\PhpSpreadsheet\Cell\IValueBinder;
use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException;
@@ -81,8 +80,8 @@ abstract class BaseReader implements IReader
protected ?IValueBinder $valueBinder = null;
/** @var null|Closure(string):bool function to return whether image path is okay */
protected ?Closure $isWhitelisted = null;
/** @var null|callable(string):bool function to return whether image path is okay */
protected $isWhitelisted;
public function __construct()
{
@@ -211,9 +210,9 @@ abstract class BaseReader implements IReader
* supplying a method which might return true
* can subject the caller to security exploits.
*
* @param Closure(string):bool $isWhitelisted
* @param callable(string):bool $isWhitelisted
*/
public function setIsWhitelisted(Closure $isWhitelisted): static
public function setIsWhitelisted(callable $isWhitelisted): static
{
$this->isWhitelisted = $isWhitelisted;
src/PhpSpreadsheet/Reader/Ods.php
+4 -5
View File
@@ -2,7 +2,6 @@
namespace PhpOffice\PhpSpreadsheet\Reader;
use Closure;
use Composer\Pcre\Preg;
use DateTime;
use DateTimeZone;
@@ -1511,11 +1510,11 @@ class Ods extends BaseReader
}
}
/** @var null|Closure(string, string):string */
private ?Closure $formatCallback = null;
/** @var null|callable(string, string):string format callback routine */
private $formatCallback;
/** @param Closure(string, string):string $formatCallback */
public function setFormatCallback(Closure $formatCallback): void
/** @param callable(string, string):string $formatCallback format callback routine */
public function setFormatCallback(callable $formatCallback): void
{
$this->formatCallback = $formatCallback;
}
src/PhpSpreadsheet/Style/NumberFormat.php
+2 -2
View File
@@ -479,12 +479,12 @@ class NumberFormat extends Supervisor
* @param null|bool|float|int|RichText|string $value Value to format
* @param string $format Format code: see = self::FORMAT_* for predefined values;
* or can be any valid MS Excel custom format string
* @param ?mixed[] $callBack Callback function for additional formatting of string
* @param null|callable(string, string): string $callBack Callback function for additional formatting of string
* @param bool $lessFloatPrecision If true, unstyled floats will be converted to a more human-friendly but less computationally accurate value
*
* @return string Formatted string
*/
public static function toFormattedString(mixed $value, string $format, ?array $callBack = null, bool $lessFloatPrecision = false): string
public static function toFormattedString(mixed $value, string $format, ?callable $callBack = null, bool $lessFloatPrecision = false): string
{
return NumberFormat\Formatter::toFormattedString($value, $format, $callBack, $lessFloatPrecision);
}
src/PhpSpreadsheet/Style/NumberFormat/Formatter.php
+10 -6
View File
@@ -48,7 +48,7 @@ class Formatter extends BaseFormatter
* @param float|int|numeric-string $value value to be formatted
* @param string[] $sections
*
* @return mixed[]
* @return array{string, string, mixed}
*/
private static function splitFormatForSectionSelection(array $sections, mixed $value): array
{
@@ -118,12 +118,12 @@ class Formatter extends BaseFormatter
* @param null|array<mixed>|bool|float|int|RichText|string $value Value to format
* @param string $format Format code: see = self::FORMAT_* for predefined values;
* or can be any valid MS Excel custom format string
* @param null|array<mixed>|callable $callBack Callback function for additional formatting of string
* @param null|callable(string, string): string $callBack Callback function for additional formatting of string
* @param bool $lessFloatPrecision If true, unstyled floats will be converted to a more human-friendly but less computationally accurate value
*
* @return string Formatted string
*/
public static function toFormattedString($value, string $format, null|array|callable $callBack = null, bool $lessFloatPrecision = false): string
public static function toFormattedString($value, string $format, ?callable $callBack = null, bool $lessFloatPrecision = false): string
{
while (is_array($value)) {
$value = array_shift($value);
@@ -148,7 +148,6 @@ class Formatter extends BaseFormatter
if (is_callable($callBack)) {
$temp = $callBack($temp, $formatx);
}
/** @var string $temp */
return str_replace(
['"', self::QUOTE_REPLACEMENT],
@@ -159,7 +158,13 @@ class Formatter extends BaseFormatter
// If we have a text value, return it "as is"
if (!is_numeric($value)) {
return StringHelper::convertToString($value, lessFloatPrecision: $lessFloatPrecision);
$temp = StringHelper::convertToString($value, lessFloatPrecision: $lessFloatPrecision);
if (is_callable($callBack)) {
$sections = preg_split(self::SECTION_SPLIT, $format) ?: [$format];
$temp = $callBack($temp, $sections[3] ?? $sections[0]);
}
return $temp;
}
// For 'General' format code, we just pass the value although this is not entirely the way Excel does it,
@@ -229,7 +234,6 @@ class Formatter extends BaseFormatter
if (is_callable($callBack)) {
$value = $callBack($value, $colors);
}
/** @var string $value */
return str_replace(chr(0x00), '.', $value);
}
tests/PhpSpreadsheetTests/Reader/Html/DataFormulaTest.php
+1 -1
View File
@@ -144,7 +144,7 @@ class DataFormulaTest extends AbstractFunctional
self::assertStringContainsString($expected, $content, 'bool non-formula');
$expected = '<td data-checkbox="1" class="column1 style1 f">=AND(TRUE,TRUE)</td>';
self::assertStringContainsString($expected, $content, 'bool formula');
$expected = '<td class="column2 style0 f">=&quot;A&quot;&amp;&quot;B&quot;&amp;&quot;C&quot;</td>';
$expected = '<td class="column2 style0 f">="A"&amp;"B"&amp;"C"</td>';
self::assertStringContainsString($expected, $content, 'string formula requiring escaped characters');
$spreadsheet->disconnectWorksheets();
tests/PhpSpreadsheetTests/Writer/Html/HtmlNumberFormatTest.php
+3 -1
View File
@@ -77,7 +77,9 @@ class HtmlNumberFormatTest extends Functional\AbstractFunctional
$tds = $rows?->item(3)?->getElementsByTagName('td');
self::assertCount(1, $tds);
$spans = $tds?->item(0)?->getElementsByTagName('span');
self::assertCount(0, $spans);
self::assertCount(1, $spans);
$style = $spans?->item(0)?->getAttribute('style');
self::assertSame(1, preg_match('/color:blue/', "$style"));
self::assertEquals('<br>', $tds?->item(0)?->textContent);
$rls = $this->writeAndReload($spreadsheet, 'Html');
tests/PhpSpreadsheetTests/Writer/Html/TextColorTest.php
+128
View File
@@ -0,0 +1,128 @@
<?php
declare(strict_types=1);
namespace PhpOffice\PhpSpreadsheetTests\Writer\Html;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Writer\Html;
use PHPUnit\Framework\TestCase;
class TextColorTest extends TestCase
{
public function testTextRotation(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->setCellValue('A1', 1);
$sheet->setCellValue('B1', -1);
$sheet->setCellValue('C1', 0);
$sheet->setCellValue('D1', 'text');
$format = '[Blue]General;[Red](0);[Green]General;[Magenta]General';
$sheet->getStyle('A1:D1')->getNumberFormat()
->setFormatCode($format);
$sheet->setCellValue('A2', 1);
$sheet->setCellValue('B2', -1);
$sheet->setCellValue('C2', 0);
$sheet->setCellValue('D2', 'text');
$format = '[Blue]General';
$sheet->getStyle('A2:D2')->getNumberFormat()
->setFormatCode($format);
$sheet->setCellValue('A3', 1);
$sheet->setCellValue('B3', -1);
$sheet->setCellValue('C3', 0);
$sheet->setCellValue('D3', 'text');
$format = '[Blue]General;[Red](0)';
$sheet->getStyle('A3:D3')->getNumberFormat()
->setFormatCode($format);
$sheet->setCellValue('A4', 1);
$sheet->setCellValue('B4', -1);
$sheet->setCellValue('C4', 0);
$sheet->setCellValue('D4', 'text');
$format = 'General;[Red](0)';
$sheet->getStyle('A4:D4')->getNumberFormat()
->setFormatCode($format);
$sheet->setCellValue('A5', 1);
$sheet->setCellValue('B5', -1);
$sheet->setCellValue('C5', 0);
$sheet->setCellValue('D5', 'text');
$writer = new Html($spreadsheet);
$html = $writer->generateHtmlAll();
$spreadsheet->disconnectWorksheets();
$html = preg_replace('/^\s+/m', '', $html) ?? $html;
$html = preg_replace('/\r?\n/m', '', $html) ?? $html;
self::assertStringContainsString(
'<tr class="row0">'
. '<td class="column0 style1 n"><span style="color:blue">1</span></td>'
. '<td class="column1 style1 n"><span style="color:red">(1)</span></td>'
. '<td class="column2 style1 n"><span style="color:green">0</span></td>'
. '<td class="column3 style1 s"><span style="color:magenta">text</span></td>'
. '</tr>',
$html
);
self::assertStringContainsString(
'<tr class="row1">'
. '<td class="column0 style2 n"><span style="color:blue">1</span></td>'
. '<td class="column1 style2 n"><span style="color:blue">-1</span></td>'
. '<td class="column2 style2 n"><span style="color:blue">0</span></td>'
. '<td class="column3 style2 s"><span style="color:blue">text</span></td>'
. '</tr>',
$html
);
self::assertStringContainsString(
'<tr class="row2">'
. '<td class="column0 style3 n"><span style="color:blue">1</span></td>'
. '<td class="column1 style3 n"><span style="color:red">(1)</span></td>'
. '<td class="column2 style3 n"><span style="color:blue">0</span></td>'
. '<td class="column3 style3 s"><span style="color:blue">text</span></td>'
. '</tr>',
$html
);
self::assertStringContainsString(
'<tr class="row3">'
. '<td class="column0 style4 n">1</td>'
. '<td class="column1 style4 n"><span style="color:red">(1)</span></td>'
. '<td class="column2 style4 n">0</td>'
. '<td class="column3 style4 s">text</td>'
. '</tr>',
$html
);
self::assertStringContainsString(
'<tr class="row4">'
. '<td class="column0 style0 n">1</td>'
. '<td class="column1 style0 n">-1</td>'
. '<td class="column2 style0 n">0</td>'
. '<td class="column3 style0 s">text</td>'
. '</tr>',
$html
);
}
public function testEscape(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$payload = '<img src=x onerror=alert(document.domain)>';
$formatCode = '[Green]General';
$sheet->setCellValue('A1', $payload);
$sheet->getStyle('A1')
->getNumberFormat()
->setFormatCode($formatCode);
$writer = new Html($spreadsheet);
$html = $writer->generateHtmlAll();
$spreadsheet->disconnectWorksheets();
self::assertStringContainsString(
'<td class="column0 style1 s"><span style="color:green">&lt;img src=x onerror=alert(document.domain)&gt;</span></td>',
$html
);
}
}