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

Remove spec non-compliant extended glob format #3423

Merged
merged 4 commits into from
May 31, 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
78 changes: 0 additions & 78 deletions spec/unit/pushprocessor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,51 +97,6 @@ describe("NotificationService", function () {
pattern: "foo*bar",
rule_id: "foobar",
},
{
actions: [
"notify",
{
set_tweak: "sound",
value: "default",
},
{
set_tweak: "highlight",
},
],
enabled: true,
pattern: "p[io]ng",
rule_id: "pingpong",
},
{
actions: [
"notify",
{
set_tweak: "sound",
value: "default",
},
{
set_tweak: "highlight",
},
],
enabled: true,
pattern: "I ate [0-9] pies",
rule_id: "pies",
},
{
actions: [
"notify",
{
set_tweak: "sound",
value: "default",
},
{
set_tweak: "highlight",
},
],
enabled: true,
pattern: "b[!ai]ke",
rule_id: "bakebike",
},
],
override: [
{
Expand Down Expand Up @@ -289,39 +244,6 @@ describe("NotificationService", function () {
expect(actions.tweaks.highlight).toEqual(true);
});

// TODO: This is not spec compliant behaviour.
//
// See https://spec.matrix.org/v1.5/client-server-api/#conditions-1 which
// describes pattern should glob:
//
// 1. * matches 0 or more characters;
// 2. ? matches exactly one character
it("should bing on character group ([abc]) bing words.", function () {
clokep marked this conversation as resolved.
Show resolved Hide resolved
testEvent.event.content!.body = "Ping!";
let actions = pushProcessor.actionsForEvent(testEvent);
expect(actions.tweaks.highlight).toEqual(true);
testEvent.event.content!.body = "Pong!";
actions = pushProcessor.actionsForEvent(testEvent);
expect(actions.tweaks.highlight).toEqual(true);
});

// TODO: This is not spec compliant behaviour. (See above.)
it("should bing on character range ([a-z]) bing words.", function () {
testEvent.event.content!.body = "I ate 6 pies";
const actions = pushProcessor.actionsForEvent(testEvent);
expect(actions.tweaks.highlight).toEqual(true);
});

// TODO: This is not spec compliant behaviour. (See above.)
it("should bing on character negation ([!a]) bing words.", function () {
testEvent.event.content!.body = "boke";
let actions = pushProcessor.actionsForEvent(testEvent);
expect(actions.tweaks.highlight).toEqual(true);
testEvent.event.content!.body = "bake";
actions = pushProcessor.actionsForEvent(testEvent);
expect(actions.tweaks.highlight).toEqual(false);
});

it("should not bing on room server ACL changes", function () {
testEvent = utils.mkEvent({
type: EventType.RoomServerAcl,
Expand Down
17 changes: 17 additions & 0 deletions spec/unit/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import {
sortEventsByLatestContentTimestamp,
safeSet,
MapWithDefault,
globToRegexp,
escapeRegExp,
} from "../../src/utils";
import { logger } from "../../src/logger";
import { mkMessage } from "../test-utils/test-utils";
Expand Down Expand Up @@ -725,4 +727,19 @@ describe("utils", function () {
await utils.immediate();
});
});

describe("escapeRegExp", () => {
it("should escape XYZ", () => {
expect(escapeRegExp("[FIT-Connect Zustelldienst \\(Testumgebung\\)]")).toMatchInlineSnapshot(
`"\\[FIT-Connect Zustelldienst \\\\\\(Testumgebung\\\\\\)\\]"`,
);
});
});

describe("globToRegexp", () => {
it("should not explode when given regexes as globs", () => {
const result = globToRegexp("[FIT-Connect Zustelldienst \\(Testumgebung\\)]");
expect(result).toMatchInlineSnapshot(`"\\[FIT-Connect Zustelldienst \\\\\\(Testumgebung\\\\\\)\\]"`);
});
});
});
2 changes: 1 addition & 1 deletion src/models/invites-ignorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class IgnoredInvites {
}
let regexp: RegExp;
try {
regexp = new RegExp(globToRegexp(glob, false));
regexp = new RegExp(globToRegexp(glob));
} catch (ex) {
// Assume invalid event.
continue;
Expand Down
29 changes: 8 additions & 21 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,27 +357,14 @@ export function escapeRegExp(string: string): string {
return string.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
}

export function globToRegexp(glob: string, extended = false): string {
// From
// https://github.com/matrix-org/synapse/blob/abbee6b29be80a77e05730707602f3bbfc3f38cb/synapse/push/__init__.py#L132
// Because micromatch is about 130KB with dependencies,
// and minimatch is not much better.
const replacements: [RegExp, string | ((substring: string, ...args: any[]) => string)][] = [
[/\\\*/g, ".*"],
[/\?/g, "."],
];
if (!extended) {
replacements.push([
/\\\[(!|)(.*)\\]/g,
(_match: string, neg: string, pat: string): string =>
["[", neg ? "^" : "", pat.replace(/\\-/, "-"), "]"].join(""),
]);
}
return replacements.reduce(
// https://github.com/microsoft/TypeScript/issues/30134
(pat, args) => (args ? pat.replace(args[0], args[1] as any) : pat),
escapeRegExp(glob),
);
/**
* Converts Matrix glob-style string to a regular expression
* https://spec.matrix.org/v1.7/appendices/#glob-style-matching
* @param glob - Matrix glob-style string
* @returns regular expression
*/
export function globToRegexp(glob: string): string {
return escapeRegExp(glob).replace(/\\\*/g, ".*").replace(/\?/g, ".");
}

export function ensureNoTrailingSlash(url: string): string;
Expand Down