Skip to content

Commit

Permalink
Forces visitor to https if full site is setup for https and http is e…
Browse files Browse the repository at this point in the history 
…ntered.

Per https://www.zen-cart.com/content.php?56-how-do-i-enable-ssl-after-i-have-installed-zen-cart,
if one is to set the entire site to https, then it is recommended that
SSL_ENABLED be set to false, but if the uri entered begins with http, then
the redirect to force the page to load via SSL is not activated and the
customer could enter enter data to the webpage without using the SSL.

Fixes zencart#1369
  • Loading branch information
@mc12345678
mc12345678 committed Jan 20, 2017
1 parent db19c24 commit 36e6b22
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion includes/modules/pages/contact_us/header_php.php
View file
Expand Up @@ -91,7 +91,7 @@
} // end action==send


if (ENABLE_SSL == 'true' && $request_type != 'SSL') {
if (ENABLE_SSL == 'true' && $request_type != 'SSL' || ENABLE_SSL != 'true' && $request_type != 'SSL' && substr(HTTP_SERVER, 0, 5) == 'https') {
zen_redirect(zen_href_link(FILENAME_CONTACT_US, '', 'SSL'));
}

Expand Down

3 comments on commit 36e6b22

@drbyte
Copy link

@drbyte drbyte commented on 36e6b22 Jan 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced this is correct.

You've changed it from "2 required conditions" to "5 conditions mixing 'and' and 'or'", which means it's probably not going to do what you thought it was.

I suspect you intended there to be (2 required) or (3 other required), but left out the parentheses.

And probably could be simplified to:

if ($request_type != 'SSL' && (ENABLE_SSL == 'true' || substr(HTTP_SERVER, 0, 5) == 'https') {

@mc12345678
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the suggested revision would be more appropriate though I didn't want to go straight there without further review. :)
It doesn't specifically fall into the "requirements" of the referenced FAQ. In that it does ignore the value of ENABLE_SSL if HTTP_SERVER is https: where the FAQ identifies the need for ENABLE_SSL to be false (therefore was added in this test until suggested otherwise).

As to the order of operations, && (and groups of them) process before || therefore, there is a group of 2 that are processed together (with equalities inside which take a lower precedence than both of those two operations) compared to a group of three values. If the first group of two are true, then none of the others will be processed, if either of the first two are false, then the right side of the h| are processed until any one is false or all are true.

Tried on an online php tester and could only get a true response if both left two were true, all three on the right were true, or all five were true.

Also in the online testing I did with my test server, the only conditions I didn't try were where
HTTPS_SERVER: http:

All other states occurred/responded satisfactorily, no error logs generated, no "connection" delays/redirects. Typed in address, with known expectation (either would or would not react) and then accepted address to see result. Each time ended up where expected.

So, if want the logic "compressed" as would be expected for computer/electronics engineering, then I'll gladly modify to suit.

@drbyte
Copy link

@drbyte drbyte commented on 36e6b22 Jan 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: the order of operations, yes, you're right. I often mentally "expect" both && and || to be treated with equal priority, since the next issue after that is readability. And in readability less is more. That said, I hate adding unnecessary parentheses too!

Compressing it down to the bare essentials is preferred ... for all the same reasons. ;)

Thanks again for testing and contributing!

Please sign in to comment.