From ef9e377fde817d838d0fafaac271d81e0d82b59f Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Mon, 13 May 2024 18:04:31 +0200 Subject: [PATCH] mvc/api - improve stream handling and prevent "headers already sent" errors from being thrown. This commit moves the output handling from the action to the Response object for stream types, which also ensures headers are being sent in the right order. --- .../OPNsense/Base/ApiControllerBase.php | 20 +++++++------------ .../OPNsense/Base/ControllerRoot.php | 3 ++- .../mvc/app/library/OPNsense/Mvc/Response.php | 17 +++++++++++++--- .../mvc/app/library/OPNsense/Mvc/Security.php | 1 - 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php index 46aaf7647..f9b8ebc4c 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php @@ -154,14 +154,12 @@ class ApiControllerBase extends ControllerRoot foreach ($records as $record) { fputcsv($stream, $record); } - fseek($stream, 0); foreach ($headers as $header) { - header($header); + $parts = explode(':', $header, 2); + $this->response->setHeader($parts[0], ltrim($parts[1])); } - ob_end_flush(); rewind($stream); - fpassthru($stream); - fclose($stream); + $this->response->setContent($stream); } /** @@ -179,17 +177,13 @@ class ApiControllerBase extends ControllerRoot ], $poll_timeout = 2 ) { - /* Never allow output compression on streams */ - ini_set('zlib.output_compression', 'Off'); - ob_end_clean(); $response = (new Backend())->configdpStream($action, $params, $poll_timeout); + foreach ($headers as $header) { - header($header); + $parts = explode(':', $header, 2); + $this->response->setHeader($parts[0], ltrim($parts[1])); } - while (ob_get_level() > 0) { - ob_end_flush(); - } - fpassthru($response); + $this->response->setContent($response); } /** diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerRoot.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerRoot.php index def058187..656ea3509 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerRoot.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ControllerRoot.php @@ -61,11 +61,12 @@ class ControllerRoot extends Controller protected $langcode = 'en_US'; /** + * XXX: remove in a future version, sessions are handled via session class * Wrap close session, for long running operations. */ protected function sessionClose() { - session_write_close(); + return; } /** diff --git a/src/opnsense/mvc/app/library/OPNsense/Mvc/Response.php b/src/opnsense/mvc/app/library/OPNsense/Mvc/Response.php index 1584870c3..9f0a0ed13 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Mvc/Response.php +++ b/src/opnsense/mvc/app/library/OPNsense/Mvc/Response.php @@ -33,7 +33,7 @@ use Exception; class Response { private Headers $headers; - private string $content = ''; + private mixed $content = ''; private bool $sent = false; public function __construct() @@ -46,7 +46,7 @@ class Response return $this->headers; } - public function setContent(string $content): void + public function setContent(mixed $content): void { $this->content = $content; } @@ -98,7 +98,18 @@ class Response } $this->headers->send(); - echo $this->content; + if (is_resource($this->content)) { + /* Never allow output compression on streams */ + ini_set('zlib.output_compression', 'Off'); + while (ob_get_level() > 0) { + ob_end_flush(); + } + fpassthru($this->content); + @fclose($this->content); + } elseif (!empty($this->content)) { + echo $this->content; + } + $this->sent = true; } diff --git a/src/opnsense/mvc/app/library/OPNsense/Mvc/Security.php b/src/opnsense/mvc/app/library/OPNsense/Mvc/Security.php index 4152166ac..f5f4a6878 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Mvc/Security.php +++ b/src/opnsense/mvc/app/library/OPNsense/Mvc/Security.php @@ -87,7 +87,6 @@ class Security */ public function checkToken(?string $tokenKey = null, ?string $tokenValue = null): bool { - $key = $tokenKey ?? $this->getTokenKey(false); if (empty($key)) { return false;