-
Notifications
You must be signed in to change notification settings - Fork 35
Fix NGINX reverse proxy configuration #45
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
Conversation
| rewrite /qbt(.*) $1 break; | ||
| proxy_pass http://127.0.0.1:30000/; | ||
| # Rewrite redirect responses to include the prefix | ||
| proxy_redirect /qbt/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proxy_redirect expects two arguments, isn't?
https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_redirect
| return 301 /qbt/; | ||
| } | ||
| location ~^/qbt/ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the specified syntax:
| location ~^/qbt/ { | |
| location ~ ^/qbt/ { |
| # Redirect to link with trailing slash, to allow correct relative path resolution | ||
| location = /qbt { | ||
| return 301 /qbt/; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From https://nginx.org/en/docs/http/ngx_http_core_module.html#location
In response to a request with URI equal to this string, but without the trailing slash, a permanent redirect with the code 301 will be returned to the requested URI with the slash appended.
So it seems this manual 301 redirection is not needed, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Chocobo1
IMO, this PR is yet another iteration of "stretching the point".
Personally, I'm not going to approve any related PR until I see real "steps to reproduce" the problem (under normal conditions) that they're supposed to solve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I'm not going to approve any related PR until I see real "steps to reproduce" the problem (under normal conditions) that they're supposed to solve.
Sure. But I'm curious to see their reply, such as why didn't it work before and why this change fixes it (with examples). We can make the judgement afterwards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. But I'm curious to see their reply, such as why didn't it work before and why this change fixes it (with examples). We can make the judgement afterwards.
Yes, this is exactly what I tried to get from people in all related Issues, but I didn't get anything significant except abstract reasoning, or unsubstantiated stories about the existence of some problem.
Edited NGINX config as per qbittorrent/qBittorrent#5693 (comment)
qbittorrent/qBittorrent#23467 (comment)