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

Spec-compliant localparts for virtual matrix users #1781

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
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 Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ RUN cd freebindfree && make
# Typescript build
FROM node:18 as builder

RUN apt-get update && apt-get install -y node-gyp --no-install-recommends
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know what caused this to flag up? It's weird that you found this without changing the deps around at all.

Copy link
Author

Choose a reason for hiding this comment

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

No idea, it just failed to compile. I wrote this on a brand new VPS. The base images were all fresh downloads. Maybe the upstream image somehow removed node-gyp?

Copy link
Author

Choose a reason for hiding this comment

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

It seems node:18.17 works node:18.18 is missing node-gyp for some reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

We've had this before where minor image versions break (and then because we target a major version, you end up not noticing).

The change looks good then. I think in the future we're just going to have to start pinning versions strictly as we can't be having this.

RUN rm -rf /var/lib/apt/lists/*

WORKDIR /build

COPY src/ /build/src/
Expand Down
5 changes: 5 additions & 0 deletions config.sample.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,11 @@ ircService:
# $NICK => The IRC nick
# $SERVER => The IRC server address (e.g. "irc.example.com")
matrixClients:
# Character mapping for matrix IDs localparts
# 0: No character mapping
# 1: Mapping according to
# https://spec.matrix.org/v1.8/appendices/#mapping-from-other-character-sets
localPartCharacterMapping: 0
# The user ID template to use when creating virtual matrix users. This
# MUST start with an @ and have $NICK somewhere in it.
# Optional. Default: "@$SERVER_$NICK".
Expand Down
4 changes: 4 additions & 0 deletions config.schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,10 @@ properties:
matrixClients:
type: "object"
properties:
localPartCharacterMapping:
type: "integer"
minimum: 0
maximum: 1
Comment on lines +416 to +419
Copy link
Author

@kbleeke kbleeke Oct 21, 2023

Choose a reason for hiding this comment

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

Is there a way to set a default value here so it doesn't break existing configs? Or does that happens automatically?

userTemplate:
type: "string"
pattern: "^@.*\\$NICK"
Expand Down
84 changes: 84 additions & 0 deletions spec/unit/IrcServer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,89 @@ describe("IrcServer", function() {
);
expect(() => {server.getNick("@💩ケ:foobar")}).toThrow();
});
})
describe("getUserLocalpart", function() {
it("does not touch valid characters", function() {
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
matrixClients: {
localPartCharacterMapping: 1,
}
})
);
expect(server.getUserLocalpart("foobar09.-+")).toEqual("irc.foobar_foobar09.-+");
});
it("encodes capital letters", function() {
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
matrixClients: {
localPartCharacterMapping: 1,
}
})
);
expect(server.getUserLocalpart("foOBaR_09.-+")).toEqual("irc.foobar_fo_o_ba_r__09.-+");
});
it("encodes invalid characters", function() {
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
matrixClients: {
localPartCharacterMapping: 1,
}
})
);
expect(server.getUserLocalpart("foobar=[m]")).toEqual("irc.foobar_foobar=3d=5bm=5d");
});
it("encodes both capital letters and invalid chars", function() {
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
matrixClients: {
localPartCharacterMapping: 1,
}
})
);
expect(server.getUserLocalpart("f_oObAr=[m]")).toEqual("irc.foobar_f__o_ob_ar=3d=5bm=5d");
});
});
describe("getNickFromUserId", function() {
it("does not touch valid characters", function() {
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
matrixClients: {
localPartCharacterMapping: 1,
}
})
);
expect(server.getNickFromUserId("irc.foobar_foobar09.-+")).toEqual("foobar09.-+");
});
it("encodes capital letters", function() {
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
matrixClients: {
localPartCharacterMapping: 1,
}
})
);
expect(server.getNickFromUserId("irc.foobar_fo_o_ba_r__09.-+")).toEqual("foOBaR_09.-+");
});
it("decodes invalid characters", function() {
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
matrixClients: {
localPartCharacterMapping: 1,
}
})
);
expect(server.getNickFromUserId("irc.foobar_foobar=3d=5bm=5d")).toEqual("foobar=[m]");
});
it("encodes both capital letters and invalid chars", function() {
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
matrixClients: {
localPartCharacterMapping: 1,
}
})
);
expect(server.getNickFromUserId("irc.foobar_f__o_ob_ar=3d=5bm=5d")).toEqual("f_oObAr=[m]");
});
});
});
25 changes: 21 additions & 4 deletions src/irc/IrcServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export interface IrcServerConfig {
federate: boolean;
};
matrixClients: {
localPartCharacterMapping: number,
userTemplate: string;
displayName: string;
joinAttempts: number;
Expand Down Expand Up @@ -533,6 +534,14 @@ export class IrcServer {
}

public getUserLocalpart(nick: string): string {

if (this.config.matrixClients.localPartCharacterMapping == 1) {
// https://spec.matrix.org/v1.8/appendices/#mapping-from-other-character-sets
nick = nick.replaceAll(/[A-Z_]/g, (c) => "_" + c.toLowerCase());
nick = nick.replaceAll(/[^a-z0-9\.\_\-\/+]/g,
(c) => "=" + c.charCodeAt(0).toString(16).padStart(2, '0'));
}

// the template is just a literal string with special vars; so find/replace
// the vars and strip the @
return renderTemplate(this.config.matrixClients.userTemplate, {
Expand Down Expand Up @@ -572,13 +581,20 @@ export class IrcServer {
if (!match) {
return null;
}
return match[1];

let nick = match[1];
if (this.config.matrixClients.localPartCharacterMapping == 1) {
// https://spec.matrix.org/v1.8/appendices/#mapping-from-other-character-sets
nick = match[1].replaceAll(/=([0-9a-f][0-9a-f])/g,
(_m, g1) => String.fromCharCode(parseInt(g1, 16)));
nick = nick.replaceAll(/_([a-z_])/g, (m, g1) => g1.toUppercase());
}

return nick;
}

public getUserIdFromNick(nick: string): string {
const template = this.config.matrixClients.userTemplate;
return template.replace(/\$NICK/g, nick).replace(/\$SERVER/g, this.domain) +
":" + this.homeserverDomain;
return "@" + this.getUserLocalpart(nick) + ":" + this.homeserverDomain;
}

public getDisplayNameFromNick(nick: string): string {
Expand Down Expand Up @@ -722,6 +738,7 @@ export class IrcServer {
mappings: {},
excludedUsers: [],
matrixClients: {
localPartCharacterMapping: 0,
userTemplate: "@$SERVER_$NICK",
displayName: "$NICK",
joinAttempts: -1,
Expand Down