Skip to content

Commit c481a11

Browse files
ZetazzzZetazzz
committed
fixed review issues
1 parent 10ce51a commit c481a11

16 files changed

Lines changed: 321 additions & 126 deletions

File tree

‎graphile/graphile-cache/src/graphile-cache.ts‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -248,34 +248,38 @@ export const closeAllCaches = async (verbose = false): Promise<void> => {
248248
if (closePromise.promise) return closePromise.promise;
249249

250250
closePromise.promise = (async () => {
251-
if (verbose) log.info('Closing all server caches...');
251+
try {
252+
if (verbose) log.info('Closing all server caches...');
252253

253-
// Collect all entries and dispose them properly
254-
const entries = [...graphileCache.entries()];
254+
// Collect all entries and dispose them properly
255+
const entries = [...graphileCache.entries()];
255256

256-
// Mark all as manual evictions
257-
for (const [key] of entries) {
258-
manualEvictionKeys.add(key);
259-
}
257+
// Mark all as manual evictions
258+
for (const [key] of entries) {
259+
manualEvictionKeys.add(key);
260+
}
260261

261-
const disposePromises = entries.map(([key, entry]) =>
262-
disposeEntry(entry, key)
263-
);
262+
const disposePromises = entries.map(([key, entry]) =>
263+
disposeEntry(entry, key)
264+
);
264265

265-
// Wait for all disposals to complete
266-
await Promise.allSettled(disposePromises);
266+
// Wait for all disposals to complete
267+
await Promise.allSettled(disposePromises);
267268

268-
// Clear the cache after disposal (dispose callback will no-op due to disposedKeys)
269-
graphileCache.clear();
269+
// Clear the cache after disposal (dispose callback will no-op due to disposedKeys)
270+
graphileCache.clear();
270271

271-
// Clear disposed keys tracking after full cleanup
272-
disposedKeys.clear();
273-
manualEvictionKeys.clear();
272+
// Clear disposed keys tracking after full cleanup
273+
disposedKeys.clear();
274+
manualEvictionKeys.clear();
274275

275-
// Close pg pools
276-
await pgCache.close();
276+
// Close pg pools
277+
await pgCache.close();
277278

278-
if (verbose) log.success('All caches disposed.');
279+
if (verbose) log.success('All caches disposed.');
280+
} finally {
281+
closePromise.promise = null;
282+
}
279283
})();
280284

281285
return closePromise.promise;

‎graphile/graphile-misc-plugins/src/PublicKeySignature.ts‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ function validateIdentifier(name: string, label: string): void {
2323
}
2424
}
2525

