-
-
Notifications
You must be signed in to change notification settings - Fork 436
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
prevent HEADERS ALREADY SENT error #23
prevent HEADERS ALREADY SENT error #23
Conversation
$image->display() directly outputs the image together with the mimeType header, causing the Mage_Core_Controller_Varien_Front->dispatch() to log an Headers already sent error on sending the actual Response object
How can I test it? |
@@ -62,5 +63,8 @@ public function directiveAction() | |||
imagedestroy($image); | |||
*/ | |||
} | |||
$this->getResponse()->setHeader('Content-type', $image->getMimeType(), true); | |||
$this->getResponse()->setBody(ob_get_contents()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use ob_get_clean()
here and drop the next line?
I've reviewed this PR and will merge it if you make the suggested change or explain why it's better to leave it as you originally wrote it. |
@Flyingmana have you seen this review? |
sorry for letting you wait, have made the change |
@Flyingmana I know this PR has been in the queue for a VERY long time. I finally had time to review it again this morning and I only have one request. Could you extend the fix to \Mage_Adminhtml_Cms_Wysiwyg_ImagesController::thumbnailAction as well? |
next try :) |
Agh, and therein lies the problem with using one branch per version.. This PR was against 1.9.1 and apparently there are many files which conflict between 1.9.1 and 1.9.2.2 so even though it let me merge it into 1.9.1, merging 1.9.1 into 1.9.3.0 is a huge mess... |
Nevermind, looks like this PR was already in upstream 1.9.3.0 (slightly different) |
$image->display() directly outputs the image together with the mimeType header,
causing the Mage_Core_Controller_Varien_Front->dispatch() to log an
Headers already sent error on sending the actual Response object
The error occurs if magento tries to display an image in the cms wysiwyg editor mode.
The error occurs only in log, it seems to not affect the output of the image.