redirect: determine the FromScheme at runtime (#1297) #1365

Merged
merged 1 commit into from Feb 16, 2017

Projects

None yet

2 participants

@tw4452852
Collaborator
tw4452852 commented Jan 21, 2017 edited

cfg.TLS.Enabled may be changed after plugin setup (here), but redirect plugin doesn't catch it.

@tw4452852 tw4452852 redirect: determine the FromScheme at runtime (#1297)
Signed-off-by: Tw <tw19881113@gmail.com>
eeb23a2
@mholt

Thanks for working on this Tw!

+ if cfg.TLS.Enabled {
+ return "https"
+ }
+ return "http"
@mholt
mholt Feb 9, 2017 Owner

Is this thread-safe?

(If not, and if the solution is to introduce locking, let's reconsider how we do the scheme changing instead...)

@tw4452852
tw4452852 Feb 9, 2017 Collaborator

cfg.TLS.Enabled is setup at the server's initialization. Unless we changed it on the fly later, it is thread-safe IMO.

@mholt
mholt approved these changes Feb 16, 2017 View changes

Thanks for fixing the issue! If a race does crop up because of this, I hope you'll be able to help us fix it. I'll merge this now.

@mholt mholt merged commit 1183d91 into mholt:master Feb 16, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@tw4452852 tw4452852 deleted the tw4452852:1297 branch Feb 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment