diff --git a/application/ajaxwebpage.class.inc.php b/application/ajaxwebpage.class.inc.php index e84252f31..ed464a163 100644 --- a/application/ajaxwebpage.class.inc.php +++ b/application/ajaxwebpage.class.inc.php @@ -1,5 +1,5 @@ sContentType == 'text/html') && ($this->sContentDisposition == 'inline')) + $s_captured_output = $this->ob_get_clean_safe(); + if (($this->sContentType == 'text/html') && ($this->sContentDisposition == 'inline')) { // inline content != attachment && html => filter all scripts for malicious XSS scripts echo self::FilterXSS($this->s_content); @@ -393,4 +392,3 @@ EOF } } -?> diff --git a/application/csvpage.class.inc.php b/application/csvpage.class.inc.php index bf0165d55..062301858 100644 --- a/application/csvpage.class.inc.php +++ b/application/csvpage.class.inc.php @@ -1,5 +1,5 @@ add_header("Content-Length: ".strlen(trim($this->s_content))); + $this->add_header("Content-Length: ".strlen(trim($this->s_content))); + + // Get the unexpected output but do nothing with it + $sTrash = $this->ob_get_clean_safe(); + foreach($this->a_headers as $s_header) { header($s_header); @@ -105,4 +109,3 @@ class CSVPage extends WebPage } } -?> diff --git a/application/itopwebpage.class.inc.php b/application/itopwebpage.class.inc.php index 707d366e0..f31e96e20 100644 --- a/application/itopwebpage.class.inc.php +++ b/application/itopwebpage.class.inc.php @@ -587,8 +587,7 @@ EOF header($s_header); } } - $s_captured_output = ob_get_contents(); - ob_end_clean(); + $s_captured_output = $this->ob_get_clean_safe(); $sHtml = "\n"; $sHtml .= "\n"; $sHtml .= "\n"; @@ -1039,4 +1038,3 @@ EOF $this->m_sMessage = $sMessage; } } -?> \ No newline at end of file diff --git a/application/webpage.class.inc.php b/application/webpage.class.inc.php index dd8e6eee0..07084db7b 100644 --- a/application/webpage.class.inc.php +++ b/application/webpage.class.inc.php @@ -1,5 +1,5 @@ sContentType = ''; $this->sContentDisposition = ''; $this->sContentFileName = ''; + $this->bTrashUnexpectedOutput = false; $this->s_OutputFormat = utils::ReadParam('output_format', 'html'); $this->a_OutputOptions = array(); ob_start(); // Start capturing the output @@ -392,22 +394,55 @@ class WebPage implements Page } /** - * Discard unexpected output data + * Discard unexpected output data (such as PHP warnings) * This is a MUST when the Page output is DATA (download of a document, download CSV export, download ...) */ public function TrashUnexpectedOutput() { - // This protection is redundant with a protection implemented in MetaModel::IncludeModule - // which detects such issues while loading module files - // Here, the purpose is to detect and discard characters produced by the code execution (echo) - $sPreviousContent = ob_get_clean(); - if (trim($sPreviousContent) != '') + $this->bTrashUnexpectedOutput = true; + } + + /** + * Read the output buffer and deal with its contents: + * - trash unexpected output if the flag has been set + * - report unexpected behaviors such as the output buffering being stopped + * + * Possible improvement: I've noticed that several output buffers are stacked, + * if they are not empty, the output will be corrupted. The solution would + * consist in unstacking all of them (and concatenate the contents). + */ + protected function ob_get_clean_safe() + { + $sOutput = ob_get_contents(); + if ($sOutput === false) { - if (Utils::GetConfig() && Utils::GetConfig()->Get('debug_report_spurious_chars')) + $sMsg = "Design/integration issue: No output buffer. Some piece of code has called ob_get_clean() or ob_end_clean() without calling ob_start()"; + if ($this->bTrashUnexpectedOutput) { - IssueLog::Error("Output already started before downloading file:\nContent was:'$sPreviousContent'\n"); + IssueLog::Error($sMsg); + $sOutput = ''; + } + else + { + $sOutput = $sMsg; } } + else + { + ob_end_clean(); // on some versions of PHP doing so when the output buffering is stopped can cause a notice + if ($this->bTrashUnexpectedOutput) + { + if (trim($sOutput) != '') + { + if (Utils::GetConfig() && Utils::GetConfig()->Get('debug_report_spurious_chars')) + { + IssueLog::Error("Trashing unexpected output:'$s_captured_output'\n"); + } + } + $sOutput = ''; + } + } + return $sOutput; } /** @@ -419,8 +454,8 @@ class WebPage implements Page { header($s_header); } - $s_captured_output = ob_get_contents(); - ob_end_clean(); + + $s_captured_output = $this->ob_get_clean_safe(); echo "\n"; echo "\n"; echo "\n"; diff --git a/application/xmlpage.class.inc.php b/application/xmlpage.class.inc.php index 3e9188f01..d9d6b99fc 100644 --- a/application/xmlpage.class.inc.php +++ b/application/xmlpage.class.inc.php @@ -1,5 +1,5 @@ m_bPassThrough) { + // Get the unexpected output but do nothing with it + $sTrash = $this->ob_get_clean_safe(); + $this->s_content = "\n".trim($this->s_content); $this->add_header("Content-Length: ".strlen($this->s_content)); foreach($this->a_headers as $s_header) @@ -79,8 +82,7 @@ class XMLPage extends WebPage } else { - $s_captured_output = ob_get_contents(); - ob_end_clean(); + $s_captured_output = $this->ob_get_clean_safe(); foreach($this->a_headers as $s_header) { header($s_header); @@ -101,13 +103,4 @@ class XMLPage extends WebPage public function table($aConfig, $aData, $aParams = array()) { } - - public function TrashUnexpectedOutput() - { - if (!$this->m_bPassThrough) - { - parent::TrashUnexpectedOutput(); - } - } } -?>