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

Wrong URL for File Load/Download Buttons through NGINX Reverse Proxy Subfolder #1391

Closed
mirage335 opened this issue Jun 26, 2016 · 5 comments
Labels
bug Issue describes a bug done Done but not yet released
Milestone

Comments

@mirage335
Copy link

mirage335 commented Jun 26, 2016

What were you doing?

Clicking the Load/Download buttons.

When NGINX is used as a reverse proxy, redirecting a subfolder like /octoprint to an octopi installation, the links provided by file load/download buttons are wrong.

This is posted when the reverse proxy is used to redirect a subfolder.
https://local:port/octoprint/https,https://local:port/octoprint/downloads/files/local/file.gcode

This posted when the reverse proxy is used with a root directory (ie. location "/"), or when the octopi server is used directly.
https://local:port/octoprint/downloads/files/local/file.gcode

What did you expect to happen?

Actually Load/Download files.

What happened instead?

Nothing, due to wrong URL POST.

Branch & Commit or Version of OctoPrint

Version: 1.2.13 (master branch)

Printer model & used firmware incl. version

TazStiff, Marlin Firmware

Browser and Version of Browser, Operating System running Browser

Firefox.

Link to octoprint.log

Nothing notable.

Link to contents of terminal tab or serial.log

Nothing notable.

Link to contents of Javascript console in the browser

POST
XHR
https://mirage335-base.soaringindustries.space:7740/octoprint/https,https://mirage335-base.soaringindustries.space:7740/octoprint/api/files/local/diagrid_s_20160618-122805_1.2.10-dev.gcode [HTTP/1.1 404 NOT FOUND 231ms]

Screenshot(s) showing the problem:

Revelant portions of my NGINX configuration are as follows, based on the example at https://github.com/foosel/OctoPrint/wiki/Reverse-proxy-configuration-examples .

location /octoprint/ {
    proxy_pass https://remote:port/;
    #proxy_set_header Host $host;
    proxy_set_header Host $http_host;
    proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
    proxy_set_header X-Scheme $scheme;
    proxy_set_header X-Script-Name /octoprint;
}

location /octoprint/sockjs {
    proxy_pass https://remote:port/sockjs; # NO trailing slash here!
    proxy_http_version 1.1;
    proxy_redirect off;
    proxy_set_header Upgrade $http_upgrade;
    proxy_set_header Connection "upgrade";
    proxy_set_header Host $http_host;
    proxy_set_header X-Real-IP $remote_addr;
}

I have read the FAQ.

@GitIssueBot
Copy link

Hi @mirage335,

It looks like there is some information missing from your bug report that will be needed in order to solve the problem. Please take a look at the Contribution Guidelines which will provide you with a template to fill out here so that your bug report is ready to be investigated (I promise I'll go away then too!).

If you did not intend to report a bug, please take special note of the title format to use as described in the Contribution Guidelines.

I'm marking this one now as needing some more information. Please understand that if you do not provide that information within the next two weeks (until 2016-07-10 16:20) I'll close this ticket so it doesn't clutter the bug tracker. This is nothing personal, so please just be considerate and help the maintainers solve this problem quickly by following the guidelines linked above. Remember, the less time the devs have to spend running after information on tickets, the more time they have to actually solve problems and add awesome new features. Thank you!

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being, so don't expect any replies from me :) Your ticket is read by humans too, I'm just not one of them.

@GitIssueBot GitIssueBot added incomplete Issue template has not been fully filled out, no further processing until fixed triage This issue needs triage and removed incomplete Issue template has not been fully filled out, no further processing until fixed labels Jun 26, 2016
@foosel
Copy link
Member

foosel commented Jun 29, 2016

At first I couldn't reproduce this at all. http nginx -> OctoPrint, http nginx -> http haproxy -> OctoPrint, https nginx -> OctoPrint, https nginx -> http haproxy -> OctoPrint, all worked flawlessly. Then I tried https nginx -> https haproxy -> OctoPrint and finally could reproduce.

The problem is actually two fold:

  1. OctoPrint uses what ever is in the X-Scheme header as protocol prefix for generated external URLs. Headers might contain multiple values however (e.g. https,https), which isn't a valid protocol and will cause the browser to interpret such an URL as a relative one to the URL of the page it's contained in.
  2. The haproxy configuration shipped with OctoPi will always add a X-Scheme header, even if one is already there from a prior reverse proxy. That will produce X-Scheme: https,https headers if the first and second reverse proxy are both setup to add an X-Scheme header and are both accessed through https. E.g. https nginx -> https haproxy -> OctoPrint.

Problem number 1 I've solved in 0376bc4. Problem number 2 needs to be solved in the intermediary reverse proxy, by ensuring an X-Scheme header is only added if none already exists on the request. For that a PR against OctoPi needs to be made in order to adjust haproxy.cfg accordingly. A first test I've done looks like

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

should do it.

Since only problem number 1 was actually an OctoPrint problem, I'm marking this one as solved now. The fix mentioned above is already in the maintenance branch and I'll also merge it into the devel branch, making sure it's part of the next release.

@foosel foosel added bug Issue describes a bug done Done but not yet released and removed triage This issue needs triage labels Jun 29, 2016
@foosel foosel modified the milestone: 1.2.14 Jun 29, 2016
@mirage335
Copy link
Author

Can now confirm problem solved. Much thanks!

@foosel
Copy link
Member

foosel commented Jul 28, 2016

Merged to master and released with 1.2.14

@foosel foosel closed this as completed Jul 28, 2016
@mirage335
Copy link
Author

Awesome, thanks for everything!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue describes a bug done Done but not yet released
Projects
None yet
Development

No branches or pull requests

3 participants