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

Double reverse proxies with https lead to double X-Scheme headers generated by haproxy #239

Closed
foosel opened this Issue Jun 29, 2016 · 1 comment

Comments

Projects
None yet
1 participant
@foosel
Collaborator

foosel commented Jun 29, 2016

See foosel/OctoPrint#1391 and specifically this comment.

The current haproxy.cfg as shipped with OctoPi will always add an X-Scheme header to the request towards OctoPrint, even if an X-Scheme header already exists on the request. That lead to problems within OctoPrint since it would read two (or more) X-Scheme headers as https,https, generating wrong URLs as a consequence. While that has now been solved (and the corresponding fix will be part of the next release) we should still make sure to not generate secondary X-Scheme headers - if a header is already set, we need to just forward it and not touch it in any way.

I've experimented a bit with haproxy.cfg and I think I've found a proper way to solve this:

backend octoprint
        acl needs_scheme req.hdr_cnt(X-Scheme) eq 0

        reqrep ^([^\ :]*)\ /(.*) \1\ /\2
        reqadd X-Scheme:\ https if needs_scheme { ssl_fc }
        reqadd X-Scheme:\ http if needs_scheme !{ ssl_fc }
        option forwardfor
        server octoprint1 127.0.0.1:5000

This should ensure X-Scheme: https to only be set if haproxy is accessed through SSL and there's not already an existing X-Scheme header. Likewise for X-Scheme: http if haproxy is not accessed through SSL and there's no existing header already.

I'd like to give this a bit more testing before preparing a PR though.

@foosel

This comment has been minimized.

Show comment
Hide comment
@foosel

foosel Mar 1, 2017

Collaborator

Fixed through merged PR #317

Collaborator

foosel commented Mar 1, 2017

Fixed through merged PR #317

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