Skip to content

Loading…

Bug 796088 - Adding SSL-only support for Bouncer. #14

Merged
merged 1 commit into from

3 participants

@brandonsavage

No description provided.

@rik rik commented on the diff
bouncer/php/index.php
@@ -202,6 +217,8 @@ function getGlobalFallbackProhibited($region_id) {
product_id = %d AND
os_id = %d", array($product_id, $os_id));
+ $client_ip = null;
@rik
rik added a note

Looks like a test variable that slipped.

That's actually intentional. The $client_ip variable is referenced before it's used elsewhere in the code. PHP doesn't consider this a fatal error, but does emit a notice, which I corrected by initializing the variable before it would be referenced by other code. This will be overwritten later when it discovers the client IP address.

@rik
rik added a note

Ah, nevermind then ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rik
rik commented

Logic looks good to me.

Aren't we using South for database migrations now though?

@brandonsavage brandonsavage merged commit f7fe017 into mozilla:master
@superawesome

Hmm... I'm no expert, but the "default = True" line concerns me a bit.... I believe new products are often added in a programmatic way, which currently doesn't know about this setting. that means new "update" products would get added and have this set by default, but we aren't planning to serve updates via HTTPS due to cost. We could of course change whatever code is currently doing that, but I don't know what/where that is or how complicated it might be to implement such a change.

Am I misunderstanding?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Showing with 33 additions and 6 deletions.
  1. +3 −1 apps/mirror/models.py
  2. +26 −5 bouncer/php/index.php
  3. +4 −0 sql/incremental.sql
View
4 apps/mirror/models.py
@@ -68,7 +68,9 @@ class Product(models.Model):
active = models.BooleanField(db_index=True, default=True)
checknow = models.BooleanField(db_index=True, verbose_name='Check Now?',
default=True)
-
+ ssl_only = models.BooleanField(db_index=False,
+ verbose_name='Serve Over SSL Only?',
+ default=True)
def __unicode__(self):
return self.name
View
31 bouncer/php/index.php
@@ -135,6 +135,15 @@ function getGlobalFallbackProhibited($region_id) {
}
}
+function setHttpType($ssl_only) {
+ if ($ssl_only) {
+ $http_type = "https://";
+ } else {
+ $http_type = "http://";
+ }
+ return $http_type;
+}
+
// if we don't have an os, make it windows, playing the odds
if (empty($_GET['os'])) {
@@ -184,12 +193,18 @@ function getGlobalFallbackProhibited($region_id) {
// get product for this language (if applicable)
$buf = $sdo->get_one("
- SELECT prod.id FROM mirror_products AS prod
+ SELECT prod.id, prod.ssl_only FROM mirror_products AS prod
LEFT JOIN mirror_product_langs AS langs ON (prod.id = langs.product_id)
WHERE prod.name LIKE '%s'
AND (langs.language LIKE '%s' OR langs.language IS NULL)",
- array($product_name, $where_lang), MYSQL_NUM);
- if (!empty($buf[0])) $product_id = $buf[0]; else $product_id = null;
+ array($product_name, $where_lang), MYSQL_ASSOC);
+ if (!empty($buf['id'])) {
+ $product_id = $buf['id'];
+ $ssl_only = $buf['ssl_only'];
+ } else {
+ $product_id = null;
+ $ssl_only = 0;
+ }
// do we have a valid os and product?
if (!empty($os_id) && !empty($product_id)) {
$location = $sdo->get_one("
@@ -202,6 +217,8 @@ function getGlobalFallbackProhibited($region_id) {
product_id = %d AND
os_id = %d", array($product_id, $os_id));
+ $client_ip = null;
@rik
rik added a note

Looks like a test variable that slipped.

That's actually intentional. The $client_ip variable is referenced before it's used elsewhere in the code. PHP doesn't consider this a fatal error, but does emit a notice, which I corrected by initializing the variable before it would be referenced by other code. This will be overwritten later when it discovers the client IP address.

@rik
rik added a note

Ah, nevermind then ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
// did we get a valid location?
if (!empty($location)) {
$mirrors = false;
@@ -237,6 +254,7 @@ function getGlobalFallbackProhibited($region_id) {
}
if ($region_id) {
+ $http_type = setHttpType($ssl_only);
$mirrors = $sdo->get("
SELECT
mirror_mirrors.id,
@@ -254,7 +272,8 @@ function getGlobalFallbackProhibited($region_id) {
mirror_location_mirror_map.location_id = %d AND
geoip_mirror_region_map.region_id = %d AND
mirror_mirrors.active='1' AND
- mirror_location_mirror_map.active ='1'
+ mirror_location_mirror_map.active ='1' AND
+ mirror_mirrors.baseurl LIKE '$http_type%%'
ORDER BY rating",
array($where_lang, $location['id'], $client_region), MYSQL_ASSOC, 'id');
}
@@ -264,6 +283,7 @@ function getGlobalFallbackProhibited($region_id) {
$fallback_global = getGlobalFallbackProhibited($client_region);
if (empty($mirrors) && !$fallback_global) {
// either no region chosen or no mirror found in the given region
+ $http_type = setHttpType($ssl_only);
$mirrors = $sdo->get("
SELECT
mirror_mirrors.id,
@@ -278,7 +298,8 @@ function getGlobalFallbackProhibited($region_id) {
mirror_mirrors.id = mirror_location_mirror_map.mirror_id AND
mirror_location_mirror_map.location_id = %d AND
mirror_mirrors.active='1' AND
- mirror_location_mirror_map.active ='1'
+ mirror_location_mirror_map.active ='1' AND
+ mirror_mirrors.baseurl LIKE '$http_type%%'
ORDER BY rating",
array($where_lang, $location['id']), MYSQL_ASSOC, 'id');
}
View
4 sql/incremental.sql
@@ -115,3 +115,7 @@ ALTER TABLE `geoip_regions` ADD COLUMN `fallback_id` integer;
ALTER TABLE `geoip_regions` ADD CONSTRAINT `fallback_id_refs_id_e6bfe66d` FOREIGN KEY (`fallback_id`) REFERENCES `geoip_regions` (`id`);
CREATE INDEX `geoip_regions_e28329c2` ON `geoip_regions` (`fallback_id`);
ALTER TABLE geoip_regions ADD COLUMN prevent_global_fallback int(1) NULL;
+
+-- Add SSL only support (bug 796088)
+ALTER TABLE mirror_products ADD COLUMN `ssl_only` tinyint(1) NOT NULL DEFAULT 0;
+
Something went wrong with that request. Please try again.