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

[7.x] Escape attributes automatically in some situations #31945

Merged
merged 7 commits into from Mar 13, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/Illuminate/View/Compilers/ComponentTagCompiler.php
Expand Up @@ -23,6 +23,13 @@ class ComponentTagCompiler
*/
protected $aliases = [];

/**
* The "bind:" attributes that have been compiled for the current component.
*
* @var array
*/
protected $boundAttributes = [];

/**
* Create new component tag compiler.
*
Expand Down Expand Up @@ -96,6 +103,8 @@ protected function compileOpeningTags(string $value)
/x";

return preg_replace_callback($pattern, function (array $matches) {
$this->boundAttributes = [];

$attributes = $this->getAttributesFromAttributeString($matches['attributes']);

return $this->componentString($matches[1], $attributes);
Expand Down Expand Up @@ -136,6 +145,8 @@ protected function compileSelfClosingTags(string $value)
/x";

return preg_replace_callback($pattern, function (array $matches) {
$this->boundAttributes = [];

$attributes = $this->getAttributesFromAttributeString($matches['attributes']);

return $this->componentString($matches[1], $attributes)."\n@endcomponentClass";
Expand Down Expand Up @@ -165,15 +176,15 @@ protected function componentString(string $component, array $attributes)
if (! class_exists($class)) {
$parameters = [
'view' => "'$class'",
'data' => '['.$this->attributesToString($data->all()).']',
'data' => '['.$this->attributesToString($data->all(), $escapeBound = false).']',
Copy link
Contributor

Choose a reason for hiding this comment

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

This is currently not covered by any test (Changing to $escapeBound = true doesn't cause any tests to fail).

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is an attempt to cover this in testClasslessComponents(): perifer@46176c6

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks

];

$class = AnonymousComponent::class;
} else {
$parameters = $data->all();
}

return " @component('{$class}', [".$this->attributesToString($parameters).'])
return " @component('{$class}', [".$this->attributesToString($parameters, $escapeBound = false).'])
<?php $component->withAttributes(['.$this->attributesToString($attributes->all()).']); ?>';
}

Expand Down Expand Up @@ -319,6 +330,8 @@ protected function getAttributesFromAttributeString(string $attributeString)

if (Str::startsWith($attribute, 'bind:')) {
$attribute = Str::after($attribute, 'bind:');

$this->boundAttributes[$attribute] = true;
} else {
$value = "'".str_replace("'", "\\'", $value)."'";
}
Expand Down Expand Up @@ -349,13 +362,16 @@ protected function parseBindAttributes(string $attributeString)
* Convert an array of attributes to a string.
*
* @param array $attributes
* @param bool $escapeBound
* @return string
*/
protected function attributesToString(array $attributes)
protected function attributesToString(array $attributes, $escapeBound = true)
{
return collect($attributes)
->map(function (string $value, string $attribute) {
return "'{$attribute}' => {$value}";
->map(function (string $value, string $attribute) use ($escapeBound) {
return $escapeBound && isset($this->boundAttributes[$attribute]) && $value !== 'true' && ! is_numeric($value)
? "'{$attribute}' => \Illuminate\View\Compilers\BladeCompiler::sanitizeComponentAttribute({$value})"
: "'{$attribute}' => {$value}";
})
->implode(',');
}
Expand Down
14 changes: 14 additions & 0 deletions src/Illuminate/View/Compilers/Concerns/CompilesComponents.php
Expand Up @@ -154,4 +154,18 @@ protected function compileProps($expression)
} ?>
<?php unset(\$__defined_vars); ?>";
}

/**
* Sanitize the given component attribute value.
*
* @param mixed $value
* @return mixed
*/
public static function sanitizeComponentAttribute($value)
{
return is_string($value) ||
(is_object($value) && method_exists($value, '__toString'))
? e($value)
: $value;
}
}
32 changes: 32 additions & 0 deletions tests/View/Blade/BladeComponentTagCompilerTest.php
Expand Up @@ -5,6 +5,7 @@
use Illuminate\Container\Container;
use Illuminate\Contracts\Foundation\Application;
use Illuminate\Contracts\View\Factory;
use Illuminate\View\Compilers\BladeCompiler;
use Illuminate\View\Compilers\ComponentTagCompiler;
use Illuminate\View\Component;
use Mockery;
Expand Down Expand Up @@ -52,6 +53,22 @@ public function testDataCamelCasing()
<?php \$component->withAttributes([]); ?> @endcomponentClass", trim($result));
}

public function testColonData()
{
$result = (new ComponentTagCompiler(['profile' => TestProfileComponent::class]))->compileTags('<x-profile :user-id="1"></x-profile>');

$this->assertSame("@component('Illuminate\Tests\View\Blade\TestProfileComponent', ['userId' => 1])
<?php \$component->withAttributes([]); ?> @endcomponentClass", trim($result));
}

public function testColonAttributesIsEscapedIfStrings()
{
$result = (new ComponentTagCompiler(['profile' => TestProfileComponent::class]))->compileTags('<x-profile :src="\'foo\'"></x-profile>');

$this->assertSame("@component('Illuminate\Tests\View\Blade\TestProfileComponent', [])
<?php \$component->withAttributes(['src' => \Illuminate\View\Compilers\BladeCompiler::sanitizeComponentAttribute('foo')]); ?> @endcomponentClass", trim($result));
}

public function testColonNestedComponentParsing()
{
$result = (new ComponentTagCompiler(['foo:alert' => TestAlertComponent::class]))->compileTags('<x-foo:alert></x-foo:alert>');
Expand Down Expand Up @@ -157,6 +174,21 @@ public function testClasslessComponents()
<?php \$component->withAttributes(['name' => 'Taylor','age' => 31,'wire:model' => 'foo']); ?>
@endcomponentClass", trim($result));
}

public function testAttributeSanitization()
{
$class = new class {
public function __toString()
{
return '<hi>';
}
};

$this->assertEquals(e('<hi>'), BladeCompiler::sanitizeComponentAttribute('<hi>'));
$this->assertEquals(e('1'), BladeCompiler::sanitizeComponentAttribute('1'));
$this->assertEquals(1, BladeCompiler::sanitizeComponentAttribute(1));
$this->assertEquals(e('<hi>'), BladeCompiler::sanitizeComponentAttribute($class));
}
}

class TestAlertComponent extends Component
Expand Down