432484 - CakePHP's FULL_BASE_URL does not handle Netscalers+HTTPS

Status: RESOLVED WONTFIX

(addons.mozilla.org Graveyard :: Public Pages, defect, P5)

Description

[Fred Wenzel [:wenzel]]

The global constant FULL_BASE_URL is set by CakePHP automatically (see the API: http://api.cakephp.org/paths_8php-source.html) and should handle HTTPS correctly but due to the netscalers, we have to perform a different check.

This leads to all URLs that contain HTTPS being displayed as HTTP only on AMO, at the moment, such ass RSS feeds (bug 425729). In config/bootstrap.php, there is a bunch of workaround code to fix FULL_BASE_URL if it doesn't contain https:// but that could be saved if we fixed the constant in the first place.

(FYI, we need to check for $_SERVER['HTTP_MOZ_REQ_METHOD'] == 'HTTPS').

Comment 1

[Fred Wenzel [:wenzel]]

(In reply to comment #0)
> This leads to all URLs that contain HTTPS being displayed as HTTP only on AMO

All *absolute* URLs, I mean -- there aren't too many of them fortunately.

Comment 2

[Justin Scott [:fligtar]]

or you can use SITE_URL which is defined by IT in config for every production and staging site we have because I didn't know the auto-constant existed until it was too late.

Comment 3

[Fred Wenzel [:wenzel]]

Looking at http://api.cakephp.org/controller_8php-source.html#l00376, this problem is likely the cause for bug 412015. When using redirect(), the complete URL is generated using FULL_BASE_URL; if that does not contain https correctly, we redirect to http, and that shouldn't happen.

CCing clouserw as 412015 is his bug.

Comment 4

[Wil Clouser [:clouserw]]

Did we make up HTTP_MOZ_REQ_METHOD?  Google has no results for that string.  Just wondering if we could send a patch to cakephp for anyone else behind a proxy.

Comment 5

[Fred Wenzel [:wenzel]]

(In reply to comment #4)
> Did we make up HTTP_MOZ_REQ_METHOD?

The MOZ in it makes me believe so.

Comment 6

[Wil Clouser [:clouserw]]

oremj would know

Comment 7

[Jeremy Orem [:oremj]]

I can add SetEnv HTTPS to the apache vhost, so you can use the regular $_SERVER['HTTPS'] check.  Just make sure you aren't looking at SERVER_PORT, because it will not be port 443.

Comment 8

[Wil Clouser [:clouserw]]

That sounds fine.  We don't use SERVER_PORT anywhere.  This sounds like something we should test on a box with SSL before we switch it on the live site.  Can you do this on preview.amo?

Comment 9

[Wil Clouser [:clouserw]]

(In reply to comment #0)
> This leads to all URLs that contain HTTPS being displayed as HTTP only on AMO,
> at the moment, such ass RSS feeds (bug 425729). In config/bootstrap.php, there
> is a bunch of workaround code to fix FULL_BASE_URL if it doesn't contain
> https:// but that could be saved if we fixed the constant in the first place.

I'm happy to do this still, but where is a URL example?  The RSS feeds are all relative as far as I can tell.

Comment 10

[Fred Wenzel [:wenzel]]

There are a few instances of this in our code:

$ grep -ir full_base_url *
config/bootstrap.php:        $fullbaseurl = FULL_BASE_URL;
config/bootstrap.php:    $fullbaseurl = FULL_BASE_URL;
tests/views/addons/searchengines.test.php:            .'"'.FULL_BASE_URL.$this->controller->base.'/'.LANG.'/'.APP_SHORTNAME.'/'.FILES_URL.'/'.'"\+fileid\+"/"\+basename\+".src",[\s]*'
tests/views/addons/searchengines.test.php:            .'"'.FULL_BASE_URL.$this->controller->base.'/'.LANG.'/'.APP_SHORTNAME.'/images/addon_icon/"\+addonid\+"/"\+basename\+"."\+ext,[\s]*'
views/addons/share.thtml:        $url = FULL_BASE_URL . $html->url("/addon/{$addon_id}");
views/collections/share.thtml:        $url = FULL_BASE_URL . $html->url("/collection/{$collection_uuid}");
views/editors/email/inforequest_plain.thtml:Please go to <?=FULL_BASE_URL.$html->url("/developers/discuss/{$info['infoid']}", true, false, false)?> to reply.
views/editors/email/inforequest_reply_plain.thtml:Please go to <?=FULL_BASE_URL.$html->url("/developers/discuss/{$info['infoid']}", true, false, false)?> if you would like to reply.
views/editors/email/notify_update_plain.thtml:URL: <?=FULL_BASE_URL.$html->url("/addon/{$info['id']}", true, false)?> 
views/editors/email/notify_update_plain.thtml:Review Link: <?=FULL_BASE_URL.$html->url("/editors/review/{$info['versionid']}", true, false)?> 
views/editors/email/notify_version_comment_plain.thtml:Comment Link: <?=FULL_BASE_URL.$html->url("/editors/review/{$info['versionid']}#editorComment{$info['commentid']}", true, false)?> 
views/editors/email/notify_version_comment_plain.thtml:This notification was sent to you because you have participated in the discussion of this review.  You can manage all subscriptions at: <?=FULL_BASE_URL.$html->url('/users/edit#profile-notifications')?>
views/editors/review.thtml:                echo '&nbsp;<b>'.$html->link($platforms[$file['platform_id']], '/downloads/file/'.$file['id'].'/'.$file['filename'], array('onClick' => $this->controller->Amo->installTrigger($addon['Addon']['addontype_id'], FULL_BASE_URL.$html->url('/downloads/file/'.$file['id'].'/'.$file['filename']), $addon['Translation']['name']['string'].' '.$version['Version']['version'], '', $file['hash']).' return false;')).'</b>';
views/elements/install.thtml:                'engineURL' => FULL_BASE_URL . $html->urlFile($file['id'], $file['filename'], @$collection_uuid),
views/helpers/install_button.php:                    'engineURL' => FULL_BASE_URL . $this->Html->urlFile($file['id'], $file['filename'], $this->collectionUuid()),

judging my comment 2, we could just go ahead and use that everywhere, then stop worrying about Cake's constant.

Comment 11

[Fred Wenzel [:wenzel]]

(line-break fail)

Comment 12

[Fred Wenzel [:wenzel]]

This is in Remora: We won't fix this anymore.