Skip to content

Commit

Permalink
do validation for import path
Browse files Browse the repository at this point in the history
  • Loading branch information
mertalev committed Mar 15, 2024
1 parent 91e8cf5 commit be4f864
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 36 deletions.
2 changes: 1 addition & 1 deletion server/src/domain/library/library.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class ValidateLibraryResponseDto {

export class ValidateLibraryImportPathResponseDto {
importPath!: string;
isValid?: boolean = false;
isValid: boolean = false;
message?: string;
}

Expand Down
55 changes: 28 additions & 27 deletions server/src/domain/library/library.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import {
userStub,
} from '@test';
import { when } from 'jest-when';
import { R_OK } from 'node:constants';
import { Stats } from 'node:fs';
import { join } from 'node:path';
import { ILibraryFileJob, ILibraryRefreshJob, JobName } from '../job';
import {
IAssetRepository,
Expand All @@ -32,7 +32,6 @@ import {
JobStatus,
StorageEventType,
} from '../repositories';
import { ENCODED_VIDEO_DIR, THUMBNAIL_DIR } from '../storage/storage.core';
import { SystemConfigCore } from '../system-config/system-config.core';
import { mapLibrary } from './library.dto';
import { LibraryService } from './library.service';
Expand Down Expand Up @@ -293,31 +292,6 @@ describe(LibraryService.name, () => {
expect(assetMock.updateAll).not.toHaveBeenCalledWith(expect.anything(), { isOffline: true });
expect(jobMock.queueAll).not.toHaveBeenCalled();
});

it('should ignore generated assets', async () => {
const mockLibraryJob: ILibraryRefreshJob = {
id: libraryStub.externalLibrary1.id,
refreshModifiedFiles: false,
refreshAllFiles: false,
};

libraryMock.get.mockResolvedValue(libraryStub.externalLibrary1);
// eslint-disable-next-line @typescript-eslint/require-await
storageMock.walk.mockImplementation(async function* generator() {
yield join(THUMBNAIL_DIR, 'path', 'to', 'photo.jpg');
yield assetStub.image.originalPath;
yield join(ENCODED_VIDEO_DIR, 'path', 'to', 'video.mp4');
});
assetMock.getLibraryAssetPaths.mockResolvedValue({
items: [assetStub.image],
hasNextPage: false,
});

await sut.handleQueueAssetRefresh(mockLibraryJob);

expect(assetMock.updateAll).not.toHaveBeenCalled();
expect(jobMock.queueAll).not.toHaveBeenCalled();
});
});

describe('handleAssetRefresh', () => {
Expand Down Expand Up @@ -1659,5 +1633,32 @@ describe(LibraryService.name, () => {
},
]);
});

it('should detect when import path is in immich media folder', async () => {
storageMock.stat.mockResolvedValue({ isDirectory: () => true } as Stats);
const validImport = libraryStub.hasImmichPaths.importPaths[1];
when(storageMock.checkFileExists).calledWith(validImport, R_OK).mockResolvedValue(true);

const result = await sut.validate(authStub.external1, libraryStub.hasImmichPaths.id, {
importPaths: libraryStub.hasImmichPaths.importPaths,
});

expect(result.importPaths).toEqual([
{
importPath: libraryStub.hasImmichPaths.importPaths[0],
isValid: false,
message: 'Cannot use media upload folder for external libraries',
},
{
importPath: validImport,
isValid: true,
},
{
importPath: libraryStub.hasImmichPaths.importPaths[2],
isValid: false,
message: 'Cannot use media upload folder for external libraries',
},
]);
});
});
});
15 changes: 7 additions & 8 deletions server/src/domain/library/library.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,13 @@ export class LibraryService extends EventEmitter {
const validation = new ValidateLibraryImportPathResponseDto();
validation.importPath = importPath;

if (StorageCore.isImmichPath(importPath)) {
validation.message = 'Cannot use media upload folder for external libraries';
return validation;
}

try {
const stat = await this.storageRepository.stat(importPath);

if (!stat.isDirectory()) {
validation.message = 'Not a directory';
return validation;
Expand Down Expand Up @@ -679,13 +683,13 @@ export class LibraryService extends EventEmitter {
this.logger.debug(`Will import ${crawledAssetPaths.size} new asset(s)`);
}

const batch = [];
let batch = [];
for (const assetPath of crawledAssetPaths) {
batch.push(assetPath);

if (batch.length >= LIBRARY_SCAN_BATCH_SIZE) {
await this.scanAssets(job.id, batch, library.ownerId, job.refreshAllFiles ?? false);
batch.length = 0;
batch = [];
}
}

Expand Down Expand Up @@ -721,11 +725,6 @@ export class LibraryService extends EventEmitter {

const trie = new Trie<string>();
for await (const filePath of generator) {
// avoid feedback loop
if (StorageCore.isGeneratedAsset(filePath)) {
continue;
}

trie.add(filePath);
}

Expand Down
16 changes: 16 additions & 0 deletions server/test/fixtures/library.stub.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { APP_MEDIA_LOCATION, THUMBNAIL_DIR } from '@app/domain';
import { LibraryEntity, LibraryType } from '@app/infra/entities';
import { join } from 'node:path';
import { userStub } from './user.stub';

export const libraryStub = {
Expand Down Expand Up @@ -100,4 +102,18 @@ export const libraryStub = {
isVisible: true,
exclusionPatterns: ['**/dir1/**'],
}),
hasImmichPaths: Object.freeze<LibraryEntity>({
id: 'library-id1337',
name: 'importpath-exclusion-library1',
assets: [],
owner: userStub.admin,
ownerId: 'user-id',
type: LibraryType.EXTERNAL,
importPaths: [join(THUMBNAIL_DIR, 'library'), '/xyz', join(APP_MEDIA_LOCATION, 'library')],
createdAt: new Date('2023-01-01'),
updatedAt: new Date('2023-01-01'),
refreshedAt: null,
isVisible: true,
exclusionPatterns: ['**/dir1/**'],
}),
};

0 comments on commit be4f864

Please sign in to comment.