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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .config/example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.)


# ┌──────────────────────────┐
#───┘ PostgreSQL configuration └────────────────────────────────

Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/boot/master.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export async function masterMain() {
await spawnWorkers(config.clusterLimit);
}

bootLogger.succ(`Now listening on port ${config.port} on ${config.url}`, null, true);
bootLogger.succ(`Now listening on ${config.host ? `[${config.host}]:` : ''}${config.port} for ${config.url}`, null, true);
}

function showEnvironment(): void {
Expand Down
2 changes: 2 additions & 0 deletions packages/backend/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export type Source = {
feedback_url?: string;
url: string;
port: number;
host: string;
disableHsts?: boolean;
db: {
host: string;
Expand Down Expand Up @@ -125,6 +126,7 @@ export function loadConfig() {
config.url = url.origin;

config.port = config.port ?? parseInt(process.env.PORT ?? '', 10);
config.host = config.host ?? process.env.HOST;

mixin.version = meta.version;
mixin.host = url.host;
Expand Down
6 changes: 3 additions & 3 deletions packages/backend/src/server/ServerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,10 @@ export class ServerService {
fastify.server.on('error', err => {
switch ((err as any).code) {
case 'EACCES':
this.logger.error(`You do not have permission to listen on port ${this.config.port}.`);
this.logger.error(`You do not have permission to listen on port ${this.config.port}${this.config.host ? ` of host ${this.config.host}` : ''}`);
break;
case 'EADDRINUSE':
this.logger.error(`Port ${this.config.port} is already in use by another process.`);
this.logger.error(`Port ${this.config.port}${this.config.host ? ` on ${this.config.host}` : ''} is already in use by another process.`);
break;
default:
this.logger.error(err);
Expand All @@ -195,6 +195,6 @@ export class ServerService {
}
});

fastify.listen({ port: this.config.port, host: '0.0.0.0' });
fastify.listen({ port: this.config.port, host: this.config.host });
}
}
11 changes: 6 additions & 5 deletions packages/backend/test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const _dirname = dirname(_filename);

const config = loadConfig();
export const port = config.port;
export const host = config.host || "localhost";

export const api = async (endpoint: string, params: any, me?: any) => {
endpoint = endpoint.replace(/^\//, '');
Expand All @@ -28,7 +29,7 @@ export const api = async (endpoint: string, params: any, me?: any) => {
} : {};

try {
const res = await got<string>(`http://localhost:${port}/api/${endpoint}`, {
const res = await got<string>(`http://${host}:${port}/api/${endpoint}`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Expand Down Expand Up @@ -66,7 +67,7 @@ export const request = async (path: string, params: any, me?: any): Promise<{ bo
i: me.token,
} : {};

const res = await fetch(`http://localhost:${port}/${path}`, {
const res = await fetch(`http://${host}:${port}/${path}`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Expand Down Expand Up @@ -123,7 +124,7 @@ export const uploadFile = async (user: any, _path?: string): Promise<any> => {
formData.append('file', fs.createReadStream(absPath));
formData.append('force', 'true');

const res = await got<string>(`http://localhost:${port}/api/drive/files/create`, {
const res = await got<string>(`http://${host}:${port}/api/drive/files/create`, {
method: 'POST',
body: formData,
retry: {
Expand Down Expand Up @@ -160,7 +161,7 @@ export const uploadUrl = async (user: any, url: string) => {

export function connectStream(user: any, channel: string, listener: (message: Record<string, any>) => any, params?: any): Promise<WebSocket> {
return new Promise((res, rej) => {
const ws = new WebSocket(`ws://localhost:${port}/streaming?i=${user.token}`);
const ws = new WebSocket(`ws://${host}:${port}/streaming?i=${user.token}`);

ws.on('open', () => {
ws.on('message', data => {
Expand Down Expand Up @@ -222,7 +223,7 @@ export const waitFire = async (user: any, channel: string, trgr: () => any, cond
export const simpleGet = async (path: string, accept = '*/*'): Promise<{ status?: number, type?: string, location?: string }> => {
// node-fetchだと3xxを取れない
return await new Promise((resolve, reject) => {
const req = http.request(`http://localhost:${port}${path}`, {
const req = http.request(`http://${host}:${port}${path}`, {
headers: {
Accept: accept,
},
Expand Down