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.

Sign up for GitHub

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

fix unescaped slash error in preg match #3513

Closed
wants to merge 5 commits into from
Closed

fix unescaped slash error in preg match #3513

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0917ed5
escape slash character because preg_replace function fails
SaidkhojaIftikhor Apr 8, 2023
5e17c32
update changelog.md
SaidkhojaIftikhor Apr 8, 2023
bc62e26
Merge branch 'master' into iftikhor/fix-preg-match-error
SaidkhojaIftikhor Apr 10, 2023
1766df7
Merge branch 'master' into iftikhor/fix-preg-match-error
SaidkhojaIftikhor Apr 11, 2023
467a076
remove unnecessary escaped slashes
SaidkhojaIftikhor Apr 11, 2023
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
1,584 changes: 796 additions & 788 deletions CHANGELOG.md
View file
Open in desktop
SaidkhojaIftikhor marked this conversation as resolved.
Show resolved Hide resolved

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions src/PhpSpreadsheet/Calculation/Engine/FormattedNumber.php
View file
Open in desktop
SaidkhojaIftikhor marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ public static function convertToNumberIfFormatted(string &$operand): bool
*/
public static function convertToNumberIfNumeric(string &$operand): bool
{
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator());
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator(), '/');
$value = preg_replace(['/(\d)' . $thousandsSeparator . '(\d)/u', '/([+-])\s+(\d)/u'], ['$1$2', '$1$2'], trim($operand));
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator());
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator(), '/');
$value = preg_replace(['/(\d)' . $decimalSeparator . '(\d)/u', '/([+-])\s+(\d)/u'], ['$1.$2', '$1$2'], $value ?? '');

if (is_numeric($value)) {
Expand Down Expand Up @@ -90,9 +90,9 @@ public static function convertToNumberIfFraction(string &$operand): bool
*/
public static function convertToNumberIfPercent(string &$operand): bool
{
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator());
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator(), '/');
$value = preg_replace('/(\d)' . $thousandsSeparator . '(\d)/u', '$1$2', trim($operand));
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator());
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator(), '/');
$value = preg_replace(['/(\d)' . $decimalSeparator . '(\d)/u', '/([+-])\s+(\d)/u'], ['$1.$2', '$1$2'], $value ?? '');

$match = [];
Expand Down Expand Up @@ -134,8 +134,8 @@ public static function convertToNumberIfCurrency(string &$operand): bool

public static function currencyMatcherRegexp(): string
{
$currencyCodes = sprintf(self::CURRENCY_CONVERSION_LIST, preg_quote(StringHelper::getCurrencyCode()));
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator());
$currencyCodes = sprintf(self::CURRENCY_CONVERSION_LIST, preg_quote(StringHelper::getCurrencyCode(), '/'));
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator(), '/');

return '~^(?:(?: *(?<PrefixedSign>[-+])? *(?<PrefixedCurrency>[' . $currencyCodes . ']) *(?<PrefixedSign2>[-+])? *(?<PrefixedValue>[0-9]+[' . $decimalSeparator . ']?[0-9*]*(?:E[-+]?[0-9]*)?) *)|(?: *(?<PostfixedSign>[-+])? *(?<PostfixedValue>[0-9]+' . $decimalSeparator . '?[0-9]*(?:E[-+]?[0-9]*)?) *(?<PostfixedCurrency>[' . $currencyCodes . ']) *))$~ui';
}
Expand Down
6 changes: 3 additions & 3 deletions src/PhpSpreadsheet/Calculation/Engine/Operands/StructuredReference.php
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crux of the PR; and I've verified that the change is necessary her.
Can you provide a unit test that will demonstrate the issue, and prove that this fix works?

Copy link
Author

@SaidkhojaIftikhor SaidkhojaIftikhor Apr 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will try. I am new to this repository and I don't understand a lot of things, that's why I added delimiter everywhere in code.

