From 05ddad0d85923f22f1b344f1866583422346d20e Mon Sep 17 00:00:00 2001 From: Andrea Date: Sat, 1 Mar 2025 12:36:41 +0100 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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({