Skip to content

Commit d4c3dc3

Browse files
fix(uploads): address review feedback — input validation, stream safety, tests
- Fix Readable.from(buffer) → Readable.from([buffer]) to avoid byte-by-byte iteration - Add readable error handler for reliable Promise rejection on stream failures - Add Buffer.isBuffer validation with clear AppError (422) for invalid inputs - Add defensive Array.isArray check for kindConfig.formats - Extract MIME_TO_EXT map to module-level constant - Add tests for null/undefined buffer, non-Buffer input, and missing formats
1 parent 80117d7 commit d4c3dc3

3 files changed

Lines changed: 48 additions & 2 deletions

File tree

lib/services/gridfs.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ const createFromBuffer = (buffer, filename, contentType, metadata = {}) => new P
7575
const id = new mongoose.Types.ObjectId();
7676
const uploadStream = bucket.openUploadStreamWithId(id, filename, { contentType, metadata });
7777

78-
const readable = Readable.from(buffer);
78+
const readable = Readable.from([buffer]);
79+
readable.on('error', reject);
7980
readable.pipe(
8081
uploadStream
8182
.on('error', reject)

modules/uploads/services/uploads.service.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@ import multerService from '../../../lib/services/multer.js';
99
import gridfs from '../../../lib/services/gridfs.js';
1010
import UploadRepository from '../repositories/uploads.repository.js';
1111

12+
const MIME_TO_EXT = {
13+
'image/jpeg': 'jpeg',
14+
'image/jpg': 'jpg',
15+
'image/png': 'png',
16+
'image/gif': 'gif',
17+
'application/pdf': 'pdf',
18+
};
19+
1220
/**
1321
* @desc Function to ask repository to get an upload
1422
* @param {String} uploadName
@@ -67,9 +75,17 @@ const remove = async (upload) => {
6775
* @returns {Promise<Object>} The created upload document
6876
*/
6977
const createFromBuffer = async (buffer, contentType, kind, metadata = {}) => {
78+
if (!Buffer.isBuffer(buffer)) {
79+
throw new AppError('Upload: buffer is required and must be a Buffer', { code: 'SERVICE_ERROR', status: 422 });
80+
}
81+
7082
const kindConfig = config.uploads?.[kind];
7183
if (!kindConfig) throw new AppError(`Upload: unknown kind "${kind}"`, { code: 'SERVICE_ERROR', status: 422 });
7284

85+
if (!Array.isArray(kindConfig.formats)) {
86+
throw new AppError(`Upload: kind "${kind}" has no formats configured`, { code: 'SERVICE_ERROR', status: 500 });
87+
}
88+
7389
if (!kindConfig.formats.includes(contentType)) {
7490
throw new AppError(`Upload: content type "${contentType}" not allowed for kind "${kind}"`, { code: 'SERVICE_ERROR', status: 422 });
7591
}
@@ -78,7 +94,6 @@ const createFromBuffer = async (buffer, contentType, kind, metadata = {}) => {
7894
throw new AppError(`Upload: buffer size ${buffer.length} exceeds limit ${kindConfig.limits.fileSize}`, { code: 'SERVICE_ERROR', status: 422 });
7995
}
8096

81-
const MIME_TO_EXT = { 'image/jpeg': 'jpeg', 'image/jpg': 'jpg', 'image/png': 'png', 'image/gif': 'gif', 'application/pdf': 'pdf' };
8297
const ext = MIME_TO_EXT[contentType] || 'bin';
8398
const filename = `${crypto.randomBytes(32).toString('hex')}.${ext}`;
8499

modules/uploads/tests/uploads.createFromBuffer.unit.tests.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,4 +128,34 @@ describe('Uploads createFromBuffer unit tests:', () => {
128128

129129
expect(mockGridfs.createFromBuffer).not.toHaveBeenCalled();
130130
});
131+
132+
test('should throw error when buffer is null or undefined', async () => {
133+
await expect(
134+
UploadsService.createFromBuffer(null, 'image/jpeg', 'snapshot'),
135+
).rejects.toThrow(/buffer is required/);
136+
137+
await expect(
138+
UploadsService.createFromBuffer(undefined, 'image/jpeg', 'snapshot'),
139+
).rejects.toThrow(/buffer is required/);
140+
141+
expect(mockGridfs.createFromBuffer).not.toHaveBeenCalled();
142+
});
143+
144+
test('should throw error when buffer is not a Buffer', async () => {
145+
await expect(
146+
UploadsService.createFromBuffer('not a buffer', 'image/jpeg', 'snapshot'),
147+
).rejects.toThrow(/buffer is required/);
148+
149+
expect(mockGridfs.createFromBuffer).not.toHaveBeenCalled();
150+
});
151+
152+
test('should throw error when kind has no formats configured', async () => {
153+
mockConfig.uploads.broken = { kind: 'broken', limits: { fileSize: 1024 } };
154+
155+
await expect(
156+
UploadsService.createFromBuffer(Buffer.alloc(10), 'image/jpeg', 'broken'),
157+
).rejects.toThrow(/no formats configured/);
158+
159+
expect(mockGridfs.createFromBuffer).not.toHaveBeenCalled();
160+
});
131161
});

0 commit comments

Comments
 (0)