Skip to content

Commit 0685e84

Browse files
committed
fix: more tests for gaps
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
1 parent 473efb5 commit 0685e84

File tree

5 files changed

+501
-6
lines changed

5 files changed

+501
-6
lines changed

src/http/routes/object/getSignedURL.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ export default async function routes(fastify: FastifyInstance) {
6767
const objectName = request.params['*']
6868
const { expiresIn } = request.body
6969

70-
const urlPath = request.url.split('?').shift()
7170
const imageTransformationEnabled = await isImageTransformationEnabled(request.tenantId)
7271

7372
const transformationOptions = imageTransformationEnabled
@@ -82,7 +81,7 @@ export default async function routes(fastify: FastifyInstance) {
8281

8382
const signedURL = await request.storage
8483
.from(bucketName)
85-
.signObjectUrl(objectName, urlPath as string, expiresIn, transformationOptions)
84+
.signObjectUrl(objectName, expiresIn, transformationOptions)
8685

8786
return response.status(200).send({ signedURL })
8887
}

src/storage/object.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,6 @@ export class ObjectStorage {
705705
*/
706706
async signObjectUrl(
707707
objectName: string,
708-
url: string,
709708
expiresIn: number,
710709
metadata?: Record<string, string | object | undefined>
711710
) {
@@ -722,8 +721,7 @@ export class ObjectStorage {
722721
// make sure it's never able to specify a role JWT claim
723722
delete metadata['role']
724723

725-
const urlParts = url.split('/')
726-
const urlToSign = decodeURI(urlParts.splice(3).join('/'))
724+
const urlToSign = `${this.bucketId}/${objectName}`
727725
const { urlSigningKey } = await getJwtSecret(this.db.tenantId)
728726
const token = await signJWT({ url: urlToSign, ...metadata }, urlSigningKey, expiresIn)
729727

@@ -733,8 +731,13 @@ export class ObjectStorage {
733731
urlPath = 'render/image'
734732
}
735733

734+
const encodedUrlToSign = `${encodeURIComponent(this.bucketId)}/${objectName
735+
.split('/')
736+
.map((pathToken) => encodeURIComponent(pathToken))
737+
.join('/')}`
738+
736739
// @todo parse the url properly
737-
return `/${urlPath}/sign/${urlToSign}?token=${token}`
740+
return `/${urlPath}/sign/${encodedUrlToSign}?token=${token}`
738741
}
739742

740743
/**

src/test/object.test.ts

Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { withDeleteEnabled } from './utils/storage'
1818
const { jwtSecret, serviceKeyAsync, tenantId } = getConfig()
1919
const anonKey = process.env.ANON_KEY || ''
2020
const S3Backend = backends.S3Backend
21+
const TEST_OWNER_ID = '317eadce-631a-4429-a0bb-f19a7a517b4a'
2122
let appInstance: FastifyInstance
2223

2324
let tnx: Knex.Transaction | undefined
@@ -35,6 +36,19 @@ async function getSuperuserPostgrestClient() {
3536
return tnx
3637
}
3738

39+
async function seedObjectForRouteTest(name: string, bucketId = 'bucket2') {
40+
const seedTx = await getSuperuserPostgrestClient()
41+
await seedTx.from<Obj>('objects').insert({
42+
bucket_id: bucketId,
43+
name,
44+
owner: TEST_OWNER_ID,
45+
version: `seed-version-${randomUUID()}`,
46+
metadata: { mimetype: 'image/png', size: 1234 },
47+
})
48+
await seedTx.commit()
49+
tnx = undefined
50+
}
51+
3852
useMockObject()
3953
useMockQueue()
4054

@@ -1455,6 +1469,56 @@ describe('testing copy object', () => {
14551469
expect(response.statusCode).toBe(400)
14561470
expect(S3Backend.prototype.copyObject).not.toHaveBeenCalled()
14571471
})
1472+
1473+
test('can copy objects when keys include ASCII URL-reserved characters', async () => {
1474+
const sourceKey = `authenticated/copy-src-${randomUUID()}-q?foo=1&bar=%25+plus;semi:colon,.png`
1475+
const destinationKey = `authenticated/copy-dst-${randomUUID()}-q?foo=2&bar=%25+plus;semi:colon,.png`
1476+
await seedObjectForRouteTest(sourceKey)
1477+
1478+
const response = await appInstance.inject({
1479+
method: 'POST',
1480+
url: '/object/copy',
1481+
headers: {
1482+
authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
1483+
},
1484+
payload: {
1485+
bucketId: 'bucket2',
1486+
sourceKey,
1487+
destinationKey,
1488+
},
1489+
})
1490+
1491+
expect(response.statusCode).toBe(200)
1492+
expect(S3Backend.prototype.copyObject).toBeCalled()
1493+
const jsonResponse = response.json()
1494+
expect(jsonResponse.Key).toBe(`bucket2/${destinationKey}`)
1495+
expect(jsonResponse.name).toBe(destinationKey)
1496+
})
1497+
1498+
test('can copy objects when keys include Unicode and URL-reserved characters', async () => {
1499+
const sourceKey = `authenticated/copy-src-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,.png`
1500+
const destinationKey = `authenticated/copy-dst-${randomUUID()}-éè-中文-🙂-q?foo=2&bar=%25+plus;semi:colon,.png`
1501+
await seedObjectForRouteTest(sourceKey)
1502+
1503+
const response = await appInstance.inject({
1504+
method: 'POST',
1505+
url: '/object/copy',
1506+
headers: {
1507+
authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
1508+
},
1509+
payload: {
1510+
bucketId: 'bucket2',
1511+
sourceKey,
1512+
destinationKey,
1513+
},
1514+
})
1515+
1516+
expect(response.statusCode).toBe(200)
1517+
expect(S3Backend.prototype.copyObject).toBeCalled()
1518+
const jsonResponse = response.json()
1519+
expect(jsonResponse.Key).toBe(`bucket2/${destinationKey}`)
1520+
expect(jsonResponse.name).toBe(destinationKey)
1521+
})
14581522
})
14591523

14601524
/**
@@ -2458,6 +2522,72 @@ describe('testing move object', () => {
24582522
expect(S3Backend.prototype.copyObject).not.toHaveBeenCalled()
24592523
expect(S3Backend.prototype.deleteObject).not.toHaveBeenCalled()
24602524
})
2525+
2526+
test('can move objects when keys include ASCII URL-reserved characters', async () => {
2527+
const sourceKey = `authenticated/move-src-${randomUUID()}-q?foo=1&bar=%25+plus;semi:colon,.png`
2528+
const destinationKey = `authenticated/move-dst-${randomUUID()}-q?foo=2&bar=%25+plus;semi:colon,.png`
2529+
await seedObjectForRouteTest(sourceKey)
2530+
2531+
const response = await appInstance.inject({
2532+
method: 'POST',
2533+
url: '/object/move',
2534+
payload: {
2535+
bucketId: 'bucket2',
2536+
sourceKey,
2537+
destinationKey,
2538+
},
2539+
headers: {
2540+
authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
2541+
},
2542+
})
2543+
2544+
expect(response.statusCode).toBe(200)
2545+
expect(S3Backend.prototype.copyObject).toHaveBeenCalled()
2546+
expect(S3Backend.prototype.deleteObjects).toHaveBeenCalled()
2547+
expect(response.json().message).toBe('Successfully moved')
2548+
2549+
const conn = await getSuperuserPostgrestClient()
2550+
const movedObject = await conn
2551+
.table('objects')
2552+
.select('name')
2553+
.where('bucket_id', 'bucket2')
2554+
.where('name', destinationKey)
2555+
.first()
2556+
expect(movedObject?.name).toBe(destinationKey)
2557+
})
2558+
2559+
test('can move objects when keys include Unicode and URL-reserved characters', async () => {
2560+
const sourceKey = `authenticated/move-src-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,.png`
2561+
const destinationKey = `authenticated/move-dst-${randomUUID()}-éè-中文-🙂-q?foo=2&bar=%25+plus;semi:colon,.png`
2562+
await seedObjectForRouteTest(sourceKey)
2563+
2564+
const response = await appInstance.inject({
2565+
method: 'POST',
2566+
url: '/object/move',
2567+
payload: {
2568+
bucketId: 'bucket2',
2569+
sourceKey,
2570+
destinationKey,
2571+
},
2572+
headers: {
2573+
authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`,
2574+
},
2575+
})
2576+
2577+
expect(response.statusCode).toBe(200)
2578+
expect(S3Backend.prototype.copyObject).toHaveBeenCalled()
2579+
expect(S3Backend.prototype.deleteObjects).toHaveBeenCalled()
2580+
expect(response.json().message).toBe('Successfully moved')
2581+
2582+
const conn = await getSuperuserPostgrestClient()
2583+
const movedObject = await conn
2584+
.table('objects')
2585+
.select('name')
2586+
.where('bucket_id', 'bucket2')
2587+
.where('name', destinationKey)
2588+
.first()
2589+
expect(movedObject?.name).toBe(destinationKey)
2590+
})
24612591
})
24622592

24632593
describe('testing list objects', () => {
@@ -3007,6 +3137,38 @@ describe('Object key names with Unicode characters', () => {
30073137
expect(getResponse.headers['etag']).toBe('abc')
30083138
})
30093139

3140+
test('can upload and read HEAD info with a Unicode and URL-reserved key', async () => {
3141+
const objectName = `head-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.jpg`
3142+
const authorization = `Bearer ${await serviceKeyAsync}`
3143+
const path = './src/test/assets/sadcat.jpg'
3144+
const { size } = fs.statSync(path)
3145+
3146+
const uploadResponse = await appInstance.inject({
3147+
method: 'PUT',
3148+
url: `/object/bucket2/${encodeURIComponent(objectName)}`,
3149+
headers: {
3150+
authorization,
3151+
'Content-Length': size,
3152+
'Content-Type': 'image/jpeg',
3153+
},
3154+
payload: fs.createReadStream(path),
3155+
})
3156+
expect(uploadResponse.statusCode).toBe(200)
3157+
3158+
const headResponse = await appInstance.inject({
3159+
method: 'HEAD',
3160+
url: `/object/authenticated/bucket2/${encodeURIComponent(objectName)}`,
3161+
headers: {
3162+
authorization,
3163+
},
3164+
})
3165+
expect(headResponse.statusCode).toBe(200)
3166+
expect(headResponse.headers['etag']).toBe('abc')
3167+
expect(headResponse.headers['last-modified']).toBeTruthy()
3168+
expect(headResponse.headers['content-length']).toBeTruthy()
3169+
expect(headResponse.headers['cache-control']).toBe('no-cache')
3170+
})
3171+
30103172
test('should not upload if the name contains invalid characters', async () => {
30113173
const invalidObjectName = getInvalidObjectName()
30123174
const authorization = `Bearer ${await serviceKeyAsync}`
@@ -3124,6 +3286,90 @@ describe('Object key names with Unicode characters', () => {
31243286
expect(objectResponse?.name).toBe(objectName)
31253287
})
31263288

3289+
test('can generate and use a signed download URL with Unicode and URL-reserved characters', async () => {
3290+
const objectName = `signed-download-reserved-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png`
3291+
const authorization = `Bearer ${await serviceKeyAsync}`
3292+
3293+
const form = new FormData()
3294+
form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`))
3295+
const uploadHeaders = Object.assign({}, form.getHeaders(), {
3296+
authorization,
3297+
'x-upsert': 'true',
3298+
})
3299+
3300+
const uploadResponse = await appInstance.inject({
3301+
method: 'POST',
3302+
url: `/object/bucket2/${encodeURIComponent(objectName)}`,
3303+
headers: uploadHeaders,
3304+
payload: form,
3305+
})
3306+
expect(uploadResponse.statusCode).toBe(200)
3307+
3308+
const signResponse = await appInstance.inject({
3309+
method: 'POST',
3310+
url: `/object/sign/bucket2/${encodeURIComponent(objectName)}`,
3311+
headers: {
3312+
authorization,
3313+
},
3314+
payload: {
3315+
expiresIn: 60,
3316+
},
3317+
})
3318+
expect(signResponse.statusCode).toBe(200)
3319+
3320+
const signedURL = signResponse.json<{ signedURL: string }>().signedURL
3321+
const signedURLParsed = new URL(signedURL, 'http://localhost')
3322+
const token = signedURLParsed.searchParams.get('token')
3323+
expect(token).toBeTruthy()
3324+
3325+
const getResponse = await appInstance.inject({
3326+
method: 'GET',
3327+
url: `${signedURLParsed.pathname}${signedURLParsed.search}`,
3328+
})
3329+
expect(getResponse.statusCode).toBe(200)
3330+
expect(getResponse.headers['etag']).toBe('abc')
3331+
})
3332+
3333+
test('can generate and use a signed upload URL with Unicode and URL-reserved characters', async () => {
3334+
const objectName = `signed-upload-reserved-${randomUUID()}-éè-中文-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png`
3335+
const authorization = `Bearer ${await serviceKeyAsync}`
3336+
3337+
const signedUploadResponse = await appInstance.inject({
3338+
method: 'POST',
3339+
url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}`,
3340+
headers: {
3341+
authorization,
3342+
},
3343+
})
3344+
expect(signedUploadResponse.statusCode).toBe(200)
3345+
3346+
const token = signedUploadResponse.json<{ token: string }>().token
3347+
expect(token).toBeTruthy()
3348+
3349+
const form = new FormData()
3350+
form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`))
3351+
const uploadResponse = await appInstance.inject({
3352+
method: 'PUT',
3353+
url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}?token=${token}`,
3354+
headers: {
3355+
...form.getHeaders(),
3356+
},
3357+
payload: form,
3358+
})
3359+
expect(uploadResponse.statusCode).toBe(200)
3360+
3361+
const db = await getSuperuserPostgrestClient()
3362+
const objectResponse = await db
3363+
.from<Obj>('objects')
3364+
.select('*')
3365+
.where({
3366+
name: objectName,
3367+
bucket_id: 'bucket2',
3368+
})
3369+
.first()
3370+
expect(objectResponse?.name).toBe(objectName)
3371+
})
3372+
31273373
test('should not generate signed upload URL for invalid key', async () => {
31283374
const invalidObjectName = getInvalidObjectName()
31293375
const authorization = `Bearer ${await serviceKeyAsync}`

src/test/s3-protocol.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2282,6 +2282,60 @@ describe('S3 Protocol', () => {
22822282
}
22832283
})
22842284

2285+
it('can head an object with Unicode and URL-reserved characters in key', async () => {
2286+
const bucketName = await createBucket(client)
2287+
const objectName = `head-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.jpg`
2288+
2289+
await client.send(
2290+
new PutObjectCommand({
2291+
Bucket: bucketName,
2292+
Key: objectName,
2293+
Body: Buffer.alloc(1024),
2294+
})
2295+
)
2296+
2297+
const headResp = await client.send(
2298+
new HeadObjectCommand({
2299+
Bucket: bucketName,
2300+
Key: objectName,
2301+
})
2302+
)
2303+
expect(headResp.$metadata.httpStatusCode).toEqual(200)
2304+
expect(headResp.ETag).toBeTruthy()
2305+
})
2306+
2307+
it('can delete an object with Unicode and URL-reserved characters using DeleteObjectCommand', async () => {
2308+
const bucketName = await createBucket(client)
2309+
const objectName = `delete-one-${randomUUID()}-éè-中文-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.jpg`
2310+
2311+
await client.send(
2312+
new PutObjectCommand({
2313+
Bucket: bucketName,
2314+
Key: objectName,
2315+
Body: Buffer.alloc(1024),
2316+
})
2317+
)
2318+
2319+
await client.send(
2320+
new DeleteObjectCommand({
2321+
Bucket: bucketName,
2322+
Key: objectName,
2323+
})
2324+
)
2325+
2326+
try {
2327+
await client.send(
2328+
new GetObjectCommand({
2329+
Bucket: bucketName,
2330+
Key: objectName,
2331+
})
2332+
)
2333+
throw new Error('Should not reach here')
2334+
} catch (e) {
2335+
expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(404)
2336+
}
2337+
})
2338+
22852339
it('can paginate ListObjectsV2 with Unicode keys', async () => {
22862340
const bucketName = await createBucket(client)
22872341
const keys = [

0 commit comments

Comments
 (0)