How can I learn the codebase ? I tried to read the code but it's kinda hard

Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ private function adjustRowReference(string $columnName, string $reference, Cell
{
if ($columnName !== '') {
$cellReference = $columnId . $cell->getRow();
$pattern1 = '/\[' . preg_quote($columnName) . '\]/miu';
$pattern2 = '/@' . preg_quote($columnName) . '/miu';
$pattern1 = '/\[' . preg_quote($columnName, '/') . '\]/miu';
$pattern2 = '/@' . preg_quote($columnName, '/') . '/miu';
if (preg_match($pattern1, $reference) === 1) {
$reference = preg_replace($pattern1, $cellReference, $reference);
} elseif (preg_match($pattern2, $reference) === 1) {
Expand Down Expand Up @@ -328,7 +328,7 @@ private function getColumnsForColumnReference(string $reference, int $startRow,
$cellFrom = "{$columnId}{$startRow}";
$cellTo = "{$columnId}{$endRow}";
$cellReference = ($cellFrom === $cellTo) ? $cellFrom : "{$cellFrom}:{$cellTo}";
$pattern = '/\[' . preg_quote($columnName) . '\]/mui';
$pattern = '/\[' . preg_quote($columnName, '/') . '\]/mui';
if (preg_match($pattern, $reference) === 1) {
$columnsSelected = true;
$reference = preg_replace($pattern, $cellReference, $reference);
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Calculation/TextData/Extract.php
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with the Structured Reference changes, can you provide unit tests to verify that this is a problem, and that the solution does resolve that problem for all these instances where you've made the change?

Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ private static function buildDelimiter($delimiter): string
$delimiter = Functions::flattenArray($delimiter);
$quotedDelimiters = array_map(
function ($delimiter) {
return preg_quote($delimiter ?? '');
return preg_quote($delimiter ?? '', '/');
},
$delimiter
);
Expand All @@ -270,7 +270,7 @@ function ($delimiter) {
return '(' . $delimiters . ')';
}

return '(' . preg_quote($delimiter ?? '') . ')';
return '(' . preg_quote($delimiter ?? '', '/') . ')';
}

private static function matchFlags(int $matchMode): string
Expand Down
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Calculation/TextData/Format.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ public static function NUMBERVALUE($value = '', $decimalSeparator = null, $group
}

if (!is_numeric($value)) {
$decimalPositions = preg_match_all('/' . preg_quote($decimalSeparator) . '/', $value, $matches, PREG_OFFSET_CAPTURE);
$decimalPositions = preg_match_all('/' . preg_quote($decimalSeparator, '/') . '/', $value, $matches, PREG_OFFSET_CAPTURE);
if ($decimalPositions > 1) {
return ExcelError::VALUE();
}
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Calculation/TextData/Text.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ private static function buildDelimiter($delimiter): string
if (is_array($delimiter) && count($valueSet) > 1) {
$quotedDelimiters = array_map(
function ($delimiter) {
return preg_quote($delimiter ?? '');
return preg_quote($delimiter ?? '', '/');
},
$valueSet
);
Expand All @@ -202,7 +202,7 @@ function ($delimiter) {
return '(' . $delimiters . ')';
}

return '(' . preg_quote(/** @scrutinizer ignore-type */ Functions::flattenSingleValue($delimiter)) . ')';
return '(' . preg_quote(/** @scrutinizer ignore-type */ Functions::flattenSingleValue($delimiter), '/') . ')';
}

private static function matchFlags(bool $matchMode): string
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Cell/AdvancedValueBinder.php
View file
Open in desktop
SaidkhojaIftikhor marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ public function bindValue(Cell $cell, $value = null)
return $this->setImproperFraction($matches, $cell);
}

$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator());
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator());
$decimalSeparator = preg_quote(StringHelper::getDecimalSeparator(), '/');
$thousandsSeparator = preg_quote(StringHelper::getThousandsSeparator(), '/');

// Check for percentage
if (preg_match('/^\-?\d*' . $decimalSeparator . '?\d*\s?\%$/', preg_replace('/(\d)' . $thousandsSeparator . '(\d)/u', '$1$2', $value))) {
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Worksheet/Table.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private function updateStructuredReferences(string $name): void

private function updateStructuredReferencesInCells(Worksheet $worksheet, string $newName): void
{
$pattern = '/' . preg_quote($this->name) . '\[/mui';
$pattern = '/' . preg_quote($this->name, '/') . '\[/mui';

foreach ($worksheet->getCoordinates(false) as $coordinate) {
$cell = $worksheet->getCell($coordinate);
Expand All @@ -196,7 +196,7 @@ private function updateStructuredReferencesInCells(Worksheet $worksheet, string

private function updateStructuredReferencesInNamedFormulae(Spreadsheet $spreadsheet, string $newName): void
{
$pattern = '/' . preg_quote($this->name) . '\[/mui';
$pattern = '/' . preg_quote($this->name, '/') . '\[/mui';

foreach ($spreadsheet->getNamedFormulae() as $namedFormula) {
$formula = $namedFormula->getValue();
Expand Down
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Worksheet/Table/Column.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public static function updateStructuredReferences(?Worksheet $workSheet, ?string

private static function updateStructuredReferencesInCells(Worksheet $worksheet, string $oldTitle, string $newTitle): void
{
$pattern = '/\[(@?)' . preg_quote($oldTitle) . '\]/mui';
$pattern = '/\[(@?)' . preg_quote($oldTitle, '/') . '\]/mui';

foreach ($worksheet->getCoordinates(false) as $coordinate) {
$cell = $worksheet->getCell($coordinate);
Expand All @@ -241,7 +241,7 @@ private static function updateStructuredReferencesInCells(Worksheet $worksheet,

private static function updateStructuredReferencesInNamedFormulae(Spreadsheet $spreadsheet, string $oldTitle, string $newTitle): void
{
$pattern = '/\[(@?)' . preg_quote($oldTitle) . '\]/mui';
$pattern = '/\[(@?)' . preg_quote($oldTitle, '/') . '\]/mui';

foreach ($spreadsheet->getNamedFormulae() as $namedFormula) {
$formula = $namedFormula->getValue();
Expand Down