diff --git a/lib/remote-storage.js b/lib/remote-storage.js index 7c202fa..bd5a29a 100644 --- a/lib/remote-storage.js +++ b/lib/remote-storage.js @@ -24,22 +24,6 @@ const { codes, logAndThrow } = require('./StorageError') const fileExtensionPattern = /\*\.[0-9a-zA-Z]+$/ -// /** -// * Joins url path parts -// * @param {...string} args url parts -// * @returns {string} -// */ -function urlJoin (...args) { - let start = '' - if (args[0] && - args[0].startsWith('/')) { - start = '/' - } - return start + args.map(a => a && a.replace(/(^\/|\/$)/g, '')) - .filter(a => a) // remove empty strings / nulls - .join('/') -} - module.exports = class RemoteStorage { /** * @param {object} creds @@ -153,7 +137,7 @@ module.exports = class RemoteStorage { const cacheControlString = this._getCacheControlConfig(mimeType, appConfig.app) const uploadParams = { Bucket: this.bucket, - Key: urlJoin(prefix, path.basename(file)), + Key: this._urlJoin(prefix, path.basename(file)), Body: content } // if we found it in the global config, we will use it ( for now ) @@ -293,7 +277,7 @@ module.exports = class RemoteStorage { // base directory returns ".", ignore that. prefixDirectory = prefixDirectory === '.' ? '' : prefixDirectory // newPrefix is now the initial prefix plus the files relative directory path. - const newPrefix = urlJoin(prefix, prefixDirectory) + const newPrefix = this._urlJoin(prefix, prefixDirectory) const s3Res = await this.uploadFile(f, newPrefix, appConfig, dir) if (postFileUploadCallback) { postFileUploadCallback(f) @@ -306,6 +290,41 @@ module.exports = class RemoteStorage { return allResults } + /** + * Joins file path parts into a URL-friendly S3 key + * Normalizes paths by: + * - Preserving leading slash if first part starts with '/' + * - Removing leading/trailing slashes from each part + * - Converting Windows backslashes to forward slashes + * - Removing double slashes + * - Filtering out empty/null parts + * @param {...string} args - Path parts to join + * @returns {string} Normalized S3 key path + */ + _urlJoin (...args) { + // Preserve leading slash if first argument starts with one + const hasLeadingSlash = args[0] && args[0].startsWith('/') + + // Normalize each path part: + // 1. Remove leading and trailing slashes (^\/ matches start, \/$ matches end) + // 2. Convert Windows backslashes to forward slashes + // 3. Filter out empty/null values + const normalizedParts = args + .map(part => { + if (!part) return null + // Remove leading/trailing slashes and convert backslashes to forward slashes + return part.replace(/(^\/|\/$)/g, '').replace(/\\/g, '/') + }) + .filter(part => part) // Remove empty strings and nulls + + // Join parts with '/' and remove any double slashes that may result + const joined = normalizedParts.join('/') + const withoutDoubleSlashes = joined.replace(/\/+/g, '/') + + // Restore leading slash if original first part had one + return hasLeadingSlash ? '/' + withoutDoubleSlashes : withoutDoubleSlashes + } + /** * Get cache control string based on mime type and config * @param {string|boolean} mimeType - string if valid mimeType or false for unknown files diff --git a/test/lib/remote-storage.test.js b/test/lib/remote-storage.test.js index 8086c3f..80b52fc 100644 --- a/test/lib/remote-storage.test.js +++ b/test/lib/remote-storage.test.js @@ -176,6 +176,269 @@ describe('RemoteStorage', () => { }) }) + describe('_urlJoin', () => { + let rs + + beforeEach(() => { + rs = new RemoteStorage(global.fakeTVMResponse) + }) + + describe('basic path joining', () => { + test('joins simple path parts', () => { + expect(rs._urlJoin('prefix', 'file.js')).toBe('prefix/file.js') + }) + + test('joins multiple path parts', () => { + expect(rs._urlJoin('prefix', 'deep', 'nested', 'file.js')).toBe('prefix/deep/nested/file.js') + }) + + test('handles single part', () => { + expect(rs._urlJoin('file.js')).toBe('file.js') + }) + + test('handles empty string parts', () => { + expect(rs._urlJoin('prefix', '', 'file.js')).toBe('prefix/file.js') + }) + + test('handles null parts', () => { + expect(rs._urlJoin('prefix', null, 'file.js')).toBe('prefix/file.js') + }) + + test('handles undefined parts', () => { + expect(rs._urlJoin('prefix', undefined, 'file.js')).toBe('prefix/file.js') + }) + + test('handles all empty/null parts', () => { + expect(rs._urlJoin('', null, undefined)).toBe('') + }) + }) + + describe('leading slash preservation', () => { + test('preserves leading slash from first argument', () => { + expect(rs._urlJoin('/prefix', 'file.js')).toBe('/prefix/file.js') + }) + + test('preserves leading slash with multiple parts', () => { + expect(rs._urlJoin('/prefix', 'deep', 'file.js')).toBe('/prefix/deep/file.js') + }) + + test('does not add leading slash when first part does not have one', () => { + expect(rs._urlJoin('prefix', 'file.js')).toBe('prefix/file.js') + }) + + test('handles leading slash with single part', () => { + expect(rs._urlJoin('/file.js')).toBe('/file.js') + }) + }) + + describe('trailing slash removal', () => { + test('removes trailing slash from parts', () => { + expect(rs._urlJoin('prefix/', 'file.js')).toBe('prefix/file.js') + }) + + test('removes trailing slash from multiple parts', () => { + expect(rs._urlJoin('prefix/', 'deep/', 'file.js')).toBe('prefix/deep/file.js') + }) + + test('removes trailing slash from last part', () => { + expect(rs._urlJoin('prefix', 'file.js/')).toBe('prefix/file.js') + }) + + test('removes trailing slash while preserving leading slash', () => { + expect(rs._urlJoin('/prefix/', 'file.js')).toBe('/prefix/file.js') + }) + }) + + describe('leading slash removal', () => { + test('removes leading slash from non-first parts', () => { + expect(rs._urlJoin('prefix', '/deep', 'file.js')).toBe('prefix/deep/file.js') + }) + + test('removes leading slash from middle parts', () => { + expect(rs._urlJoin('prefix', '/deep', '/nested', 'file.js')).toBe('prefix/deep/nested/file.js') + }) + + test('preserves leading slash only on first part', () => { + expect(rs._urlJoin('/prefix', '/deep', '/file.js')).toBe('/prefix/deep/file.js') + }) + }) + + describe('Windows backslash conversion', () => { + test('converts Windows backslashes to forward slashes', () => { + expect(rs._urlJoin('prefix\\deep', 'file.js')).toBe('prefix/deep/file.js') + }) + + test('converts backslashes in single part', () => { + expect(rs._urlJoin('prefix\\deep\\file.js')).toBe('prefix/deep/file.js') + }) + + test('converts backslashes in multiple parts', () => { + expect(rs._urlJoin('prefix\\deep', 'nested\\file.js')).toBe('prefix/deep/nested/file.js') + }) + + test('converts backslashes while preserving leading slash', () => { + expect(rs._urlJoin('/prefix\\deep', 'file.js')).toBe('/prefix/deep/file.js') + }) + + test('converts mixed backslashes and forward slashes', () => { + expect(rs._urlJoin('prefix\\deep', 'nested/file.js')).toBe('prefix/deep/nested/file.js') + }) + + test('handles Windows-style path with backslashes', () => { + expect(rs._urlJoin('fakeprefix\\deep\\dir', 'index.js')).toBe('fakeprefix/deep/dir/index.js') + }) + }) + + describe('double slash removal', () => { + test('removes double slashes from joined path', () => { + expect(rs._urlJoin('prefix', '', 'file.js')).toBe('prefix/file.js') + }) + + test('removes multiple consecutive slashes', () => { + expect(rs._urlJoin('prefix//deep', 'file.js')).toBe('prefix/deep/file.js') + }) + + test('removes triple slashes', () => { + expect(rs._urlJoin('prefix///deep', 'file.js')).toBe('prefix/deep/file.js') + }) + + test('removes double slashes in middle of path', () => { + expect(rs._urlJoin('prefix', '//deep', 'file.js')).toBe('prefix/deep/file.js') + }) + + test('handles double slashes with leading slash', () => { + expect(rs._urlJoin('/prefix', '//deep', 'file.js')).toBe('/prefix/deep/file.js') + }) + + test('removes double slashes created by empty parts', () => { + expect(rs._urlJoin('prefix', '', '', 'file.js')).toBe('prefix/file.js') + }) + }) + + describe('edge cases with slashes', () => { + test('handles part that is just a slash', () => { + expect(rs._urlJoin('prefix', '/', 'file.js')).toBe('prefix/file.js') + }) + + test('handles part that is just backslash', () => { + expect(rs._urlJoin('prefix', '\\', 'file.js')).toBe('prefix/file.js') + }) + + test('handles multiple slashes-only parts', () => { + expect(rs._urlJoin('prefix', '/', '/', 'file.js')).toBe('prefix/file.js') + }) + + test('handles leading slash with empty parts', () => { + expect(rs._urlJoin('/prefix', '', 'file.js')).toBe('/prefix/file.js') + }) + }) + + describe('real-world S3 key scenarios', () => { + test('creates S3 key from prefix and filename', () => { + expect(rs._urlJoin('fakeprefix', 'index.js')).toBe('fakeprefix/index.js') + }) + + test('creates S3 key with nested directories', () => { + expect(rs._urlJoin('fakeprefix', 'deep', 'dir', 'index.js')).toBe('fakeprefix/deep/dir/index.js') + }) + + test('handles Windows-style prefix with trailing backslash', () => { + expect(rs._urlJoin('fakeprefix\\deep\\dir\\', 'index.js')).toBe('fakeprefix/deep/dir/index.js') + }) + + test('handles prefix with leading slash', () => { + expect(rs._urlJoin('/fakeprefix', 'index.js')).toBe('/fakeprefix/index.js') + }) + + test('handles relative directory path', () => { + expect(rs._urlJoin('deep', 'index.js')).toBe('deep/index.js') + }) + + test('handles empty prefix', () => { + expect(rs._urlJoin('', 'index.js')).toBe('index.js') + }) + + test('handles prefix with mixed separators', () => { + expect(rs._urlJoin('prefix\\deep', 'nested/file.js')).toBe('prefix/deep/nested/file.js') + }) + }) + + describe('special characters and edge cases', () => { + test('handles file with no extension', () => { + expect(rs._urlJoin('prefix', 'file')).toBe('prefix/file') + }) + + test('handles file with multiple dots', () => { + expect(rs._urlJoin('prefix', 'file.min.js')).toBe('prefix/file.min.js') + }) + + test('handles path with spaces', () => { + expect(rs._urlJoin('prefix', 'file name.js')).toBe('prefix/file name.js') + }) + + test('handles path with special characters', () => { + expect(rs._urlJoin('prefix', 'file-name_123.js')).toBe('prefix/file-name_123.js') + }) + + test('handles very long path', () => { + const longPath = Array(10).fill('deep').join('/') + expect(rs._urlJoin('prefix', longPath, 'file.js')).toBe(`prefix/${longPath}/file.js`) + }) + + test('handles path starting with dot', () => { + expect(rs._urlJoin('prefix', '.hidden', 'file.js')).toBe('prefix/.hidden/file.js') + }) + + test('handles path with dot-dot', () => { + expect(rs._urlJoin('prefix', '..', 'file.js')).toBe('prefix/../file.js') + }) + }) + + describe('empty and null handling', () => { + test('returns empty string for no arguments', () => { + expect(rs._urlJoin()).toBe('') + }) + + test('returns empty string for single empty string', () => { + expect(rs._urlJoin('')).toBe('') + }) + + test('filters out all empty parts', () => { + expect(rs._urlJoin('', '', '')).toBe('') + }) + + test('handles mix of valid and empty parts', () => { + expect(rs._urlJoin('', 'prefix', '', 'file.js', '')).toBe('prefix/file.js') + }) + + test('handles null in middle', () => { + expect(rs._urlJoin('prefix', null, 'file.js')).toBe('prefix/file.js') + }) + + test('handles undefined in middle', () => { + expect(rs._urlJoin('prefix', undefined, 'file.js')).toBe('prefix/file.js') + }) + }) + + describe('complex combinations', () => { + test('combines all normalization features', () => { + expect(rs._urlJoin('/prefix\\deep/', '//nested/', '\\file.js')).toBe('/prefix/deep/nested/file.js') + }) + + test('handles Windows path with leading slash', () => { + expect(rs._urlJoin('/prefix\\deep\\dir', 'index.js')).toBe('/prefix/deep/dir/index.js') + }) + + test('handles multiple trailing slashes', () => { + expect(rs._urlJoin('prefix///', 'deep///', 'file.js')).toBe('prefix/deep/file.js') + }) + + test('handles mixed separators and slashes', () => { + expect(rs._urlJoin('prefix\\deep', '/nested', '\\file.js')).toBe('prefix/deep/nested/file.js') + }) + }) + }) + test('folderExists missing prefix', async () => { const rs = new RemoteStorage(global.fakeTVMResponse) await expect(rs.folderExists()).rejects.toEqual(expect.objectContaining({ message: 'prefix must be a valid string' })) @@ -191,6 +454,20 @@ describe('RemoteStorage', () => { await expect(rs.uploadFile()).rejects.toEqual(expect.objectContaining({ message: 'prefix must be a valid string' })) }) + test('uploadFile with windows path', async () => { + const rs = new RemoteStorage(global.fakeTVMResponse) + // Create file using platform-agnostic paths + await global.addFakeFiles(vol, 'fakeDir', { 'deep/index.js': 'fake content' }) + // Use platform-specific path for file reading (must match actual file system) + const filePath = path.join('fakeDir', 'deep', 'index.js') + // Test that Windows-style backslashes in prefix are normalized correctly + const prefixPath = 'fakeprefix\\deep\\dir\\' + await expect(rs.uploadFile(filePath, prefixPath, global.fakeConfig, 'fakeDir')).resolves.toBeUndefined() + expect(mockS3.putObject).toHaveBeenCalledWith( + expect.objectContaining({ Bucket: 'fake-bucket', Key: 'fakeprefix/deep/dir/index.js' }) + ) + }) + test('uploadDir missing prefix', async () => { const rs = new RemoteStorage(global.fakeTVMResponse) await expect(rs.uploadDir()).rejects.toEqual(expect.objectContaining({ message: 'prefix must be a valid string' })) @@ -634,21 +911,6 @@ describe('RemoteStorage', () => { expect(putObjectCall.Metadata).not.toHaveProperty('adp-cache-control') }) - test('uploadFile includes auditUserId in metadata when set', async () => { - global.addFakeFiles(vol, 'fakeDir', { 'index.js': 'fake content' }) - const rs = new RemoteStorage(global.fakeTVMResponse) - const fakeConfig = { ...global.fakeConfig, auditUserId: 'test-user-123' } - await rs.uploadFile('fakeDir/index.js', 'fakeprefix', fakeConfig, 'fakeDir') - const body = Buffer.from('fake content', 'utf8') - const expected = { - Bucket: 'fake-bucket', - Key: 'fakeprefix/index.js', - Body: body, - ContentType: 'application/javascript' - } - expect(mockS3.putObject).toHaveBeenCalledWith(expect.objectContaining(expected)) - }) - test('uploadFile does not set Metadata when responseHeaders is empty', async () => { global.addFakeFiles(vol, 'fakeDir', { 'index.js': 'fake content' }) const rs = new RemoteStorage(global.fakeTVMResponse)