Skip to content
88 changes: 88 additions & 0 deletions src/modules/email/attachment-headers.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { describe, it, expect } from 'vitest';
import {
buildContentDisposition,
sanitizeFilename,
sanitizeMimeType,
} from './attachment-headers.js';

describe('attachment-headers', () => {
describe('sanitizeFilename', () => {
it('when given a regular filename, then it is returned untouched', () => {
expect(sanitizeFilename('photo.jpg')).toBe('photo.jpg');
});

it('when no filename is provided, then a generic fallback name is used', () => {
expect(sanitizeFilename(undefined)).toBe('attachment');
expect(sanitizeFilename('')).toBe('attachment');
});

it('when the filename only contains whitespace, then the generic fallback name is used', () => {
expect(sanitizeFilename(' ')).toBe('attachment');
});

it('when the filename contains line breaks or quotes, then those characters are stripped', () => {
expect(sanitizeFilename('photo\r\n.jpg')).toBe('photo.jpg');
expect(sanitizeFilename('a"b\\c.txt')).toBe('abc.txt');

Check warning on line 25 in src/modules/email/attachment-headers.spec.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

`String.raw` should be used to avoid escaping `\`.

See more on https://sonarcloud.io/project/issues?id=internxt_mail-server&issues=AZ6IcUOp59KNOHKleqFI&open=AZ6IcUOp59KNOHKleqFI&pullRequest=56
});

it('when the filename is very long, then it is shortened to a safe length', () => {
const longName = 'a'.repeat(500) + '.txt';
const result = sanitizeFilename(longName);
expect(result.length).toBe(255);
});

it('when the filename has non-ascii characters, then it preserves them', () => {
expect(sanitizeFilename('fôto-año.pdf')).toBe('fôto-año.pdf');
});
});

describe('sanitizeMimeType', () => {
it('when given a valid content type, then it is returned untouched', () => {
expect(sanitizeMimeType('image/jpeg')).toBe('image/jpeg');
expect(sanitizeMimeType('application/pdf')).toBe('application/pdf');
expect(sanitizeMimeType('application/vnd.ms-excel')).toBe(
'application/vnd.ms-excel',
);
});

it('when no content type is provided, then no value is returned', () => {
expect(sanitizeMimeType(undefined)).toBeNull();
expect(sanitizeMimeType('')).toBeNull();
});

it('when the content type is malformed, then it is rejected', () => {
expect(sanitizeMimeType('not-a-mime')).toBeNull();
expect(sanitizeMimeType('image/')).toBeNull();
expect(sanitizeMimeType('/jpeg')).toBeNull();
expect(sanitizeMimeType('image jpeg')).toBeNull();
});

it('when the content type contains suspicious characters, then it is rejected', () => {
expect(sanitizeMimeType('image/jpeg\r\nX-Injected: yes')).toBeNull();
expect(sanitizeMimeType('image/jpeg;charset=utf-8')).toBeNull();
});
});

describe('buildContentDisposition', () => {
it('when given a simple filename, then it is included both as ascii and utf-8', () => {
const result = buildContentDisposition('photo.jpg');
expect(result).toBe(
`attachment; filename="photo.jpg"; filename*=UTF-8''photo.jpg`,
);
});

it('when the filename has non-ascii characters, then the utf-8 part is percent-encoded', () => {
const result = buildContentDisposition('año.pdf');
expect(result).toBe(
`attachment; filename="año.pdf"; filename*=UTF-8''a%C3%B1o.pdf`,
);
});

it('when the filename has spaces, then the utf-8 part escapes them', () => {
const result = buildContentDisposition('my photo.jpg');
expect(result).toBe(
`attachment; filename="my photo.jpg"; filename*=UTF-8''my%20photo.jpg`,
);
});
});
});
18 changes: 18 additions & 0 deletions src/modules/email/attachment-headers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const MIME_TYPE_REGEX = /^[a-zA-Z0-9.+-]+\/[a-zA-Z0-9.+-]+$/;

export function sanitizeFilename(name: string | undefined): string {
if (!name) return 'attachment';
const cleaned = name.replace(/[\r\n"\\]/g, '').trim();
if (!cleaned) return 'attachment';
return cleaned.slice(0, 255);
}

export function sanitizeMimeType(type: string | undefined): string | null {
if (!type) return null;
return MIME_TYPE_REGEX.test(type) ? type : null;
}

export function buildContentDisposition(filename: string): string {
const encoded = encodeURIComponent(filename);
return `attachment; filename="${filename}"; filename*=UTF-8''${encoded}`;
}
114 changes: 113 additions & 1 deletion src/modules/email/email.controller.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { describe, it, expect, beforeEach, test } from 'vitest';
import { describe, it, expect, beforeEach, test, vi } from 'vitest';
import { Test, type TestingModule } from '@nestjs/testing';
import { createMock, type DeepMocked } from '@golevelup/ts-vitest';
import { Readable } from 'node:stream';
import type { Response } from 'express';
import { EmailController } from './email.controller.js';
import { EmailService } from './email.service.js';
import { AccountService } from '../account/account.service.js';
Expand All @@ -13,6 +15,12 @@ import {
import type { EmailListResponse } from './email.types.js';
import { MailDomain } from '../account/domain/mail-domain.domain.js';

function makeResponse(): Response {
return {
setHeader: vi.fn(),
} as unknown as Response;
}

describe('EmailController', () => {
let controller: EmailController;
let emailService: DeepMocked<EmailService>;
Expand Down Expand Up @@ -330,4 +338,108 @@ describe('EmailController', () => {
expect(emailService.uploadAttachment).not.toHaveBeenCalled();
});
});

describe('Downloading an attachment', () => {
test('when a user downloads an attachment, then the response carries the file bytes with the right content type, length and filename', async () => {
const stream = Readable.from(Buffer.from('binary'));
emailService.downloadAttachment.mockResolvedValue({
stream,
contentType: 'image/jpeg',
contentLength: 1234,
});
const res = makeResponse();

const result = await controller.downloadAttachment(
userEmail,
'email-1',
'blob-1',
'photo.jpg',
'image/jpeg',
res,
);

expect(emailService.downloadAttachment).toHaveBeenCalledWith({
userEmail,
blobId: 'blob-1',
name: 'photo.jpg',
type: 'image/jpeg',
});
expect(res.setHeader).toHaveBeenCalledWith('Content-Type', 'image/jpeg');
expect(res.setHeader).toHaveBeenCalledWith('Content-Length', 1234);
expect(res.setHeader).toHaveBeenCalledWith(
'Content-Disposition',
expect.stringContaining('filename="photo.jpg"'),
);
expect(result.getStream()).toBe(stream);
});

test('when the caller does not specify a content type, then the one reported by the storage is used', async () => {
emailService.downloadAttachment.mockResolvedValue({
stream: Readable.from(Buffer.from('x')),
contentType: 'application/pdf',
});
const res = makeResponse();

await controller.downloadAttachment(
userEmail,
'email-1',
'blob-1',
'doc.pdf',
undefined,
res,
);

expect(res.setHeader).toHaveBeenCalledWith(
'Content-Type',
'application/pdf',
);
});

test('when the storage does not report a size, then no length is sent to the caller', async () => {
emailService.downloadAttachment.mockResolvedValue({
stream: Readable.from(Buffer.from('x')),
contentType: 'application/octet-stream',
});
const res = makeResponse();

await controller.downloadAttachment(
userEmail,
'email-1',
'blob-1',
undefined,
undefined,
res,
);

expect(res.setHeader).not.toHaveBeenCalledWith(
'Content-Length',
expect.anything(),
);
});

test('when the caller provides a malformed content type, then it is discarded in favour of the one reported by the storage', async () => {
emailService.downloadAttachment.mockResolvedValue({
stream: Readable.from(Buffer.from('x')),
contentType: 'image/png',
});
const res = makeResponse();

await controller.downloadAttachment(
userEmail,
'email-1',
'blob-1',
'photo.png',
'not a mime',
res,
);

expect(emailService.downloadAttachment).toHaveBeenCalledWith({
userEmail,
blobId: 'blob-1',
name: 'photo.png',
type: undefined,
});
expect(res.setHeader).toHaveBeenCalledWith('Content-Type', 'image/png');
});
});
});
54 changes: 54 additions & 0 deletions src/modules/email/email.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ import {
Get,
HttpCode,
HttpStatus,
NotFoundException,
Param,
Patch,
Post,
Query,
Res,
StreamableFile,
UploadedFiles,
UseGuards,
UseInterceptors,
} from '@nestjs/common';
import type { Response } from 'express';
import {
ApiBadRequestResponse,
ApiBearerAuth,
Expand Down Expand Up @@ -48,6 +52,11 @@ import { AccountService } from '../account/account.service.js';
import { SkipMailAccountCheck } from '../provisioning/skip-mail-account-check.decorator.js';
import { FilesInterceptor } from '@nestjs/platform-express';
import { memoryStorage } from 'multer';
import {
buildContentDisposition,
sanitizeFilename,
sanitizeMimeType,
} from './attachment-headers.js';

export const MAX_TOTAL_BYTES = 25 * 1024 * 1024;

Expand Down Expand Up @@ -272,6 +281,51 @@ export class EmailController {
return { ...result, name: file.originalname };
}

@Get(':id/attachment/:blobId')
@ApiOperation({
summary: 'Download an attachment',
description:
'Streams the bytes of an attachment from the given email. ' +
'Optional `name` and `type` query params set the response filename and content-type.',
})
@ApiParam({ name: 'id', description: 'Email ID' })
@ApiParam({ name: 'blobId', description: 'Attachment blob ID' })
@ApiQuery({ name: 'name', required: false, example: 'photo.jpg' })
@ApiQuery({ name: 'type', required: false, example: 'image/jpeg' })
async downloadAttachment(
@MailAddress('address') email: string,
@Param('id') _id: string,
@Param('blobId') blobId: string,
@Query('name') name: string | undefined,
@Query('type') type: string | undefined,
@Res({ passthrough: true }) res: Response,
): Promise<StreamableFile> {
const safeName = sanitizeFilename(name);
const safeType = sanitizeMimeType(type);

const mail = await this.emailService.getEmail(email, _id);
if (!mail) throw new NotFoundException('Email not found');

const attachment = mail.attachments.find((a) => a.blobId === blobId);
if (!attachment) throw new NotFoundException('Attachment not found');

const result = await this.emailService.downloadAttachment({
userEmail: email,
blobId,
name: safeName,
type: safeType ?? undefined,
});

const resolvedType = safeType ?? result.contentType;

res.setHeader('Content-Type', resolvedType);
res.setHeader('Content-Disposition', buildContentDisposition(safeName));
if (result.contentLength !== undefined)
res.setHeader('Content-Length', result.contentLength);

return new StreamableFile(result.stream);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick controller logic should stay thin with no business logic. the res.setHeader stuff does belong here

}

@Patch(':id')
@HttpCode(HttpStatus.NO_CONTENT)
@ApiOperation({
Expand Down
20 changes: 20 additions & 0 deletions src/modules/email/email.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,20 @@ export class EncryptionBlockDto {
wrappedKeys!: EncryptedWrappedKeyDto[];
}

export class AttachmentRefDto {
@ApiProperty({ example: 'T1a2b3c…' })
blobId!: string;

@ApiProperty({ example: 'photo.jpg' })
name!: string;

@ApiProperty({ example: 'image/jpeg' })
type!: string;

@ApiProperty({ example: 4096, description: 'Size in bytes' })
size!: number;
}

export class SendEmailRequestDto {
@ApiProperty({
type: [EmailAddressDto],
Expand Down Expand Up @@ -91,6 +105,9 @@ export class SendEmailRequestDto {

@ApiPropertyOptional({ type: EncryptionBlockDto })
encryption?: EncryptionBlockDto;

@ApiPropertyOptional({ type: [AttachmentRefDto] })
attachments?: AttachmentRefDto[];
}

export class LookupRecipientKeysRequestDto {
Expand Down Expand Up @@ -141,6 +158,9 @@ export class DraftEmailRequestDto {

@ApiPropertyOptional({ example: '<p>Still working on this…</p>' })
htmlBody?: string;

@ApiPropertyOptional({ type: [AttachmentRefDto] })
attachments?: AttachmentRefDto[];
}

export class UpdateEmailRequestDto {
Expand Down
8 changes: 8 additions & 0 deletions src/modules/email/email.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import {
projectForCaller,
} from './email-encryption.js';
import {
DownloadAttachmentPayload,
DownloadAttachmentResponse,
UploadAttachmentPayload,
UploadAttachmentResponse,
} from '../infrastructure/jmap/jmap.types.js';
Expand Down Expand Up @@ -142,4 +144,10 @@ export class EmailService {
): Promise<UploadAttachmentResponse> {
return this.mail.uploadAttachment(payload);
}

downloadAttachment(
payload: DownloadAttachmentPayload,
): Promise<DownloadAttachmentResponse> {
return this.mail.downloadAttachment(payload);
}
}
2 changes: 2 additions & 0 deletions src/modules/email/email.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export interface SendEmailDto {
textBody?: string;
htmlBody?: string;
encryption?: EncryptionBlock;
attachments?: EmailAttachment[];
}

export interface DraftEmailDto {
Expand All @@ -96,6 +97,7 @@ export interface DraftEmailDto {
subject?: string;
textBody?: string;
htmlBody?: string;
attachments?: EmailAttachment[];
}

export interface SearchEmailDto {
Expand Down
Loading
Loading