omise / omise-opencart

Omise OpenCart Plugin
https://docs.opn.ooo/opencart-plugin
MIT License
10 stars 11 forks source link

Add supported for SSL configuration when create link through Url class #36

Closed guzzilar closed 7 years ago

guzzilar commented 7 years ago

1. Objective reason

To support OpenCart SSL configuration. If we look at the core code:

// ./system/library/url.php

class Url {
    public function __construct($domain, $ssl = '') {
        $this->domain = $domain;
        $this->ssl = $ssl;
    }

    public function link($route, $args = '', $secure = false) {
        if (!$secure) {
            $url = $this->domain;
        } else {
            $url = $this->ssl;
        }
    }
}
// ./index.php ; line: 58-65

if (!$store_query->num_rows) {
    $config->set('config_url', HTTP_SERVER);
    $config->set('config_ssl', HTTPS_SERVER);
}

// Url
$url = new Url($config->get('config_url'), $config->get('config_secure') ? $config->get('config_ssl') : $config->get('config_url'));
$registry->set('url', $url);

so, we could just add a param at the 3rd arg of that Url::link() method to tell OpenCart that we will use "SSL" config if it's possible (config_secure = true).

i.e. $this->url->link('some/url', '', 'SSL'));

2. Description of change

3. Users affected by the change

None

4. Impact of the change

None, it still support for http as normal.

5. Priority of change

Normal.

6. Alternate solution (if any)

No.

iwat commented 7 years ago

Does it need to be 'SSL' literally? The code seems to expect 3rd argument to be boolean so it could be 'true' instead.

The following code has the same effect:

$this->url->link('some/url', '', 'SSL'));
$this->url->link('some/url', '', 'noSSL'));

Never mind, I looked into the source code and found that they are using 'SSL' literally.

iwat commented 7 years ago

@nimid The plugin is very buggy without this PR. Especially, when using AJAX with HTTPS.

nimid commented 7 years ago

@iwat Ok.