Skip to content

Commit

Permalink
feat: give up checking tag query key syntax statically
Browse files Browse the repository at this point in the history
  • Loading branch information
jiftechnify committed Oct 4, 2024
1 parent c73d1d2 commit aa9d893
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 27 deletions.
17 changes: 17 additions & 0 deletions packages/kernel/src/nostr.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,27 @@ import {
FilterMatcher,
type NostrEvent,
isNoticeForReqError,
isValidTagQueryKey,
parseR2CMessage,
validateEvent,
} from "./nostr";

describe("isValidTagQueryKey", () => {
test("returns true for valid tag query keys", () => {
const validKeys = ["#p", "#e", "#a", "#I"];
for (const key of validKeys) {
expect(isValidTagQueryKey(key)).toBe(true);
}
});

test("returns false for invalid tag query keys", () => {
const invalidKeys = ["", "p", "#", "#client", "#emoji", "#expiration", "#relay", "#-"];
for (const key of invalidKeys) {
expect(isValidTagQueryKey(key)).toBe(false);
}
});
});

const validEventJSON = `{
"id": "381e2ea15a5b16f4ebdaa68ed3d9a112dc4ea6cc95641ef7eb57f1ec826f07e4",
"pubkey": "d1d1747115d16751a97c239f46ec1703292c3b7e9988b9ebdd4ec4705b15ed44",
Expand Down
22 changes: 17 additions & 5 deletions packages/kernel/src/nostr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,27 @@ export const eventKind = {
} as const;

/**
* Standardized single letter tag names.
* cf. https://github.com/nostr-protocol/nips#standardized-tags
* Keys of filter props for tag queries.
*/
type SingleLetterTags = "a" | "d" | "e" | "g" | "i" | "k" | "l" | "L" | "m" | "p" | "r" | "t" | "x";
type TagQueryKey = `#${string}`;

// [A-Za-z]
const isLatinAlphabet = (charCode: number) =>
(0x41 <= charCode && charCode <= 0x5a) || (0x61 <= charCode && charCode <= 0x7a);

/**
* Keys of filter props for tag queries.
* Checks if the given string is a "queryable" tag name.
*/
const isQueryableTagName = (s: string): boolean =>
s.length === 1 && isLatinAlphabet(s.charCodeAt(0));

/**
* Checks if the given string is a valid key of tag query in filter.
*
* A valid tag query key is a string that starts with `#` and followed by a single Latin alphabet (`[A-Za-z]`).
*/
type TagQueryKey = `#${SingleLetterTags}`;
export const isValidTagQueryKey = (s: string): boolean =>
s.startsWith("#") && isQueryableTagName(s.substring(1));

