Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

set default extras as empty array to prevent compact() errors #170

Closed
wants to merge 3 commits into from
Closed

set default extras as empty array to prevent compact() errors #170

wants to merge 3 commits into from

Conversation

kilip
Copy link
Contributor

@kilip kilip commented Nov 12, 2018

Hello,

PHP compact() function at line 413 will create php error when $args is an empty array:

public function createDataStylesheet(array $args)
{
$rel = 'stylesheet';
$type = 'text/css';
$media = 'screen';
$conditionalStylesheet = false;
$href = array_shift($args);
if ($this->isDuplicateStylesheet($href)) {
return false;
}
if ($args) {
$media = array_shift($args);
if (is_array($media)) {
$media = implode(',', $media);
} else {
$media = (string) $media;
}
}
if ($args) {
$conditionalStylesheet = array_shift($args);
if (! empty($conditionalStylesheet) && is_string($conditionalStylesheet)) {
$conditionalStylesheet = (string) $conditionalStylesheet;
} else {
$conditionalStylesheet = null;
}
}
if ($args && is_array($args[0])) {
$extras = array_shift($args);
$extras = (array) $extras;
}
$attributes = compact('rel', 'type', 'href', 'media', 'conditionalStylesheet', 'extras');
return $this->createData($attributes);
}

I have found this error in this travis build:
https://travis-ci.org/cross-solution/YAWIK/jobs/453775095#L1805
To reproducing error just simply execute this function HeadLink::prependStylesheet('/some/foo.css')

I think this compact() error only occurs in php 7.3, but the code is buggy too. We need to define default value for $extras variables.

src/Helper/HeadLink.php Outdated Show resolved Hide resolved
src/Helper/HeadLink.php Outdated Show resolved Hide resolved
@froschdesign
Copy link
Member

Reference for this change in PHP 7.3: http://php.net/manual/en/function.compact.php#refsect1-function.compact-changelog

@froschdesign froschdesign added this to the 2.10.1 milestone Nov 12, 2018
@froschdesign
Copy link
Member

Duplicate of #167

@froschdesign froschdesign marked this as a duplicate of #167 Nov 12, 2018
@froschdesign froschdesign mentioned this pull request Nov 12, 2018
@weierophinney
Copy link
Member

Thanks, @kilip - cherry-picked to master, to release with 2.10.1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants