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

Output buffering correction #545

Merged
merged 11 commits into from Feb 18, 2024
6 changes: 5 additions & 1 deletion composer.json
Expand Up @@ -50,11 +50,15 @@
"config": {
"allow-plugins": {
"phpstan/extension-installer": true
}
},
"process-timeout": 0,
"sort-packages": true
},
"scripts": {
"test": "phpunit",
"test-coverage": "rm clover.xml && XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html=coverage --coverage-clover=clover.xml && vendor/bin/coverage-check clover.xml 100",
"test-server": "echo \"Running Test Server\" && php -S localhost:8000 -t tests/server/",
"test-server-v2": "echo \"Running Test Server\" && php -S localhost:8000 -t tests/server-v2/",
"test-coverage:win": "del clover.xml && phpunit --coverage-html=coverage --coverage-clover=clover.xml && coverage-check clover.xml 100",
"lint": "phpstan --no-progress -cphpstan.neon",
"beautify": "phpcbf --standard=phpcs.xml",
Expand Down
180 changes: 115 additions & 65 deletions flight/Engine.php
Expand Up @@ -27,7 +27,7 @@
* # Core methods
* @method void start() Starts engine
* @method void stop() Stops framework and outputs current response
* @method void halt(int $code = 200, string $message = '') Stops processing and returns a given response.
* @method void halt(int $code = 200, string $message = '', bool $actuallyExit = true) Stops processing and returns a given response.
*
* # Routing
* @method Route route(string $pattern, callable $callback, bool $pass_route = false, string $alias = '')
Expand Down Expand Up @@ -66,6 +66,15 @@
*/
class Engine
{
/**
* @var array<string> List of methods that can be extended in the Engine class.
*/
private const MAPPABLE_METHODS = [
'start', 'stop', 'route', 'halt', 'error', 'notFound',
'render', 'redirect', 'etag', 'lastModified', 'json', 'jsonp',
'post', 'put', 'patch', 'delete', 'group', 'getUrl'
];

/** @var array<string, mixed> Stored variables. */
protected array $vars = [];

Expand Down Expand Up @@ -137,14 +146,7 @@ public function init(): void
$view->extension = $self->get('flight.views.extension');
});

// Register framework methods
$methods = [
'start', 'stop', 'route', 'halt', 'error', 'notFound',
'render', 'redirect', 'etag', 'lastModified', 'json', 'jsonp',
'post', 'put', 'patch', 'delete', 'group', 'getUrl',
];

foreach ($methods as $name) {
foreach (self::MAPPABLE_METHODS as $name) {
$this->dispatcher->set($name, [$this, "_$name"]);
}

Expand All @@ -156,6 +158,7 @@ public function init(): void
$this->set('flight.views.path', './views');
$this->set('flight.views.extension', '.php');
$this->set('flight.content_length', true);
$this->set('flight.v2.output_buffering', false);

// Startup configuration
$this->before('start', function () use ($self) {
Expand All @@ -169,6 +172,10 @@ public function init(): void
$self->router()->case_sensitive = $self->get('flight.case_sensitive');
// Set Content-Length
$self->response()->content_length = $self->get('flight.content_length');
// This is to maintain legacy handling of output buffering
// which causes a lot of problems. This will be removed
// in v4
$self->response()->v2_output_buffering = $this->get('flight.v2.output_buffering');
});

$this->initialized = true;
Expand Down Expand Up @@ -354,8 +361,67 @@ public function path(string $dir): void
$this->loader->addDirectory($dir);
}

// Extensible Methods
/**
* Processes each routes middleware.
*
* @param array<int, callable> $middleware Middleware attached to the route.
* @param array<mixed> $params `$route->params`.
* @param string $event_name If this is the before or after method.
*/
protected function processMiddleware(array $middleware, array $params, string $event_name): bool
{
$at_least_one_middleware_failed = false;

foreach ($middleware as $middleware) {
$middleware_object = false;

if ($event_name === 'before') {
// can be a callable or a class
$middleware_object = (is_callable($middleware) === true
? $middleware
: (method_exists($middleware, 'before') === true
? [$middleware, 'before']
: false
)
);
} elseif ($event_name === 'after') {
// must be an object. No functions allowed here
if (
is_object($middleware) === true
&& !($middleware instanceof Closure)
&& method_exists($middleware, 'after') === true
) {
$middleware_object = [$middleware, 'after'];
}
}

if ($middleware_object === false) {
continue;
}

if ($this->response()->v2_output_buffering === false) {
ob_start();
}

// It's assumed if you don't declare before, that it will be assumed as the before method
$middleware_result = $middleware_object($params);

if ($this->response()->v2_output_buffering === false) {
$this->response()->write(ob_get_clean());
}

if ($middleware_result === false) {
$at_least_one_middleware_failed = true;
break;
}
}

return $at_least_one_middleware_failed;
}

