Skip to content

Commit 734fea0

Browse files
ferhatelmasferhatelmas
authored
fix: escape wildcard characters for list (#882)
add escapeLike helper for escaping % and _ in like sql operator use it in * object list v2 non-delimiter prefix filtering * list v1 non-name sort * s3 multipart list * bucket / analytics search filters
1 parent 32ccf47 commit 734fea0

File tree

5 files changed

+477
-9
lines changed

5 files changed

+477
-9
lines changed

‎src/storage/database/knex.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ import {
2828

2929
const { isMultitenant } = getConfig()
3030

31+
export function escapeLike(str: string) {
32+
return str.replace(/([%_])/g, '\\$1')
33+
}
34+
3135
/**
3236
* Database
3337
* the only source of truth for interacting with the storage database
@@ -150,7 +154,7 @@ export class StorageKnexDB implements Database {
150154
.whereNull('deleted_at')
151155

152156
if (options?.search !== undefined && options.search.length > 0) {
153-
query.where('name', 'like', `%${options.search}%`)
157+
query.where('name', 'like', `%${escapeLike(options.search)}%`)
154158
}
155159

156160
if (options?.sortColumn !== undefined) {
@@ -392,7 +396,7 @@ export class StorageKnexDB implements Database {
392396
query.orderBy(knex.raw(`name COLLATE "C"`), sortOrder)
393397

394398
if (options?.prefix) {
395-
query.where('name', 'like', `${options.prefix}%`)
399+
query.where('name', 'like', `${escapeLike(options.prefix)}%`)
396400
}
397401

398402
if (options?.startAfter && !options?.nextToken) {
@@ -482,7 +486,7 @@ export class StorageKnexDB implements Database {
482486
const query = knex.from<Bucket>('buckets').select(selectColumns)
483487

484488
if (options?.search !== undefined && options.search.length > 0) {
485-
query.where('name', 'ilike', `%${options.search}%`)
489+
query.where('name', 'ilike', `%${escapeLike(options.search)}%`)
486490
}
487491

488492
if (options?.sortColumn !== undefined) {
@@ -526,7 +530,7 @@ export class StorageKnexDB implements Database {
526530
query.orderBy(knex.raw('key COLLATE "C", created_at'))
527531

528532
if (options?.prefix) {
529-
query.where('key', 'ilike', `${options.prefix}%`)
533+
query.where('key', 'ilike', `${escapeLike(options.prefix)}%`)
530534
}
531535

532536
if (options?.nextUploadKeyToken && !options.nextUploadToken) {
@@ -543,7 +547,7 @@ export class StorageKnexDB implements Database {
543547
const result = await knex
544548
.raw('select * from storage.list_multipart_uploads_with_delimiter(?,?,?,?,?,?)', [
545549
bucketId,
546-
options?.prefix,
550+
options?.prefix ? escapeLike(options.prefix) : options?.prefix,
547551
options?.deltimeter,
548552
options?.maxKeys,
549553
options?.nextUploadKeyToken || '',
@@ -878,15 +882,22 @@ export class StorageKnexDB implements Database {
878882

879883
async searchObjects(bucketId: string, prefix: string, options: SearchObjectOption) {
880884
return this.runQuery('SearchObjects', async (knex, signal) => {
885+
const sortColumn = options.sortBy?.column ?? 'name'
886+
const shouldEscapePattern = sortColumn !== 'name'
887+
const safePrefix = shouldEscapePattern ? escapeLike(prefix) : prefix
888+
const safeSearch = shouldEscapePattern
889+
? escapeLike(options.search || '')
890+
: options.search || ''
891+
881892
const result = await knex
882893
.raw<{ rows: Obj[] }>('select * from storage.search(?,?,?,?,?,?,?,?)', [
883-
prefix,
894+
safePrefix,
884895
bucketId,
885896
options.limit || 100,
886-
prefix.split('/').length,
897+
safePrefix.split('/').length,
887898
options.offset || 0,
888-
options.search || '',
889-
options.sortBy?.column ?? 'name',
899+
safeSearch,
900+
sortColumn,
890901
options.sortBy?.order ?? 'asc',
891902
])
892903
.abortOnSignal(signal)

‎src/test/object-list-v2.test.ts‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict'
22

3+
import { randomUUID } from 'node:crypto'
34
import { ListObjectsV2Result } from '@storage/object'
45
import { FastifyInstance } from 'fastify'
56
import { Knex } from 'knex'
@@ -575,3 +576,127 @@ describe('objects - list v2 sorting tests', () => {
575576
})
576577
}
577578
})
579+
580+
const LIST_V2_WILDCARD_BUCKET = `list-v2-wildcard-${randomUUID()}`
581+
582+
describe('objects - list v2 prefix wildcard handling', () => {
583+
beforeAll(async () => {
584+
appInstance = app()
585+
await appInstance.inject({
586+
method: 'POST',
587+
url: `/bucket`,
588+
headers: {
589+
authorization: `Bearer ${serviceKey}`,
590+
},
591+
payload: {
592+
name: LIST_V2_WILDCARD_BUCKET,
593+
},
594+
})
595+
await appInstance.close()
596+
})
597+
598+
afterAll(async () => {
599+
appInstance = app()
600+
await appInstance.inject({
601+
method: 'POST',
602+
url: `/bucket/${LIST_V2_WILDCARD_BUCKET}/empty`,
603+
headers: {
604+
authorization: `Bearer ${serviceKey}`,
605+
},
606+
})
607+
608+
await appInstance.inject({
609+
method: 'DELETE',
610+
url: `/bucket/${LIST_V2_WILDCARD_BUCKET}`,
611+
headers: {
612+
authorization: `Bearer ${serviceKey}`,
613+
},
614+
})
615+
616+
await appInstance.close()
617+
})
618+
619+
test('treats % as a literal character in list-v2 prefix filters', async () => {
620+
const runId = Date.now().toString(36)
621+
const firstObject = `percent-${runId}/first.txt`
622+
const secondObject = `percent-${runId}/second.txt`
623+
624+
await appInstance.inject({
625+
method: 'POST',
626+
url: `/object/${LIST_V2_WILDCARD_BUCKET}/${firstObject}`,
627+
payload: createUpload('first.txt', 'first'),
628+
headers: {
629+
authorization: `Bearer ${serviceKey}`,
630+
},
631+
})
632+
633+
await appInstance.inject({
634+
method: 'POST',
635+
url: `/object/${LIST_V2_WILDCARD_BUCKET}/${secondObject}`,
636+
payload: createUpload('second.txt', 'second'),
637+
headers: {
638+
authorization: `Bearer ${serviceKey}`,
639+
},
640+
})
641+
642+
const response = await appInstance.inject({
643+
method: 'POST',
644+
url: `/object/list-v2/${LIST_V2_WILDCARD_BUCKET}`,
645+
payload: {
646+
with_delimiter: false,
647+
prefix: '%',
648+
limit: 100,
649+
},
650+
headers: {
651+
authorization: `Bearer ${serviceKey}`,
652+
},
653+
})
654+
655+
expect(response.statusCode).toBe(200)
656+
const data = response.json<ListObjectsV2Result>()
657+
expect(data.folders).toHaveLength(0)
658+
expect(data.objects).toHaveLength(0)
659+
})
660+
661+
test('treats _ as a literal character in list-v2 prefix filters', async () => {
662+
const runId = randomUUID()
663+
const literalMatch = `wild_${runId}/hit.txt`
664+
const wildcardOnlyMatch = `wildX${runId}/miss.txt`
665+
666+
await appInstance.inject({
667+
method: 'POST',
668+
url: `/object/${LIST_V2_WILDCARD_BUCKET}/${literalMatch}`,
669+
payload: createUpload('hit.txt', 'hit'),
670+
headers: {
671+
authorization: `Bearer ${serviceKey}`,
672+
},
673+
})
674+
675+
await appInstance.inject({
676+
method: 'POST',
677+
url: `/object/${LIST_V2_WILDCARD_BUCKET}/${wildcardOnlyMatch}`,
678+
payload: createUpload('miss.txt', 'miss'),
679+
headers: {
680+
authorization: `Bearer ${serviceKey}`,
681+
},
682+
})
683+
684+
const response = await appInstance.inject({
685+
method: 'POST',
686+
url: `/object/list-v2/${LIST_V2_WILDCARD_BUCKET}`,
687+
payload: {
688+
with_delimiter: false,
689+
prefix: `wild_${runId}/`,
690+
limit: 100,
691+
},
692+
headers: {
693+
authorization: `Bearer ${serviceKey}`,
694+
},
695+
})
696+
697+
expect(response.statusCode).toBe(200)
698+
const data = response.json<ListObjectsV2Result>()
699+
expect(data.folders).toHaveLength(0)
700+
expect(data.objects.map((obj) => obj.name)).toEqual([literalMatch])
701+
})
702+
})

‎src/test/object.test.ts‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,6 +2576,132 @@ describe('testing list objects', () => {
25762576
expect(responseJSON[0].name).toBe('sadcat-upload23.png')
25772577
expect(responseJSON[1].name).toBe('sadcat-upload.png')
25782578
})
2579+
2580+
test('list-v1 should treat % as a literal character when using non-name sorting', async () => {
2581+
const runId = randomUUID()
2582+
const bucketName = 'bucket2'
2583+
const objectNames = [`percent-${runId}/first.txt`, `percent-${runId}/second.txt`]
2584+
2585+
const seedTx = await getSuperuserPostgrestClient()
2586+
await seedTx.from<Obj>('objects').insert(
2587+
objectNames.map((name, idx) => ({
2588+
bucket_id: bucketName,
2589+
name,
2590+
owner: '317eadce-631a-4429-a0bb-f19a7a517b4a',
2591+
version: `${runId}-${idx}`,
2592+
metadata: {
2593+
eTag: `${runId}-${idx}`,
2594+
size: idx + 1,
2595+
mimetype: 'text/plain',
2596+
},
2597+
}))
2598+
)
2599+
await seedTx.commit()
2600+
tnx = undefined
2601+
2602+
try {
2603+
const response = await appInstance.inject({
2604+
method: 'POST',
2605+
url: '/object/list/bucket2',
2606+
payload: {
2607+
prefix: '%',
2608+
limit: 100,
2609+
offset: 0,
2610+
sortBy: {
2611+
column: 'created_at',
2612+
order: 'asc',
2613+
},
2614+
},
2615+
headers: {
2616+
authorization: `Bearer ${await serviceKeyAsync}`,
2617+
},
2618+
})
2619+
2620+
expect(response.statusCode).toBe(200)
2621+
const responseJSON = response.json()
2622+
expect(responseJSON).toHaveLength(0)
2623+
} finally {
2624+
const cleanupTx = await getSuperuserPostgrestClient()
2625+
await withDeleteEnabled(cleanupTx, async (db) => {
2626+
await db
2627+
.from<Obj>('objects')
2628+
.where({ bucket_id: bucketName })
2629+
.whereIn('name', objectNames)
2630+
.delete()
2631+
})
2632+
await cleanupTx.commit()
2633+
tnx = undefined
2634+
}
2635+
})
2636+
2637+
test('list-v1 should treat _ as a literal character when using non-name sorting', async () => {
2638+
const runId = randomUUID()
2639+
const bucketName = 'bucket2'
2640+
const literalMatch = `wild_${runId}/hit.txt`
2641+
const wildcardOnlyMatch = `wildX${runId}/miss.txt`
2642+
2643+
const seedTx = await getSuperuserPostgrestClient()
2644+
await seedTx.from<Obj>('objects').insert([
2645+
{
2646+
bucket_id: bucketName,
2647+
name: literalMatch,
2648+
owner: '317eadce-631a-4429-a0bb-f19a7a517b4a',
2649+
version: `${runId}-literal`,
2650+
metadata: {
2651+
eTag: `${runId}-literal`,
2652+
size: 1,
2653+
mimetype: 'text/plain',
2654+
},
2655+
},
2656+
{
2657+
bucket_id: bucketName,
2658+
name: wildcardOnlyMatch,
2659+
owner: '317eadce-631a-4429-a0bb-f19a7a517b4a',
2660+
version: `${runId}-wildcard`,
2661+
metadata: {
2662+
eTag: `${runId}-wildcard`,
2663+
size: 2,
2664+
mimetype: 'text/plain',
2665+
},
2666+
},
2667+
])
2668+
await seedTx.commit()
2669+
tnx = undefined
2670+
2671+
try {
2672+
const response = await appInstance.inject({
2673+
method: 'POST',
2674+
url: '/object/list/bucket2',
2675+
payload: {
2676+
prefix: `wild_${runId}/`,
2677+
limit: 100,
2678+
offset: 0,
2679+
sortBy: {
2680+
column: 'created_at',
2681+
order: 'asc',
2682+
},
2683+
},
2684+
headers: {
2685+
authorization: `Bearer ${await serviceKeyAsync}`,
2686+
},
2687+
})
2688+
2689+
expect(response.statusCode).toBe(200)
2690+
const responseJSON = response.json<{ name: string }[]>()
2691+
expect(responseJSON.map((obj) => obj.name)).toEqual(['hit.txt'])
2692+
} finally {
2693+
const cleanupTx = await getSuperuserPostgrestClient()
2694+
await withDeleteEnabled(cleanupTx, async (db) => {
2695+
await db
2696+
.from<Obj>('objects')
2697+
.where({ bucket_id: bucketName })
2698+
.whereIn('name', [literalMatch, wildcardOnlyMatch])
2699+
.delete()
2700+
})
2701+
await cleanupTx.commit()
2702+
tnx = undefined
2703+
}
2704+
})
25792705
})
25802706

25812707
describe('x-robots-tag header', () => {

0 commit comments

Comments
 (0)