also looking at this
fix(client): write the cache-key delimiter as \0, not a raw NUL byte
#1102
2 files
+83
-1
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -5904,7 +5904,7 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 5904 | 5904 | // `where id = $1` with [1] and with [2] would otherwise share one |
| 5905 | 5905 | // cache entry and the second query would return the first's rows. |
| 5906 | 5906 | const cacheKey = useCache |
| 5907 | ? `${String(finalQuery)} ${JSON.stringify(whereParams)}` | |
| 5907 | ? `${String(finalQuery)}\0${JSON.stringify(whereParams)}` | |
| 5908 | 5908 | : '' |
| 5909 | 5909 | if (useCache) { |
| 5910 | 5910 | const cached = queryCache.get(cacheKey) |
| @@ -0,0 +1,82 @@ | ||
| 1 | /** | |
| 2 | * Mechanical checks on the source itself. | |
| 3 | * | |
| 4 | * `src/client.ts` carried a literal NUL byte (0x00) for two months, written as | |
| 5 | * the raw control character instead of the `\0` escape, as the delimiter in a | |
| 6 | * cache key: | |
| 7 | * | |
| 8 | * const cacheKey = `${String(finalQuery)}<0x00>${JSON.stringify(params)}` | |
| 9 | * | |
| 10 | * The runtime string was correct, so nothing failed and nothing caught it. What | |
| 11 | * it broke was every tool that reads the file. A single NUL makes grep classify | |
| 12 | * the whole file as binary, and grep then reports "binary file matches" instead | |
| 13 | * of matching lines — ripgrep, VS Code search and agent search tools inherit | |
| 14 | * the same behaviour. The repo's largest and most-edited file, 8000+ lines, | |
| 15 | * silently returned nothing for ordinary searches: | |
| 16 | * | |
| 17 | * $ grep -c "async paginate" src/client.ts | |
| 18 | * 0 # exit 1, while `grep -a` finds it | |
| 19 | * | |
| 20 | * A search that returns nothing is indistinguishable from a symbol that does | |
| 21 | * not exist, so this actively misleads anyone — human or otherwise — trying to | |
| 22 | * find code in it. | |
| 23 | * | |
| 24 | * The escape and the raw byte produce byte-identical transpiler output, so | |
| 25 | * there is never a reason to commit the raw byte. | |
| 26 | */ | |
| 27 | ||
| 28 | import { readdirSync, readFileSync, statSync } from 'node:fs' | |
| 29 | import { extname, join, relative, resolve } from 'node:path' | |
| 30 | import { describe, expect, it } from 'bun:test' | |
| 31 | ||
| 32 | const SRC_DIR = resolve(import.meta.dir, '../src') | |
| 33 | ||
| 34 | function sourceFiles(dir: string): string[] { | |
| 35 | const out: string[] = [] | |
| 36 | for (const entry of readdirSync(dir, { withFileTypes: true })) { | |
| 37 | const full = join(dir, entry.name) | |
| 38 | if (entry.isDirectory()) { | |
| 39 | out.push(...sourceFiles(full)) | |
| 40 | continue | |
| 41 | } | |
| 42 | if (statSync(full).isFile() && (extname(full) === '.ts' || extname(full) === '.tsx')) | |
| 43 | out.push(full) | |
| 44 | } | |
| 45 | return out | |
| 46 | } | |
| 47 | ||
| 48 | describe('source hygiene', () => { | |
| 49 | it('no source file contains a raw control character that makes it read as binary', () => { | |
| 50 | const offenders: string[] = [] | |
| 51 | ||
| 52 | for (const file of sourceFiles(SRC_DIR)) { | |
| 53 | const text = readFileSync(file, 'utf8') | |
| 54 | for (let i = 0; i < text.length; i++) { | |
| 55 | const code = text.charCodeAt(i) | |
| 56 | // Everything below 0x20 except tab (9), newline (10) and carriage | |
| 57 | // return (13). NUL is the one that triggers the binary heuristic, but | |
| 58 | // no other C0 control belongs in a source file either. | |
| 59 | if (code < 0x20 && code !== 9 && code !== 10 && code !== 13) { | |
| 60 | const line = text.slice(0, i).split('\n').length | |
| 61 | offenders.push( | |
| 62 | `${relative(SRC_DIR, file)}:${line} contains U+${code.toString(16).padStart(4, '0').toUpperCase()} — ` | |
| 63 | + `write it as an escape (\\0, \\u0001, …) instead of the raw byte`, | |
| 64 | ) | |
| 65 | break | |
| 66 | } | |
| 67 | } | |
| 68 | } | |
| 69 | ||
| 70 | expect(offenders).toEqual([]) | |
| 71 | }) | |
| 72 | ||
| 73 | /** | |
| 74 | * Guards the specific delimiter rather than only the byte: swapping it for a | |
| 75 | * printable character would reintroduce the collision d7c3a16 fixed, where a | |
| 76 | * SQL tail and a parameter head could run together into the same cache key. | |
| 77 | */ | |
| 78 | it('the query cache key still separates SQL from params with a NUL', () => { | |
| 79 | const client = readFileSync(join(SRC_DIR, 'client.ts'), 'utf8') | |
| 80 | expect(client).toContain('`${String(finalQuery)}\\0${JSON.stringify(whereParams)}`') | |
| 81 | }) | |
| 82 | }) | |