Page 1 of 1

[v3.0.2.0 - Concept] - Die command

Posted: Wed Oct 04, 2017 9:08 am
by straightlight
The following contains all the:

Code: Select all

die()
command which can prevent on completing important events on Opencart.

Code: Select all

admin/controller/extension/openbay/amazonus_product.php
admin/controller/extension/openbay/amazon_product.php
admin/controller/extension/openbay/etsy_product.php
catalog/controller/extension/payment/globalpay_remote.php
catalog/controller/extension/payment/realex_remote.php
catalog/model/extension/payment/alipay.php

Re: [v3.0.2.0 - Concept] - Die command

Posted: Thu Oct 05, 2017 7:54 am
by arnisraido
Is it bad?
e..g `header("Location: ...");` _must_ also have `exit;` or `die();` control at the end.

`$this->response->setOutput(json_encode($json));` on the other hand, already sends output and `die()` there seems to be no use.

Re: [v3.0.2.0 - Concept] - Die command

Posted: Thu Oct 05, 2017 8:44 am
by straightlight
Those instances should rather not use an example like the following as we already see in the catalog/controller/extension/payment/pp_express.php file:

Code: Select all

if ($this->config->get('payment_pp_express_test') == 1) {
			header('Location: https://www.sandbox.paypal.com/cgi-bin/webscr?cmd=_express-checkout&token=' . $result['TOKEN']);
		} else {
			header('Location: https://www.paypal.com/cgi-bin/webscr?cmd=_express-checkout&token=' . $result['TOKEN']);
		}
but should rather be replaced with:

Code: Select all

if ($this->config->get('payment_pp_express_test') == 1) {
			$this->response->addHeader('Location: https://www.sandbox.paypal.com/cgi-bin/webscr?cmd=_express-checkout&token=' . $result['TOKEN']);
		} else {
			$this->response->addHeader('Location: https://www.paypal.com/cgi-bin/webscr?cmd=_express-checkout&token=' . $result['TOKEN']);
		}
No exit function being used and the headers_sent function is already declared for each redirection throughout the entire platform from system/library/response.php addHeader method while the previous code I showed above the replacement does not originally use the $this->response->addHeader while it should in this case.

As for the JSON encoding, it may already sent the output but back to the browser as it is mainly used for local redirection inside the domain compared to the PHP header function. Of course, with the jQuery location function, that could also lead to outside redirections but does not reflect on any errors that it may cause compared to PHP with the headers already sent by ... . Those functions does not reflect either on the $.getJSON objects to call URL outside the domain but, still, PHP needs to be handled a bit differently with site redirection especially while using JSON encoding. Otherwise, it might definitely cause an error message in the error logs.