Fix file controller route parameter extraction (#15924)

## Problem When accessing file URLs like `/files/attachment/{token}/{filename}`, the server was returning 500 errors with no logs. The issue was that the route pattern `*path/:filename` was not properly extracting parameters, causing `request.params[0]` to be undefined. ## Solution - Changed route from `@Get('*path/:filename')` to `@Get(':folder/:token/:filename')` for explicit parameter extraction - Simplified `extractFileInfoFromRequest` to use named route parameters directly instead of complex path parsing - Removed unnecessary logging that was added during debugging - Added test to verify route parameters are correctly extracted and passed to FileService ## Testing - Added unit test that verifies folder, token, and filename parameters are correctly passed to the service - Test will catch any future regressions where route parameters are not properly extracted
2025-11-19 14:12:34 +01:00
parent ca29357d37
commit bf638c3a4e
6 changed files with 108 additions and 36 deletions
@@ -1,6 +1,8 @@
 import { type CanActivate } from '@nestjs/common';
 import { Test, type TestingModule } from '@nestjs/testing';

+import { Readable } from 'stream';
+
 import { FileApiExceptionFilter } from 'src/engine/core-modules/file/filters/file-api-exception.filter';
 import { FilePathGuard } from 'src/engine/core-modules/file/guards/file-path-guard';
 import { FileService } from 'src/engine/core-modules/file/services/file.service';
@@ -9,6 +11,7 @@ import { FileController } from './file.controller';

 describe('FileController', () => {
  let controller: FileController;
+  let fileService: FileService;
  const mock_FilePathGuard: CanActivate = { canActivate: jest.fn(() => true) };

  beforeEach(async () => {
@@ -17,7 +20,9 @@ describe('FileController', () => {
      providers: [
        {
          provide: FileService,
-          useValue: {},
+          useValue: {
+            getFileStream: jest.fn(),
+          },
        },
      ],
    })
@@ -28,9 +33,60 @@ describe('FileController', () => {
      .compile();

    controller = module.get<FileController>(FileController);
+    fileService = module.get<FileService>(FileService);
  });

  it('should be defined', () => {
    expect(controller).toBeDefined();
  });
+
+  it('should extract folder, token and filename from 3-segment path', async () => {
+    const mockStream = new Readable();
+
+    mockStream.push('file content');
+    mockStream.push(null);
+    mockStream.pipe = jest.fn();
+
+    jest.spyOn(fileService, 'getFileStream').mockResolvedValue(mockStream);
+
+    const mockRequest = {
+      path: '/files/attachment/test-token/test-file.csv',
+      workspaceId: 'workspace-id',
+    } as any;
+
+    const mockResponse = {} as any;
+
+    await controller.getFile(mockResponse, mockRequest);
+
+    expect(fileService.getFileStream).toHaveBeenCalledWith(
+      'attachment',
+      'test-file.csv',
+      'workspace-id',
+    );
+  });
+
+  it('should extract folder with size, token and filename from 4-segment path', async () => {
+    const mockStream = new Readable();
+
+    mockStream.push('file content');
+    mockStream.push(null);
+    mockStream.pipe = jest.fn();
+
+    jest.spyOn(fileService, 'getFileStream').mockResolvedValue(mockStream);
+
+    const mockRequest = {
+      path: '/files/profile-picture/original/test-token/avatar.jpg',
+      workspaceId: 'workspace-id',
+    } as any;
+
+    const mockResponse = {} as any;
+
+    await controller.getFile(mockResponse, mockRequest);
+
+    expect(fileService.getFileStream).toHaveBeenCalledWith(
+      'profile-picture/original',
+      'avatar.jpg',
+      'workspace-id',
+    );
+  });
 });
@@ -1,7 +1,6 @@
 import {
  Controller,
  Get,
-  Param,
  Req,
  Res,
  UseFilters,
@@ -24,25 +23,19 @@ import { FilePathGuard } from 'src/engine/core-modules/file/guards/file-path-gua
 import { FileService } from 'src/engine/core-modules/file/services/file.service';
 import { extractFileInfoFromRequest } from 'src/engine/core-modules/file/utils/extract-file-info-from-request.utils';
 import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard';
-import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard';

@Controller('files')
@UseFilters(FileApiExceptionFilter)
-@UseGuards(FilePathGuard)
 export class FileController {
  constructor(private readonly fileService: FileService) {}

-  @Get('*path/:filename')
-  @UseGuards(PublicEndpointGuard, NoPermissionGuard)
-  async getFile(
-    @Param() _params: string[],
-    @Res() res: Response,
-    @Req() req: Request,
-  ) {
+  @Get('*')
+  @UseGuards(FilePathGuard, NoPermissionGuard)
+  async getFile(@Res() res: Response, @Req() req: Request) {
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const workspaceId = (req as any)?.workspaceId;

-    const { filename, rawFolder } = extractFileInfoFromRequest(req);
+    const { rawFolder, filename } = extractFileInfoFromRequest(req);

    try {
      const fileStream = await this.fileService.getFileStream(
@@ -1,7 +1,7 @@
 import { type Request } from 'express';

-import { checkFilename } from 'src/engine/core-modules/file/utils/check-file-name.utils';
 import { checkFileFolder } from 'src/engine/core-modules/file/utils/check-file-folder.utils';
+import { checkFilename } from 'src/engine/core-modules/file/utils/check-file-name.utils';
 import { extractFileInfoFromRequest } from 'src/engine/core-modules/file/utils/extract-file-info-from-request.utils';

 jest.mock('src/engine/core-modules/file/utils/check-file-name.utils', () => ({
@@ -16,33 +16,53 @@ jest.mock(
  'src/engine/core-modules/file/interfaces/file-folder.interface',
  () => ({
    fileFolderConfigs: {
-      'some-folder': { ignoreExpirationToken: true },
+      attachment: { ignoreExpirationToken: false },
+      'profile-picture': { ignoreExpirationToken: true },
    },
  }),
 );

 describe('extractFileInfoFromRequest', () => {
-  it('should extract all file info correctly from request', () => {
+  it('should extract all file info correctly from 3-segment path', () => {
    const mockRequest = {
-      params: {
-        filename: 'myfile.txt',
-        '0': 'some-folder/some-subfolder/filesig123',
-      },
+      path: '/files/attachment/filesig123/myfile.txt',
    } as unknown as Request;

    (checkFilename as jest.Mock).mockReturnValue('validated-file.txt');
-    (checkFileFolder as jest.Mock).mockReturnValue('some-folder');
+    (checkFileFolder as jest.Mock).mockReturnValue('attachment');

    const result = extractFileInfoFromRequest(mockRequest);

    expect(checkFilename).toHaveBeenCalledWith('myfile.txt');
-    expect(checkFileFolder).toHaveBeenCalledWith('some-folder/some-subfolder');
+    expect(checkFileFolder).toHaveBeenCalledWith('attachment');

    expect(result).toEqual({
      filename: 'validated-file.txt',
      fileSignature: 'filesig123',
-      rawFolder: 'some-folder/some-subfolder',
-      fileFolder: 'some-folder',
+      rawFolder: 'attachment',
+      fileFolder: 'attachment',
+      ignoreExpirationToken: false,
+    });
+  });
+
+  it('should extract all file info correctly from 4-segment path with size', () => {
+    const mockRequest = {
+      path: '/files/profile-picture/original/filesig456/avatar.jpg',
+    } as unknown as Request;
+
+    (checkFilename as jest.Mock).mockReturnValue('validated-avatar.jpg');
+    (checkFileFolder as jest.Mock).mockReturnValue('profile-picture');
+
+    const result = extractFileInfoFromRequest(mockRequest);
+
+    expect(checkFilename).toHaveBeenCalledWith('avatar.jpg');
+    expect(checkFileFolder).toHaveBeenCalledWith('profile-picture/original');
+
+    expect(result).toEqual({
+      filename: 'validated-avatar.jpg',
+      fileSignature: 'filesig456',
+      rawFolder: 'profile-picture/original',
+      fileFolder: 'profile-picture',
      ignoreExpirationToken: true,
    });
  });
@@ -6,13 +6,15 @@ import { checkFileFolder } from 'src/engine/core-modules/file/utils/check-file-f
 import { checkFilename } from 'src/engine/core-modules/file/utils/check-file-name.utils';

 export const extractFileInfoFromRequest = (request: Request) => {
-  const filename = checkFilename(request.params.filename);
+  // Ex: /files/profile-picture/original/TOKEN/file.jpg
+  const pathSegments = request.path.split('/').filter((segment) => segment);

-  const parts = request.params[0].split('/');
+  const segments = pathSegments.slice(1);

-  const fileSignature = parts.pop();
-
-  const rawFolder = parts.join('/');
+  const filename = checkFilename(segments[segments.length - 1]);
+  const fileSignature = segments[segments.length - 2];
+  const folderSegments = segments.slice(0, segments.length - 2);
+  const rawFolder = folderSegments.join('/');

  const fileFolder = checkFileFolder(rawFolder);