New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reference to .env var in system.yaml for use_https doesn't obey false setting #2021

Closed
Krzemo opened this Issue Jun 15, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@Krzemo

Krzemo commented Jun 15, 2018

Description
site/settings/system.yaml doesn't allow to make use_https: .env dependant. It seems like when using {env:USE_HTTPS} reference it checks if there is anything set with empty() more then ===true.
What is strange is that it works ok (stops forcing https) when use_https: false is literally set in system.yaml but it fails to stop forcing HTTPS when use_https: {env:USE_HTTPS} and in .env USE_HTTPS=false.

To Reproduce
Steps to reproduce the behavior:

  1. Go to .env
  2. Add USE_HTTPS=false
  3. Go to site/settings/system.yaml
  4. Add use_https: {env:USE_HTTPS}
  5. Go to /cp

All link in left side menu are HTTPSed.

Expected behavior
I would expect to be able to set use_https: per .env

Environment details (please complete the following information):

  • Statamic Version 2.9.8
  • Upgrade:
  • macOS 10.13.5
  • Brower: not applicable
  • Web Server: Nginx (through brew)
  • PHP Version: 7.2.4
  • Addons installed: MobileDetect, Spock
@jasonvarga

This comment has been minimized.

Member

jasonvarga commented Jun 15, 2018

In statamic/core/API/Parse.php just before line 90, add this:

        if ($val === $matches[0]) {
            return env($matches[1]);
        }

Does that fix your issue?

@Krzemo

This comment has been minimized.

Krzemo commented Jun 15, 2018

Im afraid it doesnt...
obraz

obraz

obraz

obraz

@jasonvarga

This comment has been minimized.

Member

jasonvarga commented Jun 15, 2018

You need to wrap your env in quotes otherwise it is technically a YAML array.

use_https: '{env:USE_HTTPS}'

@Krzemo

This comment has been minimized.

Krzemo commented Jun 15, 2018

Tx! My bad.

@Krzemo Krzemo closed this Jun 15, 2018

@jasonvarga

This comment has been minimized.

Member

jasonvarga commented Jun 15, 2018

(Leave this open for now, we'll close it when we ship the fixed code) 👍

@jasonvarga jasonvarga reopened this Jun 15, 2018

@Krzemo

This comment has been minimized.

Krzemo commented Jun 15, 2018

im afraid there is no issue there - i made myself an a..
on the other hand strong typed check is always a good idea...

@jasonvarga

This comment has been minimized.

Member

jasonvarga commented Jun 15, 2018

I think there was an issue. Before you added the snippet of code I sent, use_https would be evaluated as "1" instead of true

@Krzemo

This comment has been minimized.

Krzemo commented Jun 15, 2018

AFAIK there is "not much" diff between 1 and true in PHP - thus my strong typing comment ...

@jasonvarga

This comment has been minimized.

Member

jasonvarga commented Jun 15, 2018

Ah sure. Before the change, true would be "1" and false would be "".
Depending on the situation they would probably work fine.

This change would just make things more explicit.

Anyway, fixed (or improved?) for next release.

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