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

F::load(): guard from output #4656

Merged
merged 6 commits into from
Oct 10, 2022
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
4 changes: 2 additions & 2 deletions src/Cms/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ protected function optionsFromConfig(): array

// load the main config options
$root = $this->root('config');
$options = F::load($root . '/config.php', []);
$options = F::load($root . '/config.php', [], allowOutput: false);

// merge into one clean options array
return $this->options = array_replace_recursive(Config::$data, $options);
Expand All @@ -1080,7 +1080,7 @@ protected function optionsFromEnvironment(array $props = []): array
$root = $this->root('config');

// first load `config/env.php` to access its `url` option
$envOptions = F::load($root . '/env.php', []);
$envOptions = F::load($root . '/env.php', [], allowOutput: false);

// use the option from the main `config.php`,
// but allow the `env.php` to override it
Expand Down
4 changes: 2 additions & 2 deletions src/Cms/AppPlugins.php
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ protected function extensionsFromFolders()
$class = str_replace(['.', '-', '_'], '', $name) . 'Page';

// load the model class
F::loadOnce($model);
F::loadOnce($model, allowOutput: false);

if (class_exists($class) === true) {
$models[$name] = $class;
Expand Down Expand Up @@ -908,7 +908,7 @@ protected function pluginsLoader(): array
$styles = $dir . '/index.css';

if (is_file($entry) === true) {
F::loadOnce($entry);
F::loadOnce($entry, allowOutput: false);
} elseif (is_file($script) === true || is_file($styles) === true) {
// if no PHP file is present but an index.js or index.css,
// register as anonymous plugin (without actual extensions)
Expand Down
2 changes: 1 addition & 1 deletion src/Cms/Blueprint.php
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ protected function preset(array $props): array
$preset = static::$presets[$props['preset']];

if (is_string($preset) === true) {
$preset = require $preset;
$preset = F::load($preset, allowOutput: false);
}

return $preset($props);
Expand Down
2 changes: 1 addition & 1 deletion src/Cms/Collections.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public function load(string $name)
$file = $kirby->root('collections') . '/' . $name . '.php';

if (is_file($file) === true) {
$collection = F::load($file);
$collection = F::load($file, allowOutput: false);

if ($collection instanceof Closure) {
return $collection;
Expand Down
2 changes: 1 addition & 1 deletion src/Cms/Languages.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public static function load()
$files = glob(App::instance()->root('languages') . '/*.php');

foreach ($files as $file) {
$props = F::load($file);
$props = F::load($file, allowOutput: false);

if (is_array($props) === true) {
// inject the language code from the filename
Expand Down
4 changes: 2 additions & 2 deletions src/Cms/Loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,12 @@ public function resolve($item)
{
if (is_string($item) === true) {
$item = match (F::extension($item)) {
'php' => require $item,
'php' => F::load($item, allowOutput: false),
default => Data::read($item)
};
}

if (is_callable($item)) {
if (is_callable($item) === true) {
$item = $item($this->kirby);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Cms/UserActions.php
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ protected function readCredentials(): array
$path = $this->root() . '/index.php';

if (is_file($path) === true) {
$credentials = F::load($path);
$credentials = F::load($path, allowOutput: false);

return is_array($credentials) === false ? [] : $credentials;
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/Cms/Users.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public static function load(string $root, array $inject = [])
// get role information
$path = $root . '/' . $userDirectory . '/index.php';
if (is_file($path) === true) {
$credentials = F::load($path);
$credentials = F::load($path, allowOutput: false);
}

// create user model based on role
Expand Down
2 changes: 1 addition & 1 deletion src/Data/PHP.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public static function read(string $file): array
throw new Exception('The file "' . $file . '" does not exist');
}

return (array)F::load($file, []);
return (array)F::load($file, [], allowOutput: false);
}

/**
Expand Down
70 changes: 50 additions & 20 deletions src/Filesystem/F.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Exception;
use IntlDateFormatter;
use Kirby\Cms\Helpers;
use Kirby\Http\Response;
use Kirby\Toolkit\I18n;
use Kirby\Toolkit\Str;
use Throwable;
Expand Down Expand Up @@ -347,18 +348,34 @@ public static function link(string $source, string $link, string $method = 'link
*
* @param array $data Optional array of variables to extract in the variable scope
*/
public static function load(string $file, $fallback = null, array $data = [])
{
public static function load(
string $file,
mixed $fallback = null,
array $data = [],
bool $allowOutput = true
) {
if (is_file($file) === false) {
return $fallback;
}

// we use the loadIsolated() method here to prevent the included
// file from overwriting our $fallback in this variable scope; see
// https://www.php.net/manual/en/function.include.php#example-124
$result = static::loadIsolated($file, $data);
$callback = fn () => static::loadIsolated($file, $data);

// if the loaded file should not produce any output,
// call the loaidIsolated method from the Response class
// which checks for unintended ouput and throws an error if detected
if ($allowOutput === false) {
$result = Response::guardAgainstOutput($callback);
} else {
$result = $callback();
}

if ($fallback !== null && gettype($result) !== gettype($fallback)) {
if (
$fallback !== null &&
gettype($result) !== gettype($fallback)
) {
return $fallback;
}

Expand All @@ -369,24 +386,28 @@ public static function load(string $file, $fallback = null, array $data = [])
* A super simple class autoloader
* @since 3.7.0
*/
public static function loadClasses(array $classmap, string|null $base = null): void
{
public static function loadClasses(
array $classmap,
string|null $base = null
): void {
// convert all classnames to lowercase
$classmap = array_change_key_case($classmap);

spl_autoload_register(function ($class) use ($classmap, $base) {
$class = strtolower($class);
spl_autoload_register(
fn ($class) => Response::guardAgainstOutput(function () use ($class, $classmap, $base) {
$class = strtolower($class);

if (!isset($classmap[$class])) {
return false;
}
if (isset($classmap[$class]) === false) {
return false;
}

if ($base) {
include $base . '/' . $classmap[$class];
} else {
include $classmap[$class];
}
});
if ($base) {
include $base . '/' . $classmap[$class];
} else {
include $classmap[$class];
}
})
);
}

/**
Expand All @@ -408,13 +429,22 @@ protected static function loadIsolated(string $file, array $data = [])
* Loads a file using `include_once()` and
* returns whether loading was successful
*/
public static function loadOnce(string $file): bool
{
public static function loadOnce(
string $file,
bool $allowOutput = true
): bool {
if (is_file($file) === false) {
return false;
}

include_once $file;
$callback = fn () => include_once $file;

if ($allowOutput === false) {
Response::guardAgainstOutput($callback);
} else {
$callback();
}

return true;
}

Expand Down
12 changes: 10 additions & 2 deletions src/Http/Environment.php
Original file line number Diff line number Diff line change
Expand Up @@ -786,12 +786,20 @@ public function options(string $root): array

// load the config for the host
if (empty($host) === false) {
$configHost = F::load($root . '/config.' . $host . '.php', []);
$configHost = F::load(
file: $root . '/config.' . $host . '.php',
fallback: [],
allowOutput: false
);
}

// load the config for the server IP
if (empty($addr) === false) {
$configAddr = F::load($root . '/config.' . $addr . '.php', []);
$configAddr = F::load(
file: $root . '/config.' . $addr . '.php',
fallback: [],
allowOutput: false
);
}

return array_replace_recursive($configHost, $configAddr);
Expand Down
19 changes: 19 additions & 0 deletions src/Http/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

namespace Kirby\Http;

use Closure;
use Exception;
use Kirby\Exception\LogicException;
use Kirby\Filesystem\F;
use Throwable;

Expand Down Expand Up @@ -182,6 +184,23 @@ public static function go(string $url = '/', int $code = 302): void
die(static::redirect($url, $code));
}

/**
* Ensures that the callback does not produce the first body output
* (used to show when loading a file creates side effects)
*/
public static function guardAgainstOutput(Closure $callback, ...$args): mixed
{
$before = headers_sent();
$result = $callback(...$args);
$after = headers_sent($file, $line);

if ($before === false && $after === true) {
throw new LogicException("Disallowed output from file $file:$line, possible accidental whitespace?");
}

return $result;
}

/**
* Getter for single headers
*
Expand Down
16 changes: 12 additions & 4 deletions src/Toolkit/Component.php
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public static function load(string $type): array
throw new Exception('Component definition ' . $definition . ' does not exist');
}

static::$types[$type] = $definition = F::load($definition);
static::$types[$type] = $definition = F::load($definition, allowOutput: false);
}

return $definition;
Expand All @@ -254,7 +254,11 @@ public static function setup(string $type): array

if (isset($definition['extends']) === true) {
// extend other definitions
$options = array_replace_recursive(static::defaults(), static::load($definition['extends']), $definition);
$options = array_replace_recursive(
static::defaults(),
static::load($definition['extends']),
$definition
);
} else {
// inject defaults
$options = array_replace_recursive(static::defaults(), $definition);
Expand All @@ -266,10 +270,14 @@ public static function setup(string $type): array
if (isset(static::$mixins[$mixin]) === true) {
if (is_string(static::$mixins[$mixin]) === true) {
// resolve a path to a mixin on demand
static::$mixins[$mixin] = include static::$mixins[$mixin];

static::$mixins[$mixin] = F::load(static::$mixins[$mixin], allowOutput: false);
}

$options = array_replace_recursive(static::$mixins[$mixin], $options);
$options = array_replace_recursive(
static::$mixins[$mixin],
$options
);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Blueprint/NodeI18nTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace Kirby\Blueprint;

/**
* @covers \Kirby\Blueprint\NodeI18n
* @coversDefaultClass \Kirby\Blueprint\NodeI18n
*/
class NodeI18nTest extends TestCase
{
Expand Down
Loading