Skip to content
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

FIX Respect SS_BASE_URL scheme in CLI environment #10616

Conversation

s-kerdel
Copy link
Contributor

@s-kerdel s-kerdel commented Dec 13, 2022

Additionally set _SERVER variables for HTTPS and SSL to respect SS_BASE_URL scheme when executing builds and tasks through CLI. This should solve base tags not being provided with the correct HTTP scheme. This is important to resolve mixed content issues and insecure requests.

Parent issue

@michalkleiner michalkleiner changed the title ISSUE-10615: Respect SS_BASE_URL scheme in CLI environment. FIX Respect SS_BASE_URL scheme in CLI environment Dec 15, 2022
Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

This seems okay to me - I notice some discussion about the host in the issue, but it seems like the host is already being set correctly and any change to the way that's handled could be an additional enhancement if we discover that it would be useful.
This PR does seem to resolve a genuine issue in and off itself.

I'd like another pair of eyes over this before merging @silverstripe/core-team

Also, this should probably target the 4.12 branch so that it can be released in a patch release rather than waiting for the 4.13 minor release.

Additionally set _SERVER variables for HTTPS and SSL to respect SS_BASE_URL scheme when executing builds and tasks through CLI.
This should solve base tags not being provided with the correct HTTP scheme. This is important to resolve mixed content issues and insecure requests.
@GuySartorelli GuySartorelli force-pushed the 10615-Respect-SS_BASE_URL-for-CLI-RequestBuilder branch from da18ce7 to 4a1eb0c Compare December 19, 2022 22:16
@GuySartorelli GuySartorelli changed the base branch from 4 to 4.12 December 19, 2022 22:16
@GuySartorelli
Copy link
Member

I've changed the base of this to 4.12 so it can be in a patch release. I'll merge once CI is green again.

@GuySartorelli GuySartorelli merged commit 3564f98 into silverstripe:4.12 Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants