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.

Sign up for GitHub

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

Jump to bottom

reverseproxy: fix upstream scheme handling #5088

Merged
merged 1 commit into from
Sep 27, 2022
Merged

reverseproxy: fix upstream scheme handling #5088

merged 1 commit into from
Sep 27, 2022

Conversation

lemmi
Copy link
Contributor

@lemmi lemmi commented Sep 27, 2022

e338648 introduced multiple upstream addresses. A comment notes that mixing schemes isn't supported and therefore the first valid scheme is supposed to be used.

Fixes setting the first scheme.

fixes #5087

@lemmi
reverseproxy: fix upstream scheme handling
ae6dd64 
e338648 introduced multiple upstream
addresses. A comment notes that mixing schemes isn't supported and
therefore the first valid scheme is supposed to be used.

Fixes setting the first scheme.

fixes caddyserver#5087
@CLAassistant
Copy link

CLAassistant commented Sep 27, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@francislavoie
Copy link
Member

francislavoie commented Sep 27, 2022 •
edited
Loading

This will need tests. Show an example of a config that you think is incorrectly handled before this change, etc. Sorry I missed that you did put that info in the issue, my bad. Tests for the command are probably not necessary.

Please also remember to sign the CLA.

@francislavoie francislavoie added the under review 🧐 Review is pending before merging label Sep 27, 2022
@francislavoie francislavoie added this to the v2.6.2 milestone Sep 27, 2022
mholt
mholt approved these changes Sep 27, 2022
View reviewed changes
Copy link
Member

@mholt mholt left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Ah yeah, this makes a lot more sense. Thank you for the patch!

We do need the CLA signed before merging.

@lemmi
Copy link
Contributor Author

lemmi commented Sep 27, 2022

There you go :)

@mholt mholt added bug 🐞 Something isn't working and removed under review 🧐 Review is pending before merging labels Sep 27, 2022
@mholt mholt merged commit d055692 into caddyserver:master Sep 27, 2022
@mholt
Copy link
Member

mholt commented Sep 27, 2022

Awesome, thanks!

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

@mholt mholt mholt approved these changes

Assignees
No one assigned
Labels
bug 🐞 Something isn't working
Projects
None yet
Milestone
v2.6.2
Development

Successfully merging this pull request may close these issues.

caddy 2.6.1: caddy reverse-proxy --to https:// uses HTTP for upstream connection
4 participants
@lemmi @CLAassistant @francislavoie @mholt