Skip to content

Commit

Permalink
Merge pull request #4 from sandstorm/bugfix/klaro-unblock-and-mimetypes
Browse files Browse the repository at this point in the history 
BUGFIX: Klaro not able to unblock / application/json+ld should not be blocked
  • Loading branch information
@fheinze
fheinze authored Jan 28, 2021
2 parents 2e38bdf + 59009f4 commit 141caf1
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 25 deletions.
55 changes: 40 additions & 15 deletions Classes/Eel/Helper/CookiePunch.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,26 @@ public function blockScripts(
$tag,
$blockConfig
) use ($groupOverride) {
// IMPORTANT: keep the order here or update all tests!
// #########################################################################
// IMPORTANT: keep the order in the following section or update all tests!
// #########################################################################

$tag = TagHelper::tagRenameAttribute(
$tag,
TagHelper::SRC,
TagHelper::DATA_SRC
);

$hasType = TagHelper::tagHasAttribute($tag, TagHelper::TYPE);

$typeAttributeValue = TagHelper::tagGetAttributeValue($tag, "type");

if(!$typeAttributeValue) {
// We want to be least invasive and try to reuse the type attribute value
// if none is present we use fallback.
$typeAttributeValue = TagHelper::TYPE_JAVASCRIPT;
}

if (TagHelper::tagHasAttribute($tag, TagHelper::DATA_SRC)) {
if ($hasType) {
$tag = TagHelper::tagRenameAttribute(
Expand All @@ -126,7 +139,16 @@ public function blockScripts(
$tag = TagHelper::tagAddAttribute(
$tag,
TagHelper::TYPE,
TagHelper::TEXT_PLAIN
TagHelper::TYPE_TEXT_PLAIN
);
} else {
// IMPORTANT: we need to add data-type="text/javascript" here to prevent Klaro from
// not correctly recovering the correct value.
// we add type="text/javascript" which later will be turned into an data attribute
$tag = TagHelper::tagAddAttribute(
$tag,
TagHelper::DATA_TYPE,
$typeAttributeValue
);
}
} else {
Expand All @@ -140,18 +162,18 @@ public function blockScripts(
$tag = TagHelper::tagAddAttribute(
$tag,
TagHelper::TYPE,
TagHelper::TEXT_PLAIN
TagHelper::TYPE_TEXT_PLAIN
);
} else {
$tag = TagHelper::tagAddAttribute(
$tag,
TagHelper::TYPE,
TagHelper::TEXT_PLAIN
TagHelper::TYPE_TEXT_PLAIN
);
$tag = TagHelper::tagAddAttribute(
$tag,
TagHelper::DATA_TYPE,
TagHelper::TEXT_JAVASCRIPT
$typeAttributeValue
);
}
}
Expand Down Expand Up @@ -298,10 +320,12 @@ private function replaceTags(
function ($hits) use ($hitCallback) {
$tag = $hits[0];

// EARLY RETURN - NO CALLBACK
if (!$hitCallback) {
return $tag;
}

// EARLY RETURN - NEVER BLOCK
$neverBlock = $this->tagContains(
$tag,
TagHelper::DATA_NEVER_BLOCK
Expand All @@ -310,23 +334,24 @@ function ($hits) use ($hitCallback) {
return $tag;
}

// EARLY RETURN - SPECIAL MIME TYPE
$mimeType = TagHelper::tagGetAttributeValue(
$tag,
TagHelper::TYPE
);
if($mimeType === TagHelper::TYPE_TEXT_PLAIN || $mimeType === TagHelper::TYPE_APPLICATION_JSON_LD) return $tag;

// EARLY RETURN - HAS BLOCKING ATTRIBUTES
// if a part of the markup was already processed
$hasBlockingAttributes =
TagHelper::tagHasAttribute($tag, TagHelper::DATA_SRC) ||
TagHelper::tagHasAttribute(
$tag,
TagHelper::TYPE,
TagHelper::TEXT_PLAIN
) ||
TagHelper::tagHasAttribute(
$tag,
TagHelper::DATA_TYPE,
TagHelper::TEXT_JAVASCRIPT
TagHelper::DATA_TYPE
);
// We do not check if TagHelper::DATA_NAME is present, because we might want the editor
// to choose a group, e.g. in the inspector and still block tags.
if ($hasBlockingAttributes) {
return $tag;
}
if ($hasBlockingAttributes) return $tag;

$blockConfig = $this->getBlockConfig($tag);

Expand Down
21 changes: 19 additions & 2 deletions Classes/TagHelper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ class TagHelper
const DATA_NEVER_BLOCK = "data-never-block";

// Type Constants
const TEXT_PLAIN = "text/plain";
const TEXT_JAVASCRIPT = "text/javascript";
const TYPE_TEXT_PLAIN = "text/plain";
const TYPE_APPLICATION_JSON_LD = "application/ld+json";

// TODO: switch to "text/javascript" at some point
// update tests
const TYPE_JAVASCRIPT = "text/javascript";

/**
* @param string $tag
Expand Down Expand Up @@ -68,6 +72,19 @@ function ($hits) use ($newName) {
);
}

/**
* @param string $tag
* @param string $name
* @return string
*/
static function tagGetAttributeValue(
string $tag,
string $name
): ?string {
preg_match(self::buildMatchAttributeNameWithAnyValueReqex($name), $tag, $matches);
return isset($matches['value']) ? $matches['value'] : null;
}

/**
* @param string $tag
* @param string $name
Expand Down
28 changes: 21 additions & 7 deletions Tests/Unit/Eel/Helper/CookiePunchTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use Sandstorm\CookiePunch\Eel\Helper\CookiePunch;

/**
* Testcase for the ConvertNodeUris Fusion implementation
* Testcase
*/
class CookiePunchTest extends UnitTestCase
{
Expand Down Expand Up @@ -48,10 +48,11 @@ public function scriptTagsWillBeBlocked()
self::assertEquals($expected, $actual);

// ### <script> with src attribute ###

// IMPORTANT: we need to add data-type="text/javascript" here to prevent Klaro from
// not correctly recovering the correct value.
$markup = '<script src="myscripts.js"></script>';
$expected =
'<script data-src="myscripts.js" data-name="default"></script>';
'<script data-src="myscripts.js" data-type="text/javascript" data-name="default"></script>';
$actual = $blockExternalContentHelper->blockScripts($markup);
self::assertEquals($expected, $actual);

Expand All @@ -69,6 +70,14 @@ public function scriptTagsWillBeBlocked()
'<script data-src="myscripts.js" data-type="text/javascript" type="text/plain" data-name="default"/>';
$actual = $blockExternalContentHelper->blockScripts($markup);
self::assertEquals($expected, $actual);

// ### <script> with "application/javascript" or other types will be blocked ###
$markup =
'<script src="myscripts.js" defer type="application/javascript"></script>';
$expected =
'<script data-src="myscripts.js" defer data-type="application/javascript" type="text/plain" data-name="default"></script>';
$actual = $blockExternalContentHelper->blockScripts($markup);
self::assertEquals($expected, $actual);
}

/**
Expand All @@ -86,7 +95,7 @@ public function tagsWithDataNameWillBeBlocked()

$markup = '<script src="myscripts.js" data-name="default"></script>';
$expected =
'<script data-src="myscripts.js" data-name="default"></script>';
'<script data-src="myscripts.js" data-name="default" data-type="text/javascript"></script>';
$actual = $blockExternalContentHelper->blockScripts($markup);
self::assertEquals($expected, $actual);

Expand All @@ -112,7 +121,7 @@ public function tagsWillUseGroupFromEelHelperAndNotSettings()
);

$markup = '<script src="myscripts.js"></script>';
$expected = '<script data-src="myscripts.js" data-name="foo"></script>';
$expected = '<script data-src="myscripts.js" data-type="text/javascript" data-name="foo"></script>';
$actual = $blockExternalContentHelper->blockScripts(
$markup,
true,
Expand All @@ -134,7 +143,7 @@ public function tagsWillUseGroupFromEelHelperAndNotSettings()
/**
* @test
*/
public function strangeScriptTagsWillNeverBeBlocked()
public function scriptTagsWithSpecialMimetypesWillNeverBeBlocked()
{
$blockExternalContentHelper = new CookiePunch();
ObjectAccess::setProperty(
Expand All @@ -150,6 +159,11 @@ public function strangeScriptTagsWillNeverBeBlocked()
$expected = '<script type="text/plain"></script>';
$actual = $blockExternalContentHelper->blockScripts($markup);
self::assertEquals($expected, $actual);

$markup = '<script type="application/ld+json"></script>';
$expected = '<script type="application/ld+json"></script>';
$actual = $blockExternalContentHelper->blockScripts($markup);
self::assertEquals($expected, $actual);
}

/**
Expand Down Expand Up @@ -322,7 +336,7 @@ public function patternOptionsWillBeAddedToTags()
// <script>
$markup = '<script src="Packages/Vendor.Example/myscripts.js"/>';
$expected =
'<script data-src="Packages/Vendor.Example/myscripts.js" data-name="default" data-options="{&quot;foo&quot;:&quot;bar&quot;}"/>';
'<script data-src="Packages/Vendor.Example/myscripts.js" data-type="text/javascript" data-name="default" data-options="{&quot;foo&quot;:&quot;bar&quot;}"/>';
$actual = $blockExternalContentHelper->blockScripts($markup);
self::assertEquals($expected, $actual);

Expand Down
21 changes: 20 additions & 1 deletion Tests/Unit/TagHelperTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use Sandstorm\CookiePunch\TagHelper;

/**
* Testcase for the ConvertNodeUris Fusion implementation
* Testcase
*/
class TagHelperTest extends UnitTestCase
{
Expand All @@ -14,6 +14,25 @@ class TagHelperTest extends UnitTestCase
*/
public function tagHelper()
{
// ### Get Value of attribute ###
$markup = '<div style="with-style" data-foo="bar"></div>';
$expected = 'with-style';
$actual = TagHelper::tagGetAttributeValue($markup, 'style');

self::assertEquals($expected, $actual);

$markup = '<div style="with-style" data-type="text/application"></div>';
$expected = 'text/application';
$actual = TagHelper::tagGetAttributeValue($markup, 'data-type');

self::assertEquals($expected, $actual);

$markup = '<div style="with-style" data-foo="bar"></div>';
$expected = null;
$actual = TagHelper::tagGetAttributeValue($markup, 'no-attribute');

self::assertEquals($expected, $actual);

// ### Rename Attribute ###
$markup = '<div style="with-style" data-foo="bar"></div>';
$expected = '<div data-style="with-style" data-foo="bar"></div>';
Expand Down

0 comments on commit 141caf1

Please sign in to comment.