Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate Input to SetSelectedCells #2280

Merged
merged 3 commits into from
Sep 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -4825,31 +4825,6 @@ parameters:
count: 2
path: src/PhpSpreadsheet/Worksheet/Worksheet.php

-
message: "#^Parameter \\#3 \\$subject of function preg_replace expects array\\|string, string\\|null given\\.$#"
count: 3
path: src/PhpSpreadsheet/Worksheet/Worksheet.php

-
message: "#^Parameter \\#1 \\$coord of static method PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Coordinate\\:\\:coordinateIsRange\\(\\) expects string, string\\|null given\\.$#"
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php

-
message: "#^Parameter \\#1 \\$pRange of static method PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Coordinate\\:\\:splitRange\\(\\) expects string, string\\|null given\\.$#"
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\:\\:\\$activeCell \\(string\\) does not accept string\\|null\\.$#"
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\:\\:\\$selectedCells \\(string\\) does not accept string\\|null\\.$#"
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php

-
message: "#^Cannot call method getValue\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Cell\\\\Cell\\|null\\.$#"
count: 4
Expand Down
47 changes: 41 additions & 6 deletions src/PhpSpreadsheet/Worksheet/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -2373,6 +2373,38 @@ public function setSelectedCell($pCoordinate)
return $this->setSelectedCells($pCoordinate);
}

/**
* Sigh - Phpstan thinks, correctly, that preg_replace can return null.
* But Scrutinizer doesn't. Try to satisfy both.
*
* @param mixed $str
*/
private static function ensureString($str): string
{
return is_string($str) ? $str : '';
}

private static function pregReplace(string $pattern, string $replacement, string $subject): string
{
return self::ensureString(preg_replace($pattern, $replacement, $subject));
}

private function tryDefinedName(string $pCoordinate): string
{
// Uppercase coordinate
$pCoordinate = strtoupper($pCoordinate);
// Eliminate leading equal sign
$pCoordinate = self::pregReplace('/^=/', '', $pCoordinate);
$defined = $this->parent->getDefinedName($pCoordinate, $this);
if ($defined !== null) {
if ($defined->getWorksheet() === $this && !$defined->isFormula()) {
$pCoordinate = self::pregReplace('/^=/', '', $defined->getValue());
}
}

return $pCoordinate;
}

