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

fix: do not overwrite config.port from URL, if it's already set #9305

Merged
merged 1 commit into from
Feb 17, 2021

Conversation

ahwayakchih
Copy link
Contributor

If URL was set to something like http://example.com:8080, and port
was set to 4567, keep listening on port 4567 and keep linking through
URL that was specified.
This allows to listen on port 4567, while having NGINX (or any proxy)
set to listen on port 8080 and route traffic to port 4567.
So NodeBB can be "hidden" behind proxy while URL can still contain
non-standard port, i.e., port different than 80 and 443.

If URL was set to something like `http://example.com:8080`, and port
was set to 4567, keep listening on port 4567 and keep linking through
URL that was specified.
This allows to listen on port 4567, while having NGINX (or any proxy)
set to listen on port 8080 and route traffic to port 4567.
So NodeBB can be "hidden" behind proxy while URL can still contain
non-standard port, i.e., port different than 80 and 443.
@CLAassistant
Copy link

CLAassistant commented Feb 16, 2021

CLA assistant check
All committers have signed the CLA.

@julianlam
Copy link
Member

Hm... changing it here seems to not be the correct approach, I think.

If I understand your use case correctly, you want to have NodeBB listen on port 4567, but have the URL set to domain.tld:8080.
The problem as I understand is, NodeBB is instead taking the port from the URL instead of what is defined in port. Let me know if I am mistaken...

@julianlam
Copy link
Member

Nevermind, I am mistaken, NodeBB does handle it correctly on startup, your change is fixing the overwrite on the install step 👍

@julianlam julianlam merged commit 34096b7 into NodeBB:master Feb 17, 2021
julianlam added a commit that referenced this pull request Feb 17, 2021
@ahwayakchih
Copy link
Contributor Author

Yes, only setup was overwriting my config. After config is fixed, everything works just fine :).
Thanks!

@ahwayakchih ahwayakchih deleted the fix-setup-port-overwrite branch February 17, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants