Skip to content
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
20 changes: 13 additions & 7 deletions apps/aggregator/src/routes/xrpc/searchPackages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,15 +347,21 @@ const CAPABILITY_FILTER_SQL = `
`;

/** Quote a user-supplied search string for FTS5 MATCH. FTS5 treats `"`,
* `*`, `(`, `)`, `.`, `:`, `^`, `+`, `-` as syntax. The simplest robust
* escape is to wrap the whole query as a single phrase string and double
* any embedded quotes. This loses prefix-search functionality
* (`"foo*"` is treated literally) but is safe and sufficient for v1; if
* advanced query syntax becomes a product feature we'll layer a parsed
* mode on top. */
* `*`, `(`, `)`, `.`, `:`, `^`, `+`, `-` as syntax, so each whitespace-separated
* term becomes its own quoted phrase with embedded quotes doubled, and user
* input can never form an operator. The last term gets a `*` so a partly typed
* word still matches (`newslet` finds `newsletter`). Terms with no letters or
* digits are dropped: the tokenizer would turn them into empty phrases. */
const FTS_QUOTE_RE = /"/g;
const WHITESPACE_RE = /\s+/;
const WORD_CHAR_RE = /[\p{L}\p{N}]/u;
function quoteFtsQuery(raw: string): string {
return `"${raw.replace(FTS_QUOTE_RE, '""')}"`;
const terms = raw
.split(WHITESPACE_RE)
.filter((term) => WORD_CHAR_RE.test(term))
.map((term) => `"${term.replace(FTS_QUOTE_RE, '""')}"`);
if (terms.length === 0) return '""';
return `${terms.join(" ")}*`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[suggestion] The function appends its own * after the last quoted term, but it doesn't remove user-supplied * characters from the term itself. An input like foo* would produce "foo*"* — a literal * inside a phrase plus a phrase-level prefix *. I can't run SQLite here to verify, but that looks like a potential FTS5 syntax error and would weaken the existing "FTS-unsafe chars don't 500" guarantee. Since the code already owns prefixing, consider stripping * from terms before quoting.

Suggested change
return `${terms.join(" ")}*`;
function quoteFtsQuery(raw: string): string {
const terms = raw
.replace(/\*/g, "")
.split(WHITESPACE_RE)
.filter((term) => WORD_CHAR_RE.test(term))
.map((term) => `"${term.replace(FTS_QUOTE_RE, '""')}"`);
if (terms.length === 0) return '""';
return `${terms.join(" ")}*`;
}

}

function clampLimit(raw: number | undefined): number {
Expand Down
31 changes: 27 additions & 4 deletions apps/aggregator/test/read-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,29 @@ describe("searchPackages", () => {
expect(overlap).toEqual([]);
});

it("matches a partly typed last word as a prefix", async () => {
await seedPackage({ slug: "bulletin", name: "Bulletin", description: "Email newsletters" });
await seedPackage({ slug: "gallery", name: "Gallery", description: "Image gallery" });

const res = await SELF.fetch(
`https://test/xrpc/${NSID.aggregatorSearchPackages}?q=${encodeURIComponent("email newslet")}`,
);
const body = (await res.json()) as { packages: Array<{ slug: string }> };

expect(body.packages.map((p) => p.slug)).toEqual(["bulletin"]);
});

it("returns no matches for a query with no word characters", async () => {
await seedPackage({ slug: "demo", name: "Demo" });

const res = await SELF.fetch(
`https://test/xrpc/${NSID.aggregatorSearchPackages}?q=${encodeURIComponent('( * "')}`,
);

expect(res.status).toBe(200);
await expect(res.json()).resolves.toEqual({ packages: [] });
});

it("doesn't blow up on FTS-unsafe query chars (defensive quoting)", async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[suggestion] The new regression test validates prefix matching inside a multi-word query (email newslet). Adding a single-term test for the exact reported case — e.g., q=newslet returning bulletin — would pin that a lone partly typed word now matches and guard against the prefix * being applied to the wrong part of the joined output.

Suggested change
it("doesn't blow up on FTS-unsafe query chars (defensive quoting)", async () => {
it("matches a single partly typed word as a prefix", async () => {
await seedPackage({ slug: "bulletin", name: "Bulletin", description: "Email newsletters" });
const res = await SELF.fetch(
`https://test/xrpc/${NSID.aggregatorSearchPackages}?q=${encodeURIComponent("newslet")}`,
);
const body = (await res.json()) as { packages: Array<{ slug: string }> };
expect(body.packages.map((p) => p.slug)).toEqual(["bulletin"]);
});

await seedPackage({ slug: "demo", name: "Demo" });
const res = await SELF.fetch(
Expand All @@ -699,10 +722,10 @@ describe("searchPackages", () => {
await seedPackage({ slug: "alpha", name: "Alpha" });
await seedPackage({ slug: "beta", name: "Beta" });
// `alpha OR beta` would match both packages if `OR` were interpreted
// as the FTS5 operator. With proper escaping the whole string is one
// literal phrase that can't possibly appear in either record's
// indexed text → zero matches. A buggy escape that stripped the
// quotes would return *both* packages.
// as the FTS5 operator. With proper escaping every term is a literal
// that must appear, and neither record contains all three → zero
// matches. A buggy escape that stripped the quotes would return
// *both* packages.
const res = await SELF.fetch(
`https://test/xrpc/${NSID.aggregatorSearchPackages}?q=${encodeURIComponent("alpha OR beta")}`,
);
Expand Down
Loading