/**
* Select a range of cells.
*
Expand All @@ -2382,20 +2414,23 @@ public function setSelectedCell($pCoordinate)
*/
public function setSelectedCells($pCoordinate)
{
// Uppercase coordinate
$pCoordinate = strtoupper($pCoordinate);
$originalCoordinate = $pCoordinate;
$pCoordinate = $this->tryDefinedName($pCoordinate);

// Convert 'A' to 'A:A'
$pCoordinate = preg_replace('/^([A-Z]+)$/', '${1}:${1}', $pCoordinate);
$pCoordinate = self::pregReplace('/^([A-Z]+)$/', '${1}:${1}', $pCoordinate);

// Convert '1' to '1:1'
$pCoordinate = preg_replace('/^(\d+)$/', '${1}:${1}', $pCoordinate);
$pCoordinate = self::pregReplace('/^(\d+)$/', '${1}:${1}', $pCoordinate);

// Convert 'A:C' to 'A1:C1048576'
$pCoordinate = preg_replace('/^([A-Z]+):([A-Z]+)$/', '${1}1:${2}1048576', $pCoordinate);
$pCoordinate = self::pregReplace('/^([A-Z]+):([A-Z]+)$/', '${1}1:${2}1048576', $pCoordinate);

// Convert '1:3' to 'A1:XFD3'
$pCoordinate = preg_replace('/^(\d+):(\d+)$/', 'A${1}:XFD${2}', $pCoordinate);
$pCoordinate = self::pregReplace('/^(\d+):(\d+)$/', 'A${1}:XFD${2}', $pCoordinate);
if (preg_match('/^\\$?[A-Z]{1,3}\\$?\d{1,7}(:\\$?[A-Z]{1,3}\\$?\d{1,7})?$/', $pCoordinate) !== 1) {
throw new Exception("Invalid setSelectedCells $originalCoordinate $pCoordinate");
}

if (Coordinate::coordinateIsRange($pCoordinate)) {
[$first] = Coordinate::splitRange($pCoordinate);
Expand Down
120 changes: 120 additions & 0 deletions tests/PhpSpreadsheetTests/NamedRange2Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests;

use PhpOffice\PhpSpreadsheet\DefinedName;
use PhpOffice\PhpSpreadsheet\Exception as Except;
use PhpOffice\PhpSpreadsheet\NamedRange;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;

class NamedRange2Test extends TestCase
{
/** @var ?Spreadsheet */
private $spreadsheet;

protected function setUp(): void
{
$spreadsheet = $this->spreadsheet = new Spreadsheet();

$worksheet1 = $spreadsheet->getActiveSheet();
$worksheet1->setTitle('SheetOne');
$spreadsheet->addNamedRange(
new NamedRange('FirstRel', $worksheet1, '=A1')
);
$spreadsheet->addNamedRange(
new NamedRange('FirstAbs', $worksheet1, '=$B$1')
);
$spreadsheet->addNamedRange(
new NamedRange('FirstRelMult', $worksheet1, '=C1:D2')
);
$spreadsheet->addNamedRange(
new NamedRange('FirstAbsMult', $worksheet1, '$E$3:$F$4')
);

$worksheet2 = $spreadsheet->createSheet();
$worksheet2->setTitle('SheetTwo');
$spreadsheet->addNamedRange(
new NamedRange('SecondRel', $worksheet2, '=A1')
);
$spreadsheet->addNamedRange(
new NamedRange('SecondAbs', $worksheet2, '=$B$1')
);
$spreadsheet->addNamedRange(
new NamedRange('SecondRelMult', $worksheet2, '=C1:D2')
);
$spreadsheet->addNamedRange(
new NamedRange('SecondAbsMult', $worksheet2, '$E$3:$F$4')
);

$spreadsheet->addDefinedName(DefinedName::createInstance('FirstFormula', $worksheet1, '=TODAY()-1'));
$spreadsheet->addDefinedName(DefinedName::createInstance('SecondFormula', $worksheet2, '=TODAY()-2'));

$this->spreadsheet->setActiveSheetIndex(0);
}

protected function tearDown(): void
{
if ($this->spreadsheet !== null) {
$this->spreadsheet->disconnectWorksheets();
$this->spreadsheet = null;
}
}

private function getSpreadsheet(): Spreadsheet
{
if ($this->spreadsheet !== null) {
return $this->spreadsheet;
}
$this->spreadsheet = new Spreadsheet();

return $this->spreadsheet;
}

public function testNamedRangeSetStyle(): void
{
$spreadsheet = $this->getSpreadsheet();
$sheet = $spreadsheet->getSheet(0);
$sheet->getStyle('FirstRel')->getNumberFormat()->setFormatCode('yyyy-mm-dd');
self::assertSame('yyyy-mm-dd', $sheet->getStyle('A1')->getNumberFormat()->getFormatCode());
$sheet->getStyle('FirstAbs')->getFont()->setName('Georgia');
self::assertSame('Georgia', $sheet->getStyle('B1')->getFont()->getName());
$sheet->getStyle('FirstRelMult')->getFont()->setItalic(true);
self::assertTrue($sheet->getStyle('D2')->getFont()->getItalic());
$sheet->getStyle('FirstAbsMult')->getFill()->setFillType('gray125');
self::assertSame('gray125', $sheet->getStyle('F4')->getFill()->getFillType());
self::assertSame('none', $sheet->getStyle('A1')->getFill()->getFillType());
$sheet = $spreadsheet->getSheet(1);
$sheet->getStyle('SecondAbsMult')->getFill()->setFillType('lightDown');
self::assertSame('lightDown', $sheet->getStyle('E3')->getFill()->getFillType());
}

/**
* @dataProvider providerRangeOrFormula
*/
public function testNamedRangeSetStyleBad(string $name): void
{
$this->expectException(Except::class);
$spreadsheet = $this->getSpreadsheet();
$sheet = $spreadsheet->getSheet(0);
$sheet->getStyle($name)->getNumberFormat()->setFormatCode('yyyy-mm-dd');
self::assertSame('yyyy-mm-dd', $sheet->getStyle('A1')->getNumberFormat()->getFormatCode());
}

public function providerRangeOrFormula(): array
{
return [
'wrong sheet rel' => ['SecondRel'],
'wrong sheet abs' => ['SecondAbs'],
'wrong sheet relmult' => ['SecondRelMult'],
'wrong sheet absmult' => ['SecondAbsMult'],
'wrong sheet formula' => ['SecondFormula'],
'right sheet formula' => ['FirstFormula'],
'non-existent name' => ['NonExistentName'],
'this sheet name' => ['SheetOne!G7'],
'other sheet name' => ['SheetTwo!G7'],
'non-existent sheet name' => ['SheetAbc!G7'],
'unnamed formula' => ['=2+3'],
];
}
}