Skip to content

Commit ece1933

Browse files
author
Jason
committed
Merge branch 'master' of https://github.com/supabase/storage into bugfix/#133_special_characters_in_filename
2 parents c81fa9a + bf91ce2 commit ece1933

File tree

12 files changed

+2554
-2621
lines changed

12 files changed

+2554
-2621
lines changed

‎.eslintrc.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,9 @@ module.exports = {
66
ecmaVersion: 2020, // Allows for the parsing of modern ECMAScript features
77
sourceType: 'module', // Allows for the use of imports
88
},
9+
rules: {
10+
'@typescript-eslint/no-explicit-any': 'warn',
11+
'@typescript-eslint/no-unused-vars': 'warn',
12+
'@typescript-eslint/no-require-imports': 'warn',
13+
},
914
}

‎package-lock.json

Lines changed: 2467 additions & 2516 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
"node": ">= 14.0.0"
2626
},
2727
"dependencies": {
28-
"@aws-sdk/client-s3": "3.633.0",
29-
"@aws-sdk/lib-storage": "3.633.0",
30-
"@aws-sdk/s3-request-presigner": "3.633.0",
28+
"@aws-sdk/client-s3": "3.654.0",
29+
"@aws-sdk/lib-storage": "3.654.0",
30+
"@aws-sdk/s3-request-presigner": "3.654.0",
3131
"@fastify/accepts": "^4.3.0",
3232
"@fastify/multipart": "^8.3.0",
3333
"@fastify/rate-limit": "^7.6.0",
@@ -93,8 +93,8 @@
9393
"@types/pg": "^8.6.4",
9494
"@types/stream-buffers": "^3.0.7",
9595
"@types/xml2js": "^0.4.14",
96-
"@typescript-eslint/eslint-plugin": "^5.12.1",
97-
"@typescript-eslint/parser": "^5.12.1",
96+
"@typescript-eslint/eslint-plugin": "^8.7.0",
97+
"@typescript-eslint/parser": "^8.7.0",
9898
"babel-jest": "^29.2.2",
9999
"esbuild": "0.21.5",
100100
"eslint": "^8.9.0",
@@ -113,7 +113,7 @@
113113
"ts-node-dev": "^1.1.8",
114114
"tsx": "^4.16.0",
115115
"tus-js-client": "^3.1.0",
116-
"typescript": "^5.6.2"
116+
"typescript": "5.2.2"
117117
},
118118
"bin": "./dist/server.js"
119119
}