/**
* Filter for Nostr event subscription.
Expand Down
47 changes: 47 additions & 0 deletions packages/nostr-fetch/src/fetcher.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,16 @@ describe.concurrent("NostrFetcher", () => {
assert(evs.every(({ content }) => content.includes("within range")));
});

test("throws error if filter has invalid tag queries", () => {
expect(() => {
fetcher.allEventsIterator(
["wss://healthy/"],
{ "#invalid": ["invalid"], "#malformed": ["malformed"], "#e": ["foo"] },
{},
);
}).toThrow("Filter has invalid tag queries: #invalid, #malformed");
});

test("throws error if time range is invalid", () => {
expect(() => {
fetcher.allEventsIterator(["wss://healthy/"], {}, { since: 1, until: 0 });
Expand Down Expand Up @@ -416,6 +426,16 @@ describe.concurrent("NostrFetcher", () => {
});

describe.concurrent("fetchAllEvents", () => {
test("throws error if filter has invalid tag queries", async () => {
await expect(() =>
fetcher.fetchAllEvents(
["wss://healthy/"],
{ "#invalid": ["invalid"], "#malformed": ["malformed"], "#e": ["foo"] },
{},
),
).rejects.toThrow("Filter has invalid tag queries: #invalid, #malformed");
});

test("throws error if time range is invalid", async () => {
await expect(() =>
fetcher.fetchAllEvents(["wss://healthy/"], {}, { since: 1, until: 0 }),
Expand Down Expand Up @@ -446,6 +466,16 @@ describe.concurrent("NostrFetcher", () => {
});

describe.concurrent("fetchLatestEvents", () => {
test("throws error if filter has invalid tag queries", async () => {
await expect(() =>
fetcher.fetchLatestEvents(
["wss://healthy/"],
{ "#invalid": ["invalid"], "#malformed": ["malformed"], "#e": ["foo"] },
100,
),
).rejects.toThrow("Filter has invalid tag queries: #invalid, #malformed");
});

test("throws error if limit <= 0", async () => {
await expect(() => fetcher.fetchLatestEvents(["wss://healthy/"], {}, 0)).rejects.toThrow(
'"limit" should be positive number',
Expand Down Expand Up @@ -518,6 +548,23 @@ describe.concurrent("NostrFetcher", () => {
});

describe.concurrent("fetchLatestEventsPerKey", () => {
test("throws error if keyName is an invalid tag query key", () => {
expect(() => {
fetcher.fetchLatestEventsPerKey("#invalid", [], { "#e": ["foo"] }, 100);
}).toThrow("Specified key '#invalid' is invalid tag query key");
});

test("throws error if otherFilter has invalid tag queries", () => {
expect(() => {
fetcher.fetchLatestEventsPerKey(
"authors",
[],
{ "#invalid": ["invalid"], "#malformed": ["malformed"], "#e": ["foo"] },
100,
);
}).toThrow("Filter has invalid tag queries: #invalid, #malformed");
});

const pkA = pubkeyFromAuthorName("alice");
const pkB = pubkeyFromAuthorName("bob");
const pkC = pubkeyFromAuthorName("cat");
Expand Down
47 changes: 28 additions & 19 deletions packages/nostr-fetch/src/fetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
isFetchTillEoseAbortedSignal,
isFetchTillEoseFailedSignal,
} from "@nostr-fetch/kernel/fetcherBackend";
import type { NostrEvent } from "@nostr-fetch/kernel/nostr";
import { type NostrEvent, isValidTagQueryKey } from "@nostr-fetch/kernel/nostr";
import { abbreviate, currUnixtimeSec, normalizeRelayUrlSet } from "@nostr-fetch/kernel/utils";

import { DefaultFetcherBackend } from "./fetcherBackend";
Expand All @@ -24,6 +24,7 @@ import {
type RelayCapabilityChecker,
assertReq,
checkIfNonEmpty,
checkIfTagQueriesAreValid,
checkIfTimeRangeIsValid,
checkIfTrue,
createdAtDesc,
Expand Down Expand Up @@ -423,14 +424,11 @@ export class NostrFetcher {
options: AllEventsIterOptions<SeenOn> = {},
): AsyncIterable<NostrEventExt<SeenOn>> {
assertReq(
{ relayUrls, timeRangeFilter },
{ relayUrls, filter, timeRangeFilter },
[
checkIfNonEmpty((r) => r.relayUrls, "warn", "Specify at least 1 relay URL"),
checkIfTimeRangeIsValid(
(r) => r.timeRangeFilter,
"error",
"Invalid time range (since > until)",
),
checkIfTagQueriesAreValid((r) => r.filter, "error"),
checkIfTimeRangeIsValid((r) => r.timeRangeFilter, "error"),
],
this.#debugLogger,
);
Expand Down Expand Up @@ -608,14 +606,11 @@ export class NostrFetcher {
options: FetchAllOptions<SeenOn> = {},
): Promise<NostrEventExt<SeenOn>[]> {
assertReq(
{ relayUrls, timeRangeFilter },
{ relayUrls, filter, timeRangeFilter },
[
checkIfNonEmpty((r) => r.relayUrls, "warn", "Specify at least 1 relay URL"),
checkIfTimeRangeIsValid(
(r) => r.timeRangeFilter,
"error",
"Invalid time range (since > until)",
),
checkIfTagQueriesAreValid((r) => r.filter, "error"),
checkIfTimeRangeIsValid((r) => r.timeRangeFilter, "error"),
],
this.#debugLogger,
);
Expand Down Expand Up @@ -668,9 +663,10 @@ export class NostrFetcher {
options: FetchLatestOptions<SeenOn> = {},
): Promise<NostrEventExt<SeenOn>[]> {
assertReq(
{ relayUrls, limit },
{ relayUrls, filter, limit },
[
checkIfNonEmpty((r) => r.relayUrls, "warn", "Specify at least 1 relay URL"),
checkIfTagQueriesAreValid((r) => r.filter, "error"),
checkIfTrue((r) => r.limit > 0, "error", '"limit" should be positive number'),
],
this.#debugLogger,
Expand Down Expand Up @@ -960,13 +956,26 @@ export class NostrFetcher {
public fetchLatestEventsPerKey<K extends FetchFilterKeyName, SeenOn extends boolean = false>(
keyName: K,
keysAndRelays: KeysAndRelays<K>,
otherFilter: Omit<FetchFilter, K>,
otherFilter: FetchFilter,
limit: number,
options: FetchLatestOptions<SeenOn> = {},
): AsyncIterable<NostrEventListWithKey<K, SeenOn>> {
assertReq(
{ limit },
[checkIfTrue((r) => r.limit > 0, "error", '"limit" should be positive number')],
{ limit, keyName, otherFilter },
[
checkIfTrue((r) => r.limit > 0, "error", '"limit" should be positive number'),
checkIfTrue(
(r) => !r.keyName.startsWith("#") || isValidTagQueryKey(r.keyName),
"error",
`Specified key '${keyName}' is invalid tag query key`,
),
checkIfTagQueriesAreValid((r) => r.otherFilter, "error"),
checkIfTrue(
({ keyName, otherFilter }) => !(keyName in otherFilter),
"warn",
`'${keyName}' field in "otherFilter" will be ignored because it is specified as main key`,
),
],
this.#debugLogger,
);

Expand All @@ -989,7 +998,7 @@ export class NostrFetcher {
async *#fetchLatestEventPerKeyBody<K extends FetchFilterKeyName, SeenOn extends boolean = false>(
keyName: K,
keysAndRelays: KeysAndRelays<K>,
otherFilter: Omit<FetchFilter, K>,
otherFilter: FetchFilter,
limit: number,
options: Required<FetchLatestOptions<SeenOn>>,
): AsyncIterable<NostrEventListWithKey<K, SeenOn>> {
Expand Down Expand Up @@ -1218,7 +1227,7 @@ export class NostrFetcher {
public async *fetchLastEventPerKey<K extends FetchFilterKeyName, SeenOn extends boolean = false>(
keyName: K,
keysAndRelays: KeysAndRelays<K>,
otherFilter: Omit<FetchFilter, K>,
otherFilter: FetchFilter,
options: FetchLatestOptions<SeenOn> = {},
): AsyncIterable<NostrEventWithKey<K, SeenOn>> {
const finalOpts = {
Expand Down
23 changes: 20 additions & 3 deletions packages/nostr-fetch/src/fetcherHelper.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { DebugLogger } from "@nostr-fetch/kernel/debugLogger";
import type { NostrFetcherCommonOptions } from "@nostr-fetch/kernel/fetcherBackend";
import { type NostrEvent, querySupportedNips } from "@nostr-fetch/kernel/nostr";
import { type NostrEvent, isValidTagQueryKey, querySupportedNips } from "@nostr-fetch/kernel/nostr";
import { normalizeRelayUrlSet } from ".";
import {
type FetchFilter,
type FetchFilterKeyElem,
type FetchFilterKeyName,
type FetchStats,
Expand Down Expand Up @@ -71,7 +72,6 @@ export function checkIfNonEmpty<T, U>(
export function checkIfTimeRangeIsValid<T>(
getTimeRange: (req: T) => { since?: number; until?: number },
severity: "error" | "warn",
msg: string,
): (req: T) => AssertionResult {
return (req: T) => {
const { since, until } = getTimeRange(req);
Expand All @@ -80,7 +80,24 @@ export function checkIfTimeRangeIsValid<T>(
// time range is always valid if at least one of the bounds is unbounded.
return { severity: "none" };
}
return since <= until ? { severity: "none" } : { severity, msg };
return since <= until
? { severity: "none" }
: { severity, msg: "Invalid time range (since > until)" };
};
}

export function checkIfTagQueriesAreValid<T>(
getFilter: (req: T) => FetchFilter,
severity: "error" | "warn",
): (req: T) => AssertionResult {
return (req: T) => {
const invalidTqks = Object.keys(getFilter(req)).filter(
(k) => k.startsWith("#") && !isValidTagQueryKey(k),
);
if (invalidTqks.length > 0) {
return { severity, msg: `Filter has invalid tag queries: ${invalidTqks.join(", ")}` };
}
return { severity: "none" };
};
}

Expand Down

0 comments on commit aa9d893

Please sign in to comment.