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: Allow configuring of the host/address to listen/bind to #9924

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

ledlamp
Copy link
Contributor

@ledlamp ledlamp commented Feb 13, 2023

First of all, the default behavior of node.js server is to listen on all addresses of both IPv4 and IPv6. However, Misskey was hard-coded to listen on "0.0.0.0", which makes node.js listen on IPv4 only.

Second, the administrator should be able to configure the host, just like any other server allows you to do. In production, when the server is behind a reverse proxy, it is a good idea to make the backend listen only on a loopback address, so that it is not publicly exposed. There are also other possible reasons, for example, binding to port 80 of an IPv6 address to connect directly to Cloudflare, without conflicting with other web servers on the same system.

So with this PR, host can be configured just like port, and it defaults to undefined so node.js listens on both stacks, not just IPv4. Closes #7738

@github-actions github-actions bot added packages/backend Server side specific issue/PR 🧪Test labels Feb 13, 2023
@@ -30,6 +30,9 @@ url: https://example.tld/
# The port that your Misskey server should listen on.
port: 3000

# The hostname or address to listen on (default: all)
#host: "127.0.0.1"
Copy link
Member

Choose a reason for hiding this comment

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

The name "host" has a wide range and can be confusing, so how about "listenHost" or something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is okay because I can not think of anything else it could mean in this context. It is simple and consistent with the node.js code.

If it's called "listenHost", then should "port" be called "listenPort"?

Copy link
Member

@saschanaz saschanaz Feb 16, 2023

Choose a reason for hiding this comment

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

Maybe hostname is better? (Oh but there's already host below...)

Copy link
Member

Choose a reason for hiding this comment

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

When people see the property config.host, they don't know what host it refers to.

then should "port" be called "listenPort"?

I think so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what about bind or bindAddress?

Copy link
Member

Choose a reason for hiding this comment

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

This is a server configuration, so IMO host/port shouldn't be too confusing, especially with the comments here.

Copy link
Member

Choose a reason for hiding this comment

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

(But then it could be confusing too because below host means the address the server connects to, and here it's to receive connections. Maybe bind is better then.)

@syuilo syuilo merged commit 3dfe3aa into misskey-dev:develop Feb 22, 2023
@syuilo
Copy link
Member

syuilo commented Feb 22, 2023

👍🏻

syuilo added a commit that referenced this pull request Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Listen address を変更可能にする
3 participants