Skip to content

Commit 5d2b7fa

Browse files
H31nz3ldanny-avila
andauthored
🪣 fix: Proper Key Extraction from S3 URL (danny-avila#11241)
* ✨ feat: Enhance S3 URL handling and add comprehensive tests for CRUD operations * 🔒 fix: Improve S3 URL key extraction with enhanced logging and additional test cases * chore: removed some duplicate testcases and fixed incorrect apostrophes * fix: Log error for malformed URLs * test: Add additional test case for extracting keys from S3 URLs * fix: Enhance S3 URL key extraction logic and improve error handling with additional test cases * test: Add test case for stripping bucket from custom endpoint URLs with forcePathStyle enabled * refactor: Update S3 path style handling and enhance environment configuration for S3-compatible services * refactor: Remove S3_FORCE_PATH_STYLE dependency and streamline S3 URL key extraction logic --------- Co-authored-by: Danny Avila <danny@librechat.ai>
1 parent 59717f5 commit 5d2b7fa

2 files changed

Lines changed: 896 additions & 2 deletions

File tree

‎api/server/services/Files/S3/crud.js‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,15 +252,63 @@ function extractKeyFromS3Url(fileUrlOrKey) {
252252

253253
try {
254254
const url = new URL(fileUrlOrKey);
255-
return url.pathname.substring(1);
255+
const hostname = url.hostname;
256+
const pathname = url.pathname.substring(1); // Remove leading slash
257+
258+
if (
259+
hostname === 's3.amazonaws.com' ||
260+
hostname.match(/^s3[-.][a-z0-9-]+\.amazonaws\.com$/) ||
261+
(bucketName && pathname.startsWith(`${bucketName}/`))
262+
) {
263+
// Path-style: https://s3.amazonaws.com/bucket-name/key or custom endpoint (MinIO, R2, etc.)
264+
// Strip the bucket name (first path segment)
265+
const firstSlashIndex = pathname.indexOf('/');
266+
if (firstSlashIndex > 0) {
267+
const key = pathname.substring(firstSlashIndex + 1);
268+
269+
if (key === '') {
270+
logger.warn(
271+
`[extractKeyFromS3Url] Extracted key is empty after removing bucket name from URL: ${fileUrlOrKey}`,
272+
);
273+
} else {
274+
logger.debug(
275+
`[extractKeyFromS3Url] fileUrlOrKey: ${fileUrlOrKey}, Extracted key: ${key}`,
276+
);
277+
}
278+
279+
return key;
280+
} else {
281+
logger.warn(
282+
`[extractKeyFromS3Url] Unable to extract key from path-style URL: ${fileUrlOrKey}`,
283+
);
284+
return '';
285+
}
286+
}
287+
288+
// Virtual-hosted-style or other: https://bucket-name.s3.amazonaws.com/key
289+
// Just return the pathname without leading slash
290+
logger.debug(`[extractKeyFromS3Url] fileUrlOrKey: ${fileUrlOrKey}, Extracted key: ${pathname}`);
291+
return pathname;
256292
} catch (error) {
293+
if (fileUrlOrKey.startsWith('http://') || fileUrlOrKey.startsWith('https://')) {
294+
logger.error(
295+
`[extractKeyFromS3Url] Error parsing URL: ${fileUrlOrKey}, Error: ${error.message}`,
296+
);
297+
} else {
298+
logger.debug(`[extractKeyFromS3Url] Non-URL input, using fallback: ${fileUrlOrKey}`);
299+
}
300+
257301
const parts = fileUrlOrKey.split('/');
258302

259303
if (parts.length >= 3 && !fileUrlOrKey.startsWith('http') && !fileUrlOrKey.startsWith('/')) {
260304
return fileUrlOrKey;
261305
}
262306

263-
return fileUrlOrKey.startsWith('/') ? fileUrlOrKey.substring(1) : fileUrlOrKey;
307+
const key = fileUrlOrKey.startsWith('/') ? fileUrlOrKey.substring(1) : fileUrlOrKey;
308+
logger.debug(
309+
`[extractKeyFromS3Url] FALLBACK. fileUrlOrKey: ${fileUrlOrKey}, Extracted key: ${key}`,
310+
);
311+
return key;
264312
}
265313
}
266314

@@ -482,4 +530,5 @@ module.exports = {
482530
refreshS3Url,
483531
needsRefresh,
484532
getNewS3URL,
533+
extractKeyFromS3Url,
485534
};

0 commit comments

Comments
 (0)