Skip to content
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Please also have a look at our

### Fixed

- Do not escape characters that do not need escaping in CSS string (#1444)
- Reject selector comprising only whitespace (#1433)
- Improve recovery parsing when a rogue `}` is encountered (#1425, #1426)
- Parse comment(s) immediately preceding a selector (#1421, #1424)
Expand Down
13 changes: 12 additions & 1 deletion src/Value/CSSString.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public function getString(): string
*/
public function render(OutputFormat $outputFormat): string
{
$string = \addslashes($this->string);
$string = $this->escape($this->string, $outputFormat->getStringQuotingType());
$string = \str_replace("\n", '\\A', $string);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should move this to the new escape method as a post-PR. Or maybe it can be done as part of this PR, as I've just spotted an issue with the changelog that also needs resolving.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer to have this as a post-PR cleanup as I still don't fully understand the \\A thing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

return $outputFormat->getStringQuotingType() . $string . $outputFormat->getStringQuotingType();
}
Expand All @@ -111,4 +111,15 @@ public function getArrayRepresentation(): array
'contents' => $this->string,
];
}

/**
* @param "'"|'"' $quote
*/
private function escape(string $string, string $quote): string
{
$charactersToEscape = '\\';
$charactersToEscape .= ($quote === '"' ? '"' : "'");

return \addcslashes($string, $charactersToEscape);
}
}
4 changes: 2 additions & 2 deletions tests/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,10 @@ public function unicodeParsing(): void
self::assertSame('"\\"\\""', $firstContentRuleAsString);
}
if ($selector === '.test-9') {
self::assertSame('"\\"\\\'"', $firstContentRuleAsString);
self::assertSame('"\\"\'"', $firstContentRuleAsString);
}
if ($selector === '.test-10') {
self::assertSame('"\\\'\\\\"', $firstContentRuleAsString);
self::assertSame('"\'\\\\"', $firstContentRuleAsString);
}
if ($selector === '.test-11') {
self::assertSame('"test"', $firstContentRuleAsString);
Expand Down
39 changes: 39 additions & 0 deletions tests/Unit/Value/CSSStringTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Sabberworm\CSS\Tests\Unit\Value;

use PHPUnit\Framework\TestCase;
use Sabberworm\CSS\OutputFormat;
use Sabberworm\CSS\Value\CSSString;
use Sabberworm\CSS\Value\PrimitiveValue;
use Sabberworm\CSS\Value\Value;
Expand Down Expand Up @@ -107,4 +108,42 @@ public function getArrayRepresentationIncludesContents(): void
self::assertArrayHasKey('contents', $result);
self::assertSame($contents, $result['contents']);
}

/**
* @test
*/
public function doesNotEscapeSingleQuotesThatDoNotNeedToBeEscaped(): void
{
$input = "data:image/svg+xml,%3Csvg width='24' height='24' viewBox='0 0 24 24' fill='none'" .
" xmlns='http://www.w3.org/2000/svg'%3E%3Cpath fill-rule='evenodd' clip-rule='evenodd'" .
" d='M14.3145 2 3V11.9987H17.5687L18 8.20761H14.3145L14.32 6.31012C14.32 5.32134 14.4207" .
' 4.79153 15.9426 4.79153H17.977V1H14.7223C10.8129 1 9.43687 2.83909 9.43687 ' .
" 5.93187V8.20804H7V11.9991H9.43687V23H14.3145Z' fill='black'/%3E%3C/svg%3E%0A";

$outputFormat = OutputFormat::createPretty();

self::assertSame("\"{$input}\"", (new CSSString($input))->render($outputFormat));

$outputFormat->setStringQuotingType("'");

$expected = str_replace("'", "\\'", $input);

self::assertSame("'{$expected}'", (new CSSString($input))->render($outputFormat));
}

/**
* @test
*/
public function doesNotEscapeDoubleQuotesThatDoNotNeedToBeEscaped(): void
{
$input = '"Hello World"';

$outputFormat = OutputFormat::createPretty();

self::assertSame('"\\"Hello World\\""', (new CSSString($input))->render($outputFormat));

$outputFormat->setStringQuotingType("'");

self::assertSame("'{$input}'", (new CSSString($input))->render($outputFormat));
}
}
2 changes: 1 addition & 1 deletion tests/fixtures/unicode.css
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
.test-6 { content: "\00A5" } /* Same as "¥" */
.test-7 { content: '\a' } /* Same as "\A" (Newline) */
.test-8 { content: "\"\22" } /* Same as "\"\"" */
.test-9 { content: "\"\27" } /* Same as ""\"\'"" */
.test-9 { content: "\"\27" } /* Same as "\"'" */
.test-10 { content: "\'\\" } /* Same as "'\" */
.test-11 { content: "\test" } /* Same as "test" */

Expand Down