‎src/config.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ type StorageConfigType = {
2626
storageS3ForcePathStyle?: boolean
2727
storageS3Region: string
2828
storageS3ClientTimeout: number
29-
storageS3UploadTimeout: number
30-
storageS3DownloadTimeout: number
3129
isMultitenant: boolean
3230
jwtSecret: string
3331
jwtAlgorithm: string
@@ -277,15 +275,7 @@ export function getConfig(options?: { reload?: boolean }): StorageConfigType {
277275
getOptionalConfigFromEnv('STORAGE_S3_FORCE_PATH_STYLE', 'GLOBAL_S3_FORCE_PATH_STYLE') ===
278276
'true',
279277
storageS3Region: getOptionalConfigFromEnv('STORAGE_S3_REGION', 'REGION') as string,
280-
storageS3ClientTimeout: Number(
281-
getOptionalConfigFromEnv('STORAGE_S3_CLIENT_TIMEOUT') || `${1000 * 600}` // 10m
282-
),
283-
storageS3DownloadTimeout: Number(
284-
getOptionalConfigFromEnv('STORAGE_S3_DOWNLOAD_TIMEOUT') || `${1000 * 43200}` //12h
285-
),
286-
storageS3UploadTimeout: Number(
287-
getOptionalConfigFromEnv('STORAGE_S3_UPLOAD_TIMEOUT') || `${1000 * 1200}` // 20m
288-
),
278+
storageS3ClientTimeout: Number(getOptionalConfigFromEnv('STORAGE_S3_CLIENT_TIMEOUT') || `0`),
289279

290280
// DB - Migrations
291281
dbAnonRole: getOptionalConfigFromEnv('DB_ANON_ROLE') || 'anon',

‎src/http/plugins/signals.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,13 @@ export const signals = fastifyPlugin(
2222

2323
// Client terminated the request before the body was fully received
2424
res.raw.once('close', () => {
25-
req.signals.response.abort()
25+
const aborted = !res.raw.writableFinished
26+
if (aborted) {
27+
req.signals.response.abort()
2628

27-
if (!req.signals.disconnect.signal.aborted) {
28-
req.signals.disconnect.abort()
29+
if (!req.signals.disconnect.signal.aborted) {
30+
req.signals.disconnect.abort()
31+
}
2932
}
3033
})
3134
})

‎src/http/plugins/tenant-id.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,16 @@ declare module 'fastify' {
77
}
88
}
99

10+
const {
11+
version,
12+
isMultitenant,
13+
tenantId: defaultTenantId,
14+
requestXForwardedHostRegExp,
15+
} = getConfig()
16+
1017
export const tenantId = fastifyPlugin(
1118
async (fastify) => {
12-
const { isMultitenant, tenantId, requestXForwardedHostRegExp } = getConfig()
13-
fastify.decorateRequest('tenantId', tenantId)
19+
fastify.decorateRequest('tenantId', defaultTenantId)
1420
fastify.addHook('onRequest', async (request) => {
1521
if (!isMultitenant || !requestXForwardedHostRegExp) return
1622
const xForwardedHost = request.headers['x-forwarded-host']
@@ -26,6 +32,7 @@ export const tenantId = fastifyPlugin(
2632
tenantId: request.tenantId,
2733
project: request.tenantId,
2834
reqId: request.id,
35+
appVersion: version,
2936
})
3037
})
3138
},

‎src/http/plugins/tracing.ts

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -32,24 +32,28 @@ export const tracing = fastifyPlugin(
3232
fastify.register(traceServerTime)
3333

3434
fastify.addHook('onRequest', async (request) => {
35-
if (isMultitenant && request.tenantId) {
36-
const tenantConfig = await getTenantConfig(request.tenantId)
37-
request.tracingMode = tenantConfig.tracingMode
38-
} else {
39-
request.tracingMode = defaultTracingMode
40-
}
35+
try {
36+
if (isMultitenant && request.tenantId) {
37+
const tenantConfig = await getTenantConfig(request.tenantId)
38+
request.tracingMode = tenantConfig.tracingMode
39+
} else {
40+
request.tracingMode = defaultTracingMode
41+
}
4142

42-
const span = trace.getSpan(context.active())
43+
const span = trace.getSpan(context.active())
4344

44-
if (span) {
45-
// We collect logs only in full,logs,debug mode
46-
if (
47-
tracingEnabled &&
48-
request.tracingMode &&
49-
!['full', 'logs', 'debug'].includes(request.tracingMode)
50-
) {
51-
traceCollector.clearTrace(span.spanContext().traceId)
45+
if (span) {
46+
// We collect logs only in full,logs,debug mode
47+
if (
48+
tracingEnabled &&
49+
request.tracingMode &&
50+
!['full', 'logs', 'debug'].includes(request.tracingMode)
51+
) {
52+
traceCollector.clearTrace(span.spanContext().traceId)
53+
}
5254
}
55+
} catch (e) {
56+
logSchema.error(request.log, 'failed setting tracing mode', { error: e, type: 'tracing' })
5357
}
5458
})
5559
},
@@ -62,12 +66,12 @@ export const traceServerTime = fastifyPlugin(
6266
return
6367
}
6468
fastify.addHook('onResponse', async (request, reply) => {
65-
const traceId = trace.getSpan(context.active())?.spanContext().traceId
69+
try {
70+
const traceId = trace.getSpan(context.active())?.spanContext().traceId
6671

67-
if (traceId) {
68-
const spans = traceCollector.getSpansForTrace(traceId)
69-
if (spans) {
70-
try {
72+
if (traceId) {
73+
const spans = traceCollector.getSpansForTrace(traceId)
74+
if (spans) {
7175
const serverTimingHeaders = spansToServerTimings(spans, reply.statusCode >= 500)
7276

7377
request.serverTimings = serverTimingHeaders
@@ -84,27 +88,30 @@ export const traceServerTime = fastifyPlugin(
8488
.join(',')
8589
reply.header('Server-Timing', httpServerTimes)
8690
}
87-
} catch (e) {
88-
logSchema.error(logger, 'failed parsing server times', { error: e, type: 'otel' })
91+
traceCollector.clearTrace(traceId)
8992
}
90-
91-
traceCollector.clearTrace(traceId)
9293
}
94+
} catch (e) {
95+
logSchema.error(request.log, 'failed tracing on response', { error: e, type: 'tracing' })
9396
}
9497
})
9598

9699
fastify.addHook('onRequestAbort', async (req) => {
97-
const span = trace.getSpan(context.active())
98-
const traceId = span?.spanContext().traceId
100+
try {
101+
const span = trace.getSpan(context.active())
102+
const traceId = span?.spanContext().traceId
99103

100-
span?.setAttribute('req_aborted', true)
104+
span?.setAttribute('req_aborted', true)
101105

102-
if (traceId) {
103-
const spans = traceCollector.getSpansForTrace(traceId)
104-
if (spans) {
105-
req.serverTimings = spansToServerTimings(spans, true)
106+
if (traceId) {
107+
const spans = traceCollector.getSpansForTrace(traceId)
108+
if (spans) {
109+
req.serverTimings = spansToServerTimings(spans, true)
110+
}
111+
traceCollector.clearTrace(traceId)
106112
}
107-
traceCollector.clearTrace(traceId)
113+
} catch (e) {
114+
logSchema.error(logger, 'failed parsing server times on abort', { error: e, type: 'otel' })
108115
}
109116
})
110117
},

‎src/http/routes/tus/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const {
3535
storageS3Endpoint,
3636
storageS3ForcePathStyle,
3737
storageS3Region,
38-
storageS3UploadTimeout,
38+
storageS3ClientTimeout,
3939
tusUrlExpiryMs,
4040
tusPath,
4141
tusPartSize,
@@ -69,7 +69,7 @@ function createTusStore() {
6969
requestHandler: new NodeHttpHandler({
7070
...agent,
7171
connectionTimeout: 5000,
72-
requestTimeout: storageS3UploadTimeout,
72+
requestTimeout: storageS3ClientTimeout,
7373
}),
7474
bucket: storageS3Bucket,
7575
region: storageS3Region,

‎src/storage/backend/index.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,8 @@ export * from './s3'
77
export * from './file'
88
export * from './adapter'
99

10-
const {
11-
storageS3Region,
12-
storageS3Endpoint,
13-
storageS3ForcePathStyle,
14-
storageS3ClientTimeout,
15-
storageS3UploadTimeout,
16-
storageS3DownloadTimeout,
17-
} = getConfig()
10+
const { storageS3Region, storageS3Endpoint, storageS3ForcePathStyle, storageS3ClientTimeout } =
11+
getConfig()
1812

1913
type ConfigForStorage<Type extends StorageBackendType> = Type extends 's3'
2014
? S3ClientOptions
@@ -34,8 +28,6 @@ export function createStorageBackend<Type extends StorageBackendType>(
3428
endpoint: storageS3Endpoint,
3529
forcePathStyle: storageS3ForcePathStyle,
3630
requestTimeout: storageS3ClientTimeout,
37-
uploadTimeout: storageS3UploadTimeout,
38-
downloadTimeout: storageS3DownloadTimeout,
3931
...(config ? config : {}),
4032
}
4133
storageBackend = new S3Backend(defaultOptions)

‎src/storage/backend/s3.ts

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,6 @@ export interface S3ClientOptions {
126126
role?: string
127127
httpAgent?: { httpAgent: Agent; httpsAgent: HttpsAgent }
128128
requestTimeout?: number
129-
downloadTimeout?: number
130-
uploadTimeout?: number
131129
}
132130

133131
/**
@@ -136,29 +134,12 @@ export interface S3ClientOptions {
136134
*/
137135
export class S3Backend implements StorageBackendAdapter {
138136
client: S3Client
139-
uploadClient: S3Client
140-
downloadClient: S3Client
141137

142138
constructor(options: S3ClientOptions) {
143139
// Default client for API operations
144140
this.client = this.createS3Client({
145141
...options,
146142
name: 's3_default',
147-
requestTimeout: options.requestTimeout,
148-
})
149-
150-
// Upload client exclusively for upload operations
151-
this.uploadClient = this.createS3Client({
152-
...options,
153-
name: 's3_upload',
154-
requestTimeout: options.uploadTimeout,
155-
})
156-
157-
// Download client exclusively for download operations
158-
this.downloadClient = this.createS3Client({
159-
...options,
160-
name: 's3_download',
161-
requestTimeout: options.downloadTimeout,
162143
})
163144
}
164145

@@ -187,7 +168,7 @@ export class S3Backend implements StorageBackendAdapter {
187168
input.IfModifiedSince = new Date(headers.ifModifiedSince)
188169
}
189170
const command = new GetObjectCommand(input)
190-
const data = await this.downloadClient.send(command, {
171+
const data = await this.client.send(command, {
191172
abortSignal: signal,
192173
})
193174

@@ -228,7 +209,7 @@ export class S3Backend implements StorageBackendAdapter {
228209
): Promise<ObjectMetadata> {
229210
try {
230211
const paralellUploadS3 = new Upload({
231-
client: this.uploadClient,
212+
client: this.client,
232213
params: {
233214
Bucket: bucketName,
234215
Key: withOptionalVersion(key, version),
@@ -312,7 +293,7 @@ export class S3Backend implements StorageBackendAdapter {
312293
CopySourceIfModifiedSince: conditions?.ifModifiedSince,
313294
CopySourceIfUnmodifiedSince: conditions?.ifUnmodifiedSince,
314295
})
315-
const data = await this.uploadClient.send(command)
296+
const data = await this.client.send(command)
316297
return {
317298
httpStatusCode: data.$metadata.httpStatusCode || 200,
318299
eTag: data.CopyObjectResult?.ETag || '',
@@ -438,9 +419,7 @@ export class S3Backend implements StorageBackendAdapter {
438419
ContentLength: length,
439420
})
440421

441-
const resp = await this.uploadClient.send(paralellUploadS3, {
442-
// overwriting the requestTimeout here to avoid the request being cancelled, as the upload can take a long time for a max 5GB upload
443-
requestTimeout: 0,
422+
const resp = await this.client.send(paralellUploadS3, {
444423
abortSignal: signal,
445424
})
446425

@@ -524,7 +503,7 @@ export class S3Backend implements StorageBackendAdapter {
524503
CopySourceRange: bytesRange ? `bytes=${bytesRange.fromByte}-${bytesRange.toByte}` : undefined,
525504
})
526505

527-
const part = await this.uploadClient.send(uploadPartCopy)
506+
const part = await this.client.send(uploadPartCopy)
528507

529508
return {
530509
eTag: part.CopyPartResult?.ETag,

0 commit comments

Comments
 (0)