From 05ddad0d85923f22f1b344f1866583422346d20e Mon Sep 17 00:00:00 2001 From: Andrea Date: Sat, 1 Mar 2025 12:36:41 +0100 Subject: [PATCH 01/12] Fix for WARNING: PUT of new resource gave 200, should be 201. --- src/webdav/handlers/PUT.handler.ts | 2 +- test/webdav/handlers/PUT.handler.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/webdav/handlers/PUT.handler.ts b/src/webdav/handlers/PUT.handler.ts index 214b9c55..ca85bede 100644 --- a/src/webdav/handlers/PUT.handler.ts +++ b/src/webdav/handlers/PUT.handler.ts @@ -154,6 +154,6 @@ export class PUTRequestHandler implements WebDavMethodHandler { await driveDatabaseManager.createFile(file, resource.path.dir + '/'); - res.status(200).send(); + res.status(201).send(); }; } diff --git a/test/webdav/handlers/PUT.handler.test.ts b/test/webdav/handlers/PUT.handler.test.ts index 41b81276..bacb7e9d 100644 --- a/test/webdav/handlers/PUT.handler.test.ts +++ b/test/webdav/handlers/PUT.handler.test.ts @@ -138,7 +138,7 @@ describe('PUT request handler', () => { const createDBFileStub = vi.spyOn(driveDatabaseManager, 'createFile').mockResolvedValue(fileFixture); await sut.handle(request, response); - expect(response.status).toHaveBeenCalledWith(200); + expect(response.status).toHaveBeenCalledWith(201); expect(getRequestedResourceStub).toHaveBeenCalledTimes(2); expect(getAndSearchItemFromResourceStub).toHaveBeenCalledTimes(2); expect(getAuthDetailsStub).toHaveBeenCalledOnce(); @@ -206,7 +206,7 @@ describe('PUT request handler', () => { const createDBFileStub = vi.spyOn(driveDatabaseManager, 'createFile').mockResolvedValue(fileFixture); await sut.handle(request, response); - expect(response.status).toHaveBeenCalledWith(200); + expect(response.status).toHaveBeenCalledWith(201); expect(getRequestedResourceStub).toHaveBeenCalledTimes(2); expect(getAndSearchItemFromResourceStub).toHaveBeenCalledTimes(2); expect(getAuthDetailsStub).toHaveBeenCalledOnce(); From 066aa7abbd242015d50e6aa7358f4278303fa037 Mon Sep 17 00:00:00 2001 From: Andrea Date: Sat, 1 Mar 2025 13:52:06 +0100 Subject: [PATCH 02/12] Fix for WARNING: MKCOL on existing collection gave 400, should be 405 (RFC2518:8.3.2); TODO: need to get the exact http error code from the SDK. --- src/utils/webdav.utils.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/utils/webdav.utils.ts b/src/utils/webdav.utils.ts index ca989fc4..6767fa81 100644 --- a/src/utils/webdav.utils.ts +++ b/src/utils/webdav.utils.ts @@ -7,7 +7,7 @@ import { DriveDatabaseManager } from '../services/database/drive-database-manage import { DriveFolderService } from '../services/drive/drive-folder.service'; import { DriveFileService } from '../services/drive/drive-file.service'; import { DriveFileItem, DriveFolderItem } from '../types/drive.types'; -import { NotFoundError } from './errors.utils'; +import { ConflictError, NotFoundError } from './errors.utils'; import { webdavLogger } from './logger.utils'; export class WebDavUtils { @@ -34,6 +34,7 @@ export class WebDavUtils { } else { requestUrl = urlObject.url; } + const decodedUrl = decodeURIComponent(requestUrl).replaceAll('/./', '/'); const parsedPath = path.parse(decodedUrl); let parentPath = path.dirname(decodedUrl); @@ -95,8 +96,23 @@ export class WebDavUtils { driveFileService?: DriveFileService, ): Promise { let item: DriveFileItem | DriveFolderItem | undefined = undefined; + if (resource.type === 'folder') { - item = await driveFolderService?.getFolderMetadataByPath(resource.url); + webdavLogger.info('Andrea: resource ->', { resource }); + // if resource has a parentPath it means it's a subfolder then try to get it; if it throws an error it means it doesn't + // exist and we should throw a 409 error in compliance with the WebDAV RFC + // catch the error during getting parent folder and throw a 409 error in compliance with the WebDAV RFC + + try { + item = await driveFolderService?.getFolderMetadataByPath(resource.url); + } + // TODO: get the exact error type from the SDK, it should be a 404 error + catch (error: any) { + // if the error is a 404 error, it means the resource doesn't exist + // in this case, throw a 409 error in compliance with the WebDAV RFC + // if error is of type AppError, throw it + throw new ConflictError(`Resource not found on Internxt Drive at ${resource.url}`); + } } if (resource.type === 'file') { item = await driveFileService?.getFileMetadataByPath(resource.url); From 0c8fad244c89a1169b25693bb4f19f4dac637a79 Mon Sep 17 00:00:00 2001 From: Andrea Date: Sat, 1 Mar 2025 14:06:50 +0100 Subject: [PATCH 03/12] Fix for WARNING: MKCOL on existing collection gave 400, should be 405 (RFC2518:8.3.2); TODO: need to get the exact http error code from the SDK. --- src/webdav/handlers/MKCOL.handler.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/webdav/handlers/MKCOL.handler.ts b/src/webdav/handlers/MKCOL.handler.ts index 44578f94..2df9351b 100644 --- a/src/webdav/handlers/MKCOL.handler.ts +++ b/src/webdav/handlers/MKCOL.handler.ts @@ -7,6 +7,7 @@ import { webdavLogger } from '../../utils/logger.utils'; import { XMLUtils } from '../../utils/xml.utils'; import { AsyncUtils } from '../../utils/async.utils'; import { DriveFolderItem } from '../../types/drive.types'; +import { MethodNotAllowed } from '../../utils/errors.utils'; export class MKCOLRequestHandler implements WebDavMethodHandler { constructor( @@ -30,6 +31,23 @@ export class MKCOLRequestHandler implements WebDavMethodHandler { driveFolderService, })) as DriveFolderItem; + var folderAlreadyExists = true; + // try to get the folder from the drive before creating it + // The method getFolderMetadataByPath will throw an error if the folder does not exist, so we need to catch it + try { + await driveFolderService.getFolderMetadataByPath(resource.url); + } + // TODO: This is a bad practice, we should catch a specific error (e.g. FolderNotFound) instead of a generic one; + // In this case a 404 error must spefically catched + catch (error) { + folderAlreadyExists = false; + } + + if (folderAlreadyExists) { + webdavLogger.info(`[MKCOL] ❌ Folder already exists`); + throw new MethodNotAllowed('Folder already exists'); + } + const [createFolder] = driveFolderService.createFolder({ plainName: resource.path.base, parentFolderUuid: parentFolderItem.uuid, From bd83bd71890f2856fc15e6a003c8b35997e335b9 Mon Sep 17 00:00:00 2001 From: Andrea Date: Sat, 1 Mar 2025 15:14:45 +0100 Subject: [PATCH 04/12] Fix for FAIL (MKCOL with weird body must fail (RFC2518:8.3.1)) and FAIL (MKCOL with weird body must fail with 415 (RFC2518:8.3.1)). --- src/webdav/handlers/MKCOL.handler.ts | 11 ++++++++--- src/webdav/webdav-server.ts | 3 ++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/webdav/handlers/MKCOL.handler.ts b/src/webdav/handlers/MKCOL.handler.ts index 2df9351b..840f146f 100644 --- a/src/webdav/handlers/MKCOL.handler.ts +++ b/src/webdav/handlers/MKCOL.handler.ts @@ -7,7 +7,7 @@ import { webdavLogger } from '../../utils/logger.utils'; import { XMLUtils } from '../../utils/xml.utils'; import { AsyncUtils } from '../../utils/async.utils'; import { DriveFolderItem } from '../../types/drive.types'; -import { MethodNotAllowed } from '../../utils/errors.utils'; +import { MethodNotAllowed, UnsupportedMediaTypeError } from '../../utils/errors.utils'; export class MKCOLRequestHandler implements WebDavMethodHandler { constructor( @@ -15,12 +15,17 @@ export class MKCOLRequestHandler implements WebDavMethodHandler { driveDatabaseManager: DriveDatabaseManager; driveFolderService: DriveFolderService; }, - ) {} + ) { } handle = async (req: Request, res: Response) => { const { driveDatabaseManager, driveFolderService } = this.dependencies; const resource = await WebDavUtils.getRequestedResource(req); + // if body is not empty throw a 500 error + if (Object.keys(req.body).length > 0) { + throw new UnsupportedMediaTypeError('Body not allowed for MKCOL method'); + } + webdavLogger.info(`[MKCOL] Request received for ${resource.type} at ${resource.url}`); const parentResource = await WebDavUtils.getRequestedResource(resource.parentPath); @@ -44,7 +49,7 @@ export class MKCOLRequestHandler implements WebDavMethodHandler { } if (folderAlreadyExists) { - webdavLogger.info(`[MKCOL] ❌ Folder already exists`); + webdavLogger.info(`[MKCOL] ❌ Folder already exists`); throw new MethodNotAllowed('Folder already exists'); } diff --git a/src/webdav/webdav-server.ts b/src/webdav/webdav-server.ts index 7062f70c..0dbca8c8 100644 --- a/src/webdav/webdav-server.ts +++ b/src/webdav/webdav-server.ts @@ -68,8 +68,9 @@ export class WebDavServer { }; private readonly registerMiddlewares = async () => { - this.app.use(bodyParser.text({ type: ['application/xml', 'text/xml'] })); this.app.use(ErrorHandlingMiddleware); + // include body of the request without any parsing + this.app.use(bodyParser.raw({ type: '*/*' })); this.app.use(AuthMiddleware(AuthService.instance)); this.app.use( RequestLoggerMiddleware({ From 75c033534d8bb3a6d80968ad3c5f1ad419cccd13 Mon Sep 17 00:00:00 2001 From: Andrea Date: Mon, 3 Mar 2025 18:50:06 +0100 Subject: [PATCH 05/12] Added MkcolMiddleware to fix FAIL (MKCOL with weird body must fail (RFC2518:8.3.1)). --- src/utils/webdav.utils.ts | 3 +-- src/webdav/handlers/MKCOL.handler.ts | 5 ----- src/webdav/middewares/mkcol.middleware.ts | 24 +++++++++++++++++++++++ src/webdav/webdav-server.ts | 5 +++-- 4 files changed, 28 insertions(+), 9 deletions(-) create mode 100644 src/webdav/middewares/mkcol.middleware.ts diff --git a/src/utils/webdav.utils.ts b/src/utils/webdav.utils.ts index 6767fa81..a3cc7b34 100644 --- a/src/utils/webdav.utils.ts +++ b/src/utils/webdav.utils.ts @@ -98,7 +98,6 @@ export class WebDavUtils { let item: DriveFileItem | DriveFolderItem | undefined = undefined; if (resource.type === 'folder') { - webdavLogger.info('Andrea: resource ->', { resource }); // if resource has a parentPath it means it's a subfolder then try to get it; if it throws an error it means it doesn't // exist and we should throw a 409 error in compliance with the WebDAV RFC // catch the error during getting parent folder and throw a 409 error in compliance with the WebDAV RFC @@ -110,7 +109,7 @@ export class WebDavUtils { catch (error: any) { // if the error is a 404 error, it means the resource doesn't exist // in this case, throw a 409 error in compliance with the WebDAV RFC - // if error is of type AppError, throw it + throw new ConflictError(`Resource not found on Internxt Drive at ${resource.url}`); } } diff --git a/src/webdav/handlers/MKCOL.handler.ts b/src/webdav/handlers/MKCOL.handler.ts index 840f146f..9515043b 100644 --- a/src/webdav/handlers/MKCOL.handler.ts +++ b/src/webdav/handlers/MKCOL.handler.ts @@ -21,11 +21,6 @@ export class MKCOLRequestHandler implements WebDavMethodHandler { const { driveDatabaseManager, driveFolderService } = this.dependencies; const resource = await WebDavUtils.getRequestedResource(req); - // if body is not empty throw a 500 error - if (Object.keys(req.body).length > 0) { - throw new UnsupportedMediaTypeError('Body not allowed for MKCOL method'); - } - webdavLogger.info(`[MKCOL] Request received for ${resource.type} at ${resource.url}`); const parentResource = await WebDavUtils.getRequestedResource(resource.parentPath); diff --git a/src/webdav/middewares/mkcol.middleware.ts b/src/webdav/middewares/mkcol.middleware.ts new file mode 100644 index 00000000..10342b5a --- /dev/null +++ b/src/webdav/middewares/mkcol.middleware.ts @@ -0,0 +1,24 @@ +import { RequestHandler } from 'express'; + +export const MkcolMiddleware: RequestHandler = (req, _, next) => { + // if request content types are not 'application/xml', 'text/xml' or not defined, return 415 + if (req.method === 'MKCOL') { + if (req.get('Content-Type')) { + if (!req.is('application/xml') && !req.is('text/xml')) { + return next({ + status: 415, + message: 'Unsupported Media Type', + }); + } + } + // body must be empty + if (req.body && Object.keys(req.body).length > 0) { + return next({ + status: 415, + message: 'Unsupported Media Type', + }); + } + } + + next(); +}; diff --git a/src/webdav/webdav-server.ts b/src/webdav/webdav-server.ts index 0dbca8c8..906230f1 100644 --- a/src/webdav/webdav-server.ts +++ b/src/webdav/webdav-server.ts @@ -29,6 +29,7 @@ import { MOVERequestHandler } from './handlers/MOVE.handler'; import { COPYRequestHandler } from './handlers/COPY.handler'; import { TrashService } from '../services/drive/trash.service'; import { Environment } from '@internxt/inxt-js'; +import { MkcolMiddleware } from './middewares/mkcol.middleware'; export class WebDavServer { constructor( @@ -69,8 +70,8 @@ export class WebDavServer { private readonly registerMiddlewares = async () => { this.app.use(ErrorHandlingMiddleware); - // include body of the request without any parsing - this.app.use(bodyParser.raw({ type: '*/*' })); + this.app.use(MkcolMiddleware); + this.app.use(bodyParser.text({ type: ['application/xml', 'text/xml'] })); this.app.use(AuthMiddleware(AuthService.instance)); this.app.use( RequestLoggerMiddleware({ From 857e974f0b292e360b9f756ded699035aac957c0 Mon Sep 17 00:00:00 2001 From: Andrea Date: Mon, 3 Mar 2025 19:30:33 +0100 Subject: [PATCH 06/12] Refactoring. --- src/utils/webdav.utils.ts | 8 ++++---- src/webdav/handlers/MKCOL.handler.ts | 10 ++++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/utils/webdav.utils.ts b/src/utils/webdav.utils.ts index a3cc7b34..a04d0d01 100644 --- a/src/utils/webdav.utils.ts +++ b/src/utils/webdav.utils.ts @@ -101,16 +101,16 @@ export class WebDavUtils { // if resource has a parentPath it means it's a subfolder then try to get it; if it throws an error it means it doesn't // exist and we should throw a 409 error in compliance with the WebDAV RFC // catch the error during getting parent folder and throw a 409 error in compliance with the WebDAV RFC - try { item = await driveFolderService?.getFolderMetadataByPath(resource.url); } - // TODO: get the exact error type from the SDK, it should be a 404 error catch (error: any) { // if the error is a 404 error, it means the resource doesn't exist // in this case, throw a 409 error in compliance with the WebDAV RFC - - throw new ConflictError(`Resource not found on Internxt Drive at ${resource.url}`); + if (error.status === 404) { + throw new ConflictError(`Resource not found on Internxt Drive at ${resource.url}`); + } + throw error; } } if (resource.type === 'file') { diff --git a/src/webdav/handlers/MKCOL.handler.ts b/src/webdav/handlers/MKCOL.handler.ts index 9515043b..6413ff84 100644 --- a/src/webdav/handlers/MKCOL.handler.ts +++ b/src/webdav/handlers/MKCOL.handler.ts @@ -7,7 +7,7 @@ import { webdavLogger } from '../../utils/logger.utils'; import { XMLUtils } from '../../utils/xml.utils'; import { AsyncUtils } from '../../utils/async.utils'; import { DriveFolderItem } from '../../types/drive.types'; -import { MethodNotAllowed, UnsupportedMediaTypeError } from '../../utils/errors.utils'; +import { MethodNotAllowed } from '../../utils/errors.utils'; export class MKCOLRequestHandler implements WebDavMethodHandler { constructor( @@ -37,10 +37,12 @@ export class MKCOLRequestHandler implements WebDavMethodHandler { try { await driveFolderService.getFolderMetadataByPath(resource.url); } - // TODO: This is a bad practice, we should catch a specific error (e.g. FolderNotFound) instead of a generic one; // In this case a 404 error must spefically catched - catch (error) { - folderAlreadyExists = false; + catch (error: any) { + // check if the error is a 404 error + if (error.status === 404) { + folderAlreadyExists = false; + } } if (folderAlreadyExists) { From ba56a91da6346f56739e758577658621e5c6da40 Mon Sep 17 00:00:00 2001 From: Andrea Date: Mon, 3 Mar 2025 19:35:40 +0100 Subject: [PATCH 07/12] Update middlewares order. --- src/webdav/webdav-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/webdav/webdav-server.ts b/src/webdav/webdav-server.ts index 906230f1..1cbaee3e 100644 --- a/src/webdav/webdav-server.ts +++ b/src/webdav/webdav-server.ts @@ -69,9 +69,9 @@ export class WebDavServer { }; private readonly registerMiddlewares = async () => { + this.app.use(bodyParser.text({ type: ['application/xml', 'text/xml'] })); this.app.use(ErrorHandlingMiddleware); this.app.use(MkcolMiddleware); - this.app.use(bodyParser.text({ type: ['application/xml', 'text/xml'] })); this.app.use(AuthMiddleware(AuthService.instance)); this.app.use( RequestLoggerMiddleware({ From c3827855de8b36418656d0e11ba71336c9c018dc Mon Sep 17 00:00:00 2001 From: larry-internxt Date: Tue, 15 Apr 2025 11:24:02 +0200 Subject: [PATCH 08/12] refactor: improve error handling in WebDavUtils and MKCOLRequestHandler --- src/utils/webdav.utils.ts | 8 ++++---- src/webdav/handlers/MKCOL.handler.ts | 14 ++++++-------- test/webdav/handlers/MKCOL.handler.test.ts | 5 +++++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/utils/webdav.utils.ts b/src/utils/webdav.utils.ts index a04d0d01..85323c97 100644 --- a/src/utils/webdav.utils.ts +++ b/src/utils/webdav.utils.ts @@ -9,6 +9,7 @@ import { DriveFileService } from '../services/drive/drive-file.service'; import { DriveFileItem, DriveFolderItem } from '../types/drive.types'; import { ConflictError, NotFoundError } from './errors.utils'; import { webdavLogger } from './logger.utils'; +import AppError from '@internxt/sdk/dist/shared/types/errors'; export class WebDavUtils { static joinURL(...pathComponents: string[]): string { @@ -98,16 +99,15 @@ export class WebDavUtils { let item: DriveFileItem | DriveFolderItem | undefined = undefined; if (resource.type === 'folder') { - // if resource has a parentPath it means it's a subfolder then try to get it; if it throws an error it means it doesn't + // if resource has a parentPath it means it's a subfolder then try to get it; if it throws an error it means it doesn't // exist and we should throw a 409 error in compliance with the WebDAV RFC // catch the error during getting parent folder and throw a 409 error in compliance with the WebDAV RFC try { item = await driveFolderService?.getFolderMetadataByPath(resource.url); - } - catch (error: any) { + } catch (error) { // if the error is a 404 error, it means the resource doesn't exist // in this case, throw a 409 error in compliance with the WebDAV RFC - if (error.status === 404) { + if ((error as AppError).status === 404) { throw new ConflictError(`Resource not found on Internxt Drive at ${resource.url}`); } throw error; diff --git a/src/webdav/handlers/MKCOL.handler.ts b/src/webdav/handlers/MKCOL.handler.ts index 6413ff84..04c04830 100644 --- a/src/webdav/handlers/MKCOL.handler.ts +++ b/src/webdav/handlers/MKCOL.handler.ts @@ -8,6 +8,7 @@ import { XMLUtils } from '../../utils/xml.utils'; import { AsyncUtils } from '../../utils/async.utils'; import { DriveFolderItem } from '../../types/drive.types'; import { MethodNotAllowed } from '../../utils/errors.utils'; +import AppError from '@internxt/sdk/dist/shared/types/errors'; export class MKCOLRequestHandler implements WebDavMethodHandler { constructor( @@ -15,7 +16,7 @@ export class MKCOLRequestHandler implements WebDavMethodHandler { driveDatabaseManager: DriveDatabaseManager; driveFolderService: DriveFolderService; }, - ) { } + ) {} handle = async (req: Request, res: Response) => { const { driveDatabaseManager, driveFolderService } = this.dependencies; @@ -31,22 +32,19 @@ export class MKCOLRequestHandler implements WebDavMethodHandler { driveFolderService, })) as DriveFolderItem; - var folderAlreadyExists = true; + let folderAlreadyExists = true; // try to get the folder from the drive before creating it // The method getFolderMetadataByPath will throw an error if the folder does not exist, so we need to catch it try { await driveFolderService.getFolderMetadataByPath(resource.url); - } - // In this case a 404 error must spefically catched - catch (error: any) { - // check if the error is a 404 error - if (error.status === 404) { + } catch (error) { + if ((error as AppError).status === 404) { folderAlreadyExists = false; } } if (folderAlreadyExists) { - webdavLogger.info(`[MKCOL] ❌ Folder already exists`); + webdavLogger.info(`[MKCOL] ❌ Folder '${resource.url}' already exists`); throw new MethodNotAllowed('Folder already exists'); } diff --git a/test/webdav/handlers/MKCOL.handler.test.ts b/test/webdav/handlers/MKCOL.handler.test.ts index 4c028b7f..a9c4fa5a 100644 --- a/test/webdav/handlers/MKCOL.handler.test.ts +++ b/test/webdav/handlers/MKCOL.handler.test.ts @@ -13,6 +13,7 @@ import { newCreateFolderResponse, newFolderItem } from '../../fixtures/drive.fix import { WebDavRequestedResource } from '../../../src/types/webdav.types'; import { WebDavUtils } from '../../../src/utils/webdav.utils'; import { DriveFolder } from '../../../src/services/database/drive-folder/drive-folder.domain'; +import AppError from '@internxt/sdk/dist/shared/types/errors'; describe('MKCOL request handler', () => { beforeEach(() => { @@ -61,6 +62,9 @@ describe('MKCOL request handler', () => { const createFolderStub = vi .spyOn(driveFolderService, 'createFolder') .mockReturnValue([Promise.resolve(newFolderResponse), { cancel: () => {} }]); + const getFolderMetadataStub = vi + .spyOn(driveFolderService, 'getFolderMetadataByPath') + .mockRejectedValue(new AppError('Folder not found', 404)); const createDatabaseFolderStub = vi .spyOn(driveDatabaseManager, 'createFolder') .mockResolvedValue({} as DriveFolder); @@ -74,5 +78,6 @@ describe('MKCOL request handler', () => { parentFolderUuid: parentFolder.uuid, }); expect(createDatabaseFolderStub).toHaveBeenCalledOnce(); + expect(getFolderMetadataStub).toHaveBeenCalledOnce(); }); }); From 98ce627515eea741f12a7ab0c20b7bbbf1bbb0c4 Mon Sep 17 00:00:00 2001 From: larry-internxt Date: Tue, 15 Apr 2025 12:13:50 +0200 Subject: [PATCH 09/12] feat: add MKCOL middleware unit tests --- src/webdav/middewares/mkcol.middleware.ts | 10 ++- .../middlewares/mkcol.middleware.test.ts | 72 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 test/webdav/middlewares/mkcol.middleware.test.ts diff --git a/src/webdav/middewares/mkcol.middleware.ts b/src/webdav/middewares/mkcol.middleware.ts index 10342b5a..d9dd3c99 100644 --- a/src/webdav/middewares/mkcol.middleware.ts +++ b/src/webdav/middewares/mkcol.middleware.ts @@ -3,8 +3,14 @@ import { RequestHandler } from 'express'; export const MkcolMiddleware: RequestHandler = (req, _, next) => { // if request content types are not 'application/xml', 'text/xml' or not defined, return 415 if (req.method === 'MKCOL') { - if (req.get('Content-Type')) { - if (!req.is('application/xml') && !req.is('text/xml')) { + let contentType = req.headers['Content-Type']; + if (contentType && contentType.length > 0) { + if (Array.isArray(contentType)) { + contentType = contentType[0]; + } + contentType = contentType.toLowerCase().trim(); + + if (contentType !== 'application/xml' && contentType !== 'text/xml') { return next({ status: 415, message: 'Unsupported Media Type', diff --git a/test/webdav/middlewares/mkcol.middleware.test.ts b/test/webdav/middlewares/mkcol.middleware.test.ts new file mode 100644 index 00000000..f2f7e771 --- /dev/null +++ b/test/webdav/middlewares/mkcol.middleware.test.ts @@ -0,0 +1,72 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { createWebDavRequestFixture, createWebDavResponseFixture } from '../../fixtures/webdav.fixture'; +import { MkcolMiddleware } from '../../../src/webdav/middewares/mkcol.middleware'; + +describe('MKCOL middleware', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it('When MKCOL content is application/xml, then it should call next', () => { + const req = createWebDavRequestFixture({ + method: 'MKCOL', + url: '/anypath', + headers: { 'Content-Type': 'application/xml' }, + }); + const res = createWebDavResponseFixture({}); + const next = vi.fn(); + + MkcolMiddleware(req, res, next); + + expect(next).toHaveBeenCalledExactlyOnceWith(); + }); + + it('When MKCOL content is text/xml, then it should call next', () => { + const req = createWebDavRequestFixture({ + method: 'MKCOL', + url: '/anypath', + headers: [{ 'Content-Type': 'text/xml' }], + }); + const res = createWebDavResponseFixture({}); + const next = vi.fn(); + + MkcolMiddleware(req, res, next); + + expect(next).toHaveBeenCalledExactlyOnceWith(); + }); + + it('When MKCOL content is not XML, then it should call next with error', () => { + const req = createWebDavRequestFixture({ + method: 'MKCOL', + url: '/anypath', + headers: { 'Content-Type': 'application/json' }, + }); + const res = createWebDavResponseFixture({}); + const next = vi.fn(); + + MkcolMiddleware(req, res, next); + + expect(next).toHaveBeenCalledExactlyOnceWith({ + status: 415, + message: 'Unsupported Media Type', + }); + }); + + it('When MKCOL has body content, then it should call next with error', () => { + const req = createWebDavRequestFixture({ + method: 'MKCOL', + url: '/anypath', + headers: { 'Content-Type': 'text/xml' }, + body: '', + }); + const res = createWebDavResponseFixture({}); + const next = vi.fn(); + + MkcolMiddleware(req, res, next); + + expect(next).toHaveBeenCalledExactlyOnceWith({ + status: 415, + message: 'Unsupported Media Type', + }); + }); +}); From 7eca0ff9394e73158f734fe350c8379b8d6d676b Mon Sep 17 00:00:00 2001 From: larry-internxt Date: Tue, 15 Apr 2025 12:36:04 +0200 Subject: [PATCH 10/12] test: add ConflictError handling in WebDavUtils unit tests --- test/utils/webdav.utils.test.ts | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/test/utils/webdav.utils.test.ts b/test/utils/webdav.utils.test.ts index f2cff85b..55713c81 100644 --- a/test/utils/webdav.utils.test.ts +++ b/test/utils/webdav.utils.test.ts @@ -11,7 +11,8 @@ import { newFileItem, newFolderItem } from '../fixtures/drive.fixture'; import { DriveFolderService } from '../../src/services/drive/drive-folder.service'; import { DriveFileService } from '../../src/services/drive/drive-file.service'; import { fail } from 'node:assert'; -import { NotFoundError } from '../../src/utils/errors.utils'; +import { ConflictError, NotFoundError } from '../../src/utils/errors.utils'; +import AppError from '@internxt/sdk/dist/shared/types/errors'; describe('Webdav utils', () => { beforeEach(() => { @@ -260,6 +261,23 @@ describe('Webdav utils', () => { expect(findFileStub).not.toHaveBeenCalled(); }); + it('When folder is not found, then it throws ConflictError', async () => { + const findFolderStub = vi + .spyOn(DriveFolderService.instance, 'getFolderMetadataByPath') + .mockRejectedValue(new AppError('Folder not found', 404)); + const findFileStub = vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockRejectedValue(new Error()); + + try { + await WebDavUtils.getDriveItemFromResource(requestFolderFixture, DriveFolderService.instance, undefined); + fail('Expected function to throw an error, but it did not.'); + } catch (error) { + expect(error).to.be.instanceOf(ConflictError); + } + + expect(findFolderStub).toHaveBeenCalledOnce(); + expect(findFileStub).not.toHaveBeenCalled(); + }); + it('When file resource is looked by its path, then it is returned', async () => { const expectedFile = newFileItem(); const findFileStub = vi.spyOn(DriveFileService.instance, 'getFileMetadataByPath').mockResolvedValue(expectedFile); From 6db5ebec18a521bc8f35196cbfa3faba4ee480f5 Mon Sep 17 00:00:00 2001 From: larry-internxt Date: Tue, 15 Apr 2025 14:01:05 +0200 Subject: [PATCH 11/12] chore: remove unused environment variables --- .env.template | 5 +---- .github/workflows/publish.yml | 3 --- src/types/config.types.ts | 3 --- 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/.env.template b/.env.template index 4baea95c..2cef71c1 100644 --- a/.env.template +++ b/.env.template @@ -2,11 +2,8 @@ DRIVE_URL=https://drive.internxt.com DRIVE_API_URL=https://drive.internxt.com/api DRIVE_NEW_API_URL=https://api.internxt.com/drive PAYMENTS_API_URL=https://api.internxt.com/payments -PHOTOS_API_URL=https://photos.internxt.com/api NETWORK_URL=https://api.internxt.com APP_CRYPTO_SECRET=6KYQBP847D4ATSFA APP_CRYPTO_SECRET2=8Q8VMUE3BJZV87GT APP_MAGIC_IV=d139cb9a2cd17092e79e1861cf9d7023 -APP_MAGIC_SALT=38dce0391b49efba88dbc8c39ebf868f0267eb110bb0012ab27dc52a528d61b1d1ed9d76f400ff58e3240028442b1eab9bb84e111d9dadd997982dbde9dbd25e -RUDDERSTACK_WRITE_KEY= -RUDDERSTACK_DATAPLANE_URL= \ No newline at end of file +APP_MAGIC_SALT=38dce0391b49efba88dbc8c39ebf868f0267eb110bb0012ab27dc52a528d61b1d1ed9d76f400ff58e3240028442b1eab9bb84e111d9dadd997982dbde9dbd25e \ No newline at end of file diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 6ff180fe..3128bf61 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -27,14 +27,11 @@ jobs: echo "DRIVE_API_URL=https://drive.internxt.com/api" >> .env echo "DRIVE_NEW_API_URL=https://api.internxt.com/drive" >> .env echo "PAYMENTS_API_URL=https://api.internxt.com/payments" >> .env - echo "PHOTOS_API_URL=https://photos.internxt.com/api" >> .env echo "NETWORK_URL=https://api.internxt.com" >> .env echo "APP_CRYPTO_SECRET=6KYQBP847D4ATSFA" >> .env echo "APP_CRYPTO_SECRET2=8Q8VMUE3BJZV87GT" >> .env echo "APP_MAGIC_IV=d139cb9a2cd17092e79e1861cf9d7023" >> .env echo "APP_MAGIC_SALT=38dce0391b49efba88dbc8c39ebf868f0267eb110bb0012ab27dc52a528d61b1d1ed9d76f400ff58e3240028442b1eab9bb84e111d9dadd997982dbde9dbd25e" >> .env - echo "RUDDERSTACK_WRITE_KEY=" >> .env - echo "RUDDERSTACK_DATAPLANE_URL=" >> .env echo "NODE_ENV=production" >> .env - run: yarn run build diff --git a/src/types/config.types.ts b/src/types/config.types.ts index 9cc43e16..3ae05aee 100644 --- a/src/types/config.types.ts +++ b/src/types/config.types.ts @@ -3,12 +3,9 @@ export interface ConfigKeys { readonly DRIVE_API_URL: string; readonly DRIVE_NEW_API_URL: string; readonly PAYMENTS_API_URL: string; - readonly PHOTOS_API_URL: string; readonly APP_CRYPTO_SECRET: string; readonly APP_CRYPTO_SECRET2: string; readonly APP_MAGIC_IV: string; readonly APP_MAGIC_SALT: string; readonly NETWORK_URL: string; - readonly RUDDERSTACK_WRITE_KEY: string; - readonly RUDDERSTACK_DATAPLANE_URL: string; } From 9e95a657fd8c9a8a3a573ed5258b6f5ae004e240 Mon Sep 17 00:00:00 2001 From: larry-internxt Date: Wed, 30 Apr 2025 11:57:18 +0200 Subject: [PATCH 12/12] refactor: enhance MKCOL middleware error handling and update tests --- src/webdav/middewares/mkcol.middleware.ts | 13 +++----- src/webdav/webdav-server.ts | 4 +-- .../middlewares/mkcol.middleware.test.ts | 30 ++++++++++--------- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/webdav/middewares/mkcol.middleware.ts b/src/webdav/middewares/mkcol.middleware.ts index d9dd3c99..5d613e30 100644 --- a/src/webdav/middewares/mkcol.middleware.ts +++ b/src/webdav/middewares/mkcol.middleware.ts @@ -1,9 +1,10 @@ import { RequestHandler } from 'express'; +import { UnsupportedMediaTypeError } from '../../utils/errors.utils'; export const MkcolMiddleware: RequestHandler = (req, _, next) => { // if request content types are not 'application/xml', 'text/xml' or not defined, return 415 if (req.method === 'MKCOL') { - let contentType = req.headers['Content-Type']; + let contentType = req.headers['Content-Type'] ?? req.get('content-type'); if (contentType && contentType.length > 0) { if (Array.isArray(contentType)) { contentType = contentType[0]; @@ -11,18 +12,12 @@ export const MkcolMiddleware: RequestHandler = (req, _, next) => { contentType = contentType.toLowerCase().trim(); if (contentType !== 'application/xml' && contentType !== 'text/xml') { - return next({ - status: 415, - message: 'Unsupported Media Type', - }); + throw new UnsupportedMediaTypeError('Unsupported Media Type'); } } // body must be empty if (req.body && Object.keys(req.body).length > 0) { - return next({ - status: 415, - message: 'Unsupported Media Type', - }); + throw new UnsupportedMediaTypeError('Unsupported Media Type'); } } diff --git a/src/webdav/webdav-server.ts b/src/webdav/webdav-server.ts index 1cbaee3e..e15f9db6 100644 --- a/src/webdav/webdav-server.ts +++ b/src/webdav/webdav-server.ts @@ -69,15 +69,15 @@ export class WebDavServer { }; private readonly registerMiddlewares = async () => { - this.app.use(bodyParser.text({ type: ['application/xml', 'text/xml'] })); this.app.use(ErrorHandlingMiddleware); - this.app.use(MkcolMiddleware); this.app.use(AuthMiddleware(AuthService.instance)); this.app.use( RequestLoggerMiddleware({ enable: true, }), ); + this.app.use(bodyParser.text({ type: ['application/xml', 'text/xml'] })); + this.app.use(MkcolMiddleware); }; private readonly registerHandlers = async () => { diff --git a/test/webdav/middlewares/mkcol.middleware.test.ts b/test/webdav/middlewares/mkcol.middleware.test.ts index f2f7e771..282ad3ab 100644 --- a/test/webdav/middlewares/mkcol.middleware.test.ts +++ b/test/webdav/middlewares/mkcol.middleware.test.ts @@ -1,6 +1,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { createWebDavRequestFixture, createWebDavResponseFixture } from '../../fixtures/webdav.fixture'; import { MkcolMiddleware } from '../../../src/webdav/middewares/mkcol.middleware'; +import { fail } from 'node:assert'; +import { UnsupportedMediaTypeError } from '../../../src/utils/errors.utils'; describe('MKCOL middleware', () => { beforeEach(() => { @@ -35,7 +37,7 @@ describe('MKCOL middleware', () => { expect(next).toHaveBeenCalledExactlyOnceWith(); }); - it('When MKCOL content is not XML, then it should call next with error', () => { + it('When MKCOL content is not XML, then it should call next with error', async () => { const req = createWebDavRequestFixture({ method: 'MKCOL', url: '/anypath', @@ -44,15 +46,15 @@ describe('MKCOL middleware', () => { const res = createWebDavResponseFixture({}); const next = vi.fn(); - MkcolMiddleware(req, res, next); - - expect(next).toHaveBeenCalledExactlyOnceWith({ - status: 415, - message: 'Unsupported Media Type', - }); + try { + await MkcolMiddleware(req, res, next); + fail('Expected function to throw an error, but it did not.'); + } catch (error) { + expect(error).to.be.instanceOf(UnsupportedMediaTypeError); + } }); - it('When MKCOL has body content, then it should call next with error', () => { + it('When MKCOL has body content, then it should call next with error', async () => { const req = createWebDavRequestFixture({ method: 'MKCOL', url: '/anypath', @@ -62,11 +64,11 @@ describe('MKCOL middleware', () => { const res = createWebDavResponseFixture({}); const next = vi.fn(); - MkcolMiddleware(req, res, next); - - expect(next).toHaveBeenCalledExactlyOnceWith({ - status: 415, - message: 'Unsupported Media Type', - }); + try { + await MkcolMiddleware(req, res, next); + fail('Expected function to throw an error, but it did not.'); + } catch (error) { + expect(error).to.be.instanceOf(UnsupportedMediaTypeError); + } }); });