26+
const MAX_PUBLIC_KEY_LENGTH = 256;
27+
const MAX_MESSAGE_LENGTH = 4096;
28+
const MAX_SIGNATURE_LENGTH = 1024;
29+
2630
export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig): GraphileConfig.Plugin => {
2731
const {
2832
schema,
@@ -90,23 +94,29 @@ export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig):
9094
const $combined = object({ input: $input, withPgClient: $withPgClient });
9195

9296
return lambda($combined, async ({ input, withPgClient }: any) => {
97+
if (!input.publicKey || typeof input.publicKey !== 'string' || input.publicKey.length > MAX_PUBLIC_KEY_LENGTH) {
98+
throw new Error('INVALID_PUBLIC_KEY');
99+
}
100+
93101
return withPgClient(null, async (pgClient: any) => {
94102
await pgClient.query('BEGIN');
95103
try {
96104
await pgQueryWithContext({
97105
client: pgClient,
98106
context: { role: 'anonymous' },
99-
query: `SELECT * FROM "${schema}".${sign_up_with_key}($1)`,
100-
variables: [input.publicKey]
107+
query: `SELECT * FROM "${schema}"."${sign_up_with_key}"($1)`,
108+
variables: [input.publicKey],
109+
skipTransaction: true
101110
});
102111

103112
const {
104113
rows: [{ [sign_in_request_challenge]: message }]
105114
} = await pgQueryWithContext({
106115
client: pgClient,
107116
context: { role: 'anonymous' },
108-
query: `SELECT * FROM "${schema}".${sign_in_request_challenge}($1)`,
109-
variables: [input.publicKey]
117+
query: `SELECT * FROM "${schema}"."${sign_in_request_challenge}"($1)`,
118+
variables: [input.publicKey],
119+
skipTransaction: true
110120
});
111121

112122
await pgClient.query('COMMIT');
@@ -125,14 +135,19 @@ export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig):
125135
const $combined = object({ input: $input, withPgClient: $withPgClient });
126136

127137
return lambda($combined, async ({ input, withPgClient }: any) => {
138+
if (!input.publicKey || typeof input.publicKey !== 'string' || input.publicKey.length > MAX_PUBLIC_KEY_LENGTH) {
139+
throw new Error('INVALID_PUBLIC_KEY');
140+
}
141+
128142
return withPgClient(null, async (pgClient: any) => {
129143
const {
130144
rows: [{ [sign_in_request_challenge]: message }]
131145
} = await pgQueryWithContext({
132146
client: pgClient,
133147
context: { role: 'anonymous' },
134-
query: `SELECT * FROM "${schema}".${sign_in_request_challenge}($1)`,
135-
variables: [input.publicKey]
148+
query: `SELECT * FROM "${schema}"."${sign_in_request_challenge}"($1)`,
149+
variables: [input.publicKey],
150+
skipTransaction: true
136151
});
137152

138153
if (!message) throw new Error('NO_ACCOUNT_EXISTS');
@@ -155,6 +170,16 @@ export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig):
155170
return lambda($combined, async ({ input, withPgClient }: any) => {
156171
const { publicKey, message, signature: _signature } = input;
157172

173+
if (!publicKey || typeof publicKey !== 'string' || publicKey.length > MAX_PUBLIC_KEY_LENGTH) {
174+
throw new Error('INVALID_PUBLIC_KEY');
175+
}
176+
if (!message || typeof message !== 'string' || message.length > MAX_MESSAGE_LENGTH) {
177+
throw new Error('INVALID_MESSAGE');
178+
}
179+
if (!_signature || typeof _signature !== 'string' || _signature.length > MAX_SIGNATURE_LENGTH) {
180+
throw new Error('INVALID_SIGNATURE');
181+
}
182+
158183
// TODO: Re-implement crypto verification (e.g. using interchainJS).
159184
// const network = Networks[crypto_network];
160185
// const result = verifyMessage(message, publicKey, signature, network);
@@ -166,8 +191,9 @@ export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig):
166191
await pgQueryWithContext({
167192
client: pgClient,
168193
context: { role: 'anonymous' },
169-
query: `SELECT * FROM "${schema}".${sign_in_record_failure}($1)`,
170-
variables: [publicKey]
194+
query: `SELECT * FROM "${schema}"."${sign_in_record_failure}"($1)`,
195+
variables: [publicKey],
196+
skipTransaction: true
171197
});
172198
throw new Error('BAD_SIGNIN');
173199
}
@@ -180,8 +206,9 @@ export const PublicKeySignature = (pubkey_challenge: PublicKeyChallengeConfig):
180206
} = await pgQueryWithContext({
181207
client: pgClient,
182208
context: { role: 'anonymous' },
183-
query: `SELECT * FROM "${schema}".${sign_in_with_challenge}($1, $2)`,
184-
variables: [publicKey, message]
209+
query: `SELECT * FROM "${schema}"."${sign_in_with_challenge}"($1, $2)`,
210+
variables: [publicKey, message],
211+
skipTransaction: true
185212
});
186213

187214
if (!token?.access_token) throw new Error('BAD_SIGNIN');

‎graphile/graphile-plugin-connection-filter-postgis/__tests__/operators.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ describe('PgConnectionArgFilterPostgisOperatorsPlugin', () => {
279279

280280
describe('SQL generation', () => {
281281
describe('function-based operators', () => {
282-
it('generates correct SQL for public schema', () => {
282+
it('generates schema-qualified SQL for public schema', () => {
283283
const { registered } = runPlugin({ schemaName: 'public' });
284284
const containsOp = registered.find(r => r.operatorName === 'contains');
285285
expect(containsOp).toBeDefined();
@@ -291,7 +291,7 @@ describe('PgConnectionArgFilterPostgisOperatorsPlugin', () => {
291291
operatorName: 'contains'
292292
});
293293
const compiled = sql.compile(result);
294-
expect(compiled.text).toBe('"st_contains"("col", "val")');
294+
expect(compiled.text).toBe('"public"."st_contains"("col", "val")');
295295
});
296296

297297
it('generates schema-qualified SQL for non-public schema', () => {
@@ -321,7 +321,7 @@ describe('PgConnectionArgFilterPostgisOperatorsPlugin', () => {
321321
operatorName: 'intersects3D'
322322
});
323323
const compiled = sql.compile(result);
324-
expect(compiled.text).toBe('"st_3dintersects"("a", "b")');
324+
expect(compiled.text).toBe('"public"."st_3dintersects"("a", "b")');
325325
});
326326
});
327327

‎graphile/graphile-plugin-connection-filter-postgis/src/plugin.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,7 @@ export const PgConnectionArgFilterPostgisOperatorsPlugin: GraphileConfig.Plugin
244244
// Process function-based operators
245245
for (const [fn, baseTypes, operatorName, description] of functionSpecs) {
246246
for (const baseType of baseTypes) {
247-
const sqlGisFunction = schemaName === 'public'
248-
? sql.identifier(fn.toLowerCase())
249-
: sql.identifier(schemaName, fn.toLowerCase());
247+
const sqlGisFunction = sql.identifier(schemaName, fn.toLowerCase());
250248

251249
allSpecs.push({
252250
typeNames: gqlTypeNamesByBase[baseType],
@@ -270,7 +268,7 @@ export const PgConnectionArgFilterPostgisOperatorsPlugin: GraphileConfig.Plugin
270268
}
271269

272270
// Sort by operator name for deterministic schema output
273-
allSpecs.sort((a, b) => (a.operatorName > b.operatorName ? 1 : -1));
271+
allSpecs.sort((a, b) => a.operatorName.localeCompare(b.operatorName));
274272

275273
// Register each operator with the connection filter plugin
276274
for (const spec of allSpecs) {

‎graphile/graphile-postgis/src/plugins/codec.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ function buildGisCodec(
106106
* The result is cast to ::text so PostgreSQL sends it as a string,
107107
* which fromPg then JSON.parses.
108108
*/
109+
// NOTE: `fragment` is evaluated 4 times in the SQL. This is acceptable because
110+
// PostGraphile v5 always passes simple column references here.
111+
// If this changes, consider wrapping in a LATERAL subexpression.
109112
castFromPg(fragment: SQL): SQL {
110113
return sql.fragment`(case when (${fragment}) is null then null else json_build_object(
111114
'__gisType', ${sql.identifier(schemaName, 'geometrytype')}(${fragment}),
@@ -122,7 +125,15 @@ function buildGisCodec(
122125
* We normalize __gisType from PostGIS uppercase to our mixed-case format.
123126
*/
124127
fromPg(value: string): GisFieldValue {
125-
const parsed = JSON.parse(value);
128+
let parsed: any;
129+
try {
130+
parsed = JSON.parse(value);
131+
} catch (e) {
132+
throw new Error(
133+
`Failed to parse PostGIS geometry value: ${e instanceof Error ? e.message : String(e)}. ` +
134+
`Raw value (first 200 chars): ${String(value).slice(0, 200)}`
135+
);
136+
}
126137
if (parsed && typeof parsed === 'object' && parsed.__gisType) {
127138
parsed.__gisType = normalizeGisType(parsed.__gisType);
128139
}

0 commit comments

Comments
 (0)