////////////////////////
// Extensible Methods //
////////////////////////
/**
* Starts the framework.
*
Expand All @@ -374,16 +440,20 @@ public function _start(): void
$self->stop();
});

// Flush any existing output
if (ob_get_length() > 0) {
$response->write(ob_get_clean()); // @codeCoverageIgnore
}
if ($response->v2_output_buffering === true) {
// Flush any existing output
if (ob_get_length() > 0) {
$response->write(ob_get_clean()); // @codeCoverageIgnore
}

// Enable output buffering
ob_start();
// Enable output buffering
// This is closed in the Engine->_stop() method
ob_start();
}

// Route the request
$failed_middleware_check = false;

while ($route = $router->route($request)) {
$params = array_values($route->params);

Expand All @@ -394,60 +464,39 @@ public function _start(): void

// Run any before middlewares
if (count($route->middleware) > 0) {
foreach ($route->middleware as $middleware) {
$middleware_object = (is_callable($middleware) === true
? $middleware
: (method_exists($middleware, 'before') === true
? [$middleware, 'before']
: false));

if ($middleware_object === false) {
continue;
}

// It's assumed if you don't declare before, that it will be assumed as the before method
$middleware_result = $middleware_object($route->params);

if ($middleware_result === false) {
$failed_middleware_check = true;
break 2;
}
$at_least_one_middleware_failed = $this->processMiddleware($route->middleware, $route->params, 'before');
if ($at_least_one_middleware_failed === true) {
$failed_middleware_check = true;
break;
}
}

if ($response->v2_output_buffering === false) {
ob_start();
}

// Call route handler
$continue = $this->dispatcher->execute(
$route->callback,
$params
);

if ($response->v2_output_buffering === false) {
$response->write(ob_get_clean());
}

// Run any before middlewares
if (count($route->middleware) > 0) {
// process the middleware in reverse order now
foreach (array_reverse($route->middleware) as $middleware) {
// must be an object. No functions allowed here
$middleware_object = false;

if (
is_object($middleware) === true
&& !($middleware instanceof Closure)
&& method_exists($middleware, 'after') === true
) {
$middleware_object = [$middleware, 'after'];
}

// has to have the after method, otherwise just skip it
if ($middleware_object === false) {
continue;
}

$middleware_result = $middleware_object($route->params);

if ($middleware_result === false) {
$failed_middleware_check = true;
break 2;
}
$at_least_one_middleware_failed = $this->processMiddleware(
array_reverse($route->middleware),
$route->params,
'after'
);

if ($at_least_one_middleware_failed === true) {
$failed_middleware_check = true;
break;
}
}

Expand All @@ -463,7 +512,7 @@ public function _start(): void
}

if ($failed_middleware_check === true) {
$this->halt(403, 'Forbidden');
$this->halt(403, 'Forbidden', empty(getenv('PHPUNIT_TEST')));
} elseif ($dispatched === false) {
$this->notFound();
}
Expand Down Expand Up @@ -514,8 +563,9 @@ public function _stop(?int $code = null): void
$response->status($code);
}

$content = ob_get_clean();
$response->write($content ?: '');
if ($response->v2_output_buffering === true && ob_get_length() > 0) {
$response->write(ob_get_clean());
}

$response->send();
}
Expand Down Expand Up @@ -599,16 +649,16 @@ public function _delete(string $pattern, callable $callback, bool $pass_route =
*
* @param int $code HTTP status code
* @param string $message Response message
* @param bool $actuallyExit Whether to actually exit the script or just send response
*/
public function _halt(int $code = 200, string $message = ''): void
public function _halt(int $code = 200, string $message = '', bool $actuallyExit = true): void
{
$this->response()
->clear()
->status($code)
->write($message)
->send();
// apologies for the crappy hack here...
if ($message !== 'skip---exit') {
if ($actuallyExit === true) {
exit(); // @codeCoverageIgnore
}
}
Expand Down Expand Up @@ -742,7 +792,7 @@ public function _etag(string $id, string $type = 'strong'): void
isset($_SERVER['HTTP_IF_NONE_MATCH']) &&
$_SERVER['HTTP_IF_NONE_MATCH'] === $id
) {
$this->halt(304);
$this->halt(304, '', empty(getenv('PHPUNIT_TEST')));
}
}

Expand All @@ -759,7 +809,7 @@ public function _lastModified(int $time): void
isset($_SERVER['HTTP_IF_MODIFIED_SINCE']) &&
strtotime($_SERVER['HTTP_IF_MODIFIED_SINCE']) === $time
) {
$this->halt(304);
$this->halt(304, '', empty(getenv('PHPUNIT_TEST')));
}
}

Expand Down
2 changes: 1 addition & 1 deletion flight/Flight.php
Expand Up @@ -22,7 +22,7 @@
* @method static void start() Starts the framework.
* @method static void path(string $path) Adds a path for autoloading classes.
* @method static void stop(?int $code = null) Stops the framework and sends a response.
* @method static void halt(int $code = 200, string $message = '')
* @method static void halt(int $code = 200, string $message = '', bool $actuallyExit = true)
* Stop the framework with an optional status code and message.
*
* # Routing
Expand Down
2 changes: 1 addition & 1 deletion flight/database/PdoWrapper.php
Expand Up @@ -136,6 +136,6 @@ protected function processInStatementSql(string $sql, array $params = []): array
$current_index += strlen($question_marks) + 4;
}

return [ 'sql' => $sql, 'params' => $params ];
return ['sql' => $sql, 'params' => $params];
}
}
43 changes: 43 additions & 0 deletions flight/net/Request.php
Expand Up @@ -331,6 +331,49 @@ public static function getHeaders(): array
return $headers;
}

/**
* Alias of Request->getHeader(). Gets a single header.
*
* @param string $header Header name. Can be caps, lowercase, or mixed.
* @param string $default Default value if the header does not exist
*
* @return string
*/
public static function header(string $header, $default = '')
{
return self::getHeader($header, $default);
}

/**
* Alias of Request->getHeaders(). Gets all the request headers
*
* @return array<string, string|int>
*/
public static function headers(): array
{
return self::getHeaders();
}

/**
* Gets the full request URL.
*
* @return string URL
*/
public function getFullUrl(): string
{
return $this->scheme . '://' . $this->host . $this->url;
}

/**
* Grabs the scheme and host. Does not end with a /
*
* @return string
*/
public function getBaseUrl(): string
{
return $this->scheme . '://' . $this->host;
}

/**
* Parse query parameters from a URL.
*
Expand Down