Skip to content

Commit

Permalink
Merge pull request #545 from flightphp/output-buffering-correction
Browse files Browse the repository at this point in the history 
Output buffering correction
  • Loading branch information
@fadrian06
fadrian06 authored Feb 18, 2024
2 parents c4c6e48 + 4d064cb commit 5073758
Show file tree
Hide file tree
Showing 19 changed files with 818 additions and 118 deletions.
6 changes: 5 additions & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit 5073758

Please sign in to comment.