🐛 fix(ai-image): save config.imageUrl with fullUrl instead of key (#9016)

This commit is contained in:
YuTengjing
2025-09-01 11:59:51 +08:00
committed by GitHub
parent 987e87ad8b
commit bad009a43c
10 changed files with 466 additions and 15 deletions
+4 -4
View File
@@ -59,9 +59,9 @@
"**/src/**/route.ts": "${dirname(1)}/${dirname} • route",
"**/src/**/index.tsx": "${dirname} • component",
"**/src/database/repositories/*/index.ts": "${dirname} • db repository",
"**/src/database/models/*.ts": "${filename} • db model",
"**/src/database/schemas/*.ts": "${filename} • db schema",
"**/packages/database/src/repositories/*/index.ts": "${dirname} • db repository",
"**/packages/database/src/models/*.ts": "${filename} • db model",
"**/packages/database/src/schemas/*.ts": "${filename} • db schema",
"**/src/services/*.ts": "${filename} • service",
"**/src/services/*/client.ts": "${dirname} • client service",
@@ -81,7 +81,7 @@
"**/src/store/*/slices/*/reducer.ts": "${dirname(2)}/${dirname} • reducer",
"**/src/config/modelProviders/*.ts": "${filename} • provider",
"**/packages/model-bank/src/aiModels/aiModels/*.ts": "${filename} • model",
"**/packages/model-bank/src/aiModels/*.ts": "${filename} • model",
"**/packages/model-runtime/src/*/index.ts": "${dirname} • runtime",
"**/src/server/services/*/index.ts": "${dirname} • server/service",
@@ -45,8 +45,8 @@ export const DEFAULT_DIMENSION_CONSTRAINTS = {
} as const;
export const CHAT_MODEL_IMAGE_GENERATION_PARAMS: ModelParamsSchema = {
imageUrl: {
default: null,
imageUrls: {
default: [],
},
prompt: { default: '' },
};
@@ -0,0 +1,138 @@
import { describe, expect, it } from 'vitest';
// Copy of the validation function from image.ts for testing
function validateNoUrlsInConfig(obj: any, path: string = ''): void {
if (typeof obj === 'string') {
if (obj.startsWith('http://') || obj.startsWith('https://')) {
throw new Error(
`Invalid configuration: Found full URL instead of key at ${path || 'root'}. ` +
`URL: "${obj.slice(0, 100)}${obj.length > 100 ? '...' : ''}". ` +
`All URLs must be converted to storage keys before database insertion.`,
);
}
} else if (Array.isArray(obj)) {
obj.forEach((item, index) => {
validateNoUrlsInConfig(item, `${path}[${index}]`);
});
} else if (obj && typeof obj === 'object') {
Object.entries(obj).forEach(([key, value]) => {
const currentPath = path ? `${path}.${key}` : key;
validateNoUrlsInConfig(value, currentPath);
});
}
}
describe('imageRouter', () => {
describe('validateNoUrlsInConfig utility', () => {
describe('valid configurations', () => {
it('should pass with normal keys', () => {
const config = {
imageUrl: 'images/photo.jpg',
imageUrls: ['files/doc.pdf', 'assets/video.mp4'],
prompt: 'Generate an image',
};
expect(() => validateNoUrlsInConfig(config)).not.toThrow();
});
it('should pass with empty strings', () => {
const config = {
imageUrl: '',
imageUrls: [],
prompt: 'Generate an image',
};
expect(() => validateNoUrlsInConfig(config)).not.toThrow();
});
it('should pass with null/undefined values', () => {
const config = {
imageUrl: null,
imageUrls: undefined,
prompt: 'Generate an image',
};
expect(() => validateNoUrlsInConfig(config)).not.toThrow();
});
});
describe('invalid configurations', () => {
it('should throw for https URL in imageUrl', () => {
const config = {
imageUrl: 'https://s3.amazonaws.com/bucket/image.jpg',
prompt: 'Generate an image',
};
expect(() => validateNoUrlsInConfig(config)).toThrow(
'Invalid configuration: Found full URL instead of key at imageUrl',
);
});
it('should throw for http URL in imageUrls array', () => {
const config = {
imageUrls: ['files/doc.pdf', 'http://example.com/image.jpg'],
prompt: 'Generate an image',
};
expect(() => validateNoUrlsInConfig(config)).toThrow(
'Invalid configuration: Found full URL instead of key at imageUrls[1]',
);
});
it('should throw for nested URL in complex object', () => {
const config = {
settings: {
imageConfig: {
url: 'https://cdn.example.com/very-long-url-that-exceeds-100-characters-to-test-truncation-functionality.jpg',
},
},
};
expect(() => validateNoUrlsInConfig(config)).toThrow(
'Invalid configuration: Found full URL instead of key at settings.imageConfig.url',
);
expect(() => validateNoUrlsInConfig(config)).toThrow(
'https://cdn.example.com/very-long-url-that-exceeds-100-characters-to-test-truncation-func',
);
});
it('should throw for presigned URL with query parameters', () => {
const config = {
imageUrl:
'https://s3.amazonaws.com/bucket/file.jpg?X-Amz-Signature=abc&X-Amz-Expires=3600',
};
expect(() => validateNoUrlsInConfig(config)).toThrow(
'All URLs must be converted to storage keys before database insertion',
);
});
});
describe('edge cases', () => {
it('should handle deeply nested structures', () => {
const config = {
level1: {
level2: {
level3: {
level4: ['normal-key', 'https://bad-url.com'],
},
},
},
};
expect(() => validateNoUrlsInConfig(config)).toThrow(
'Invalid configuration: Found full URL instead of key at level1.level2.level3.level4[1]',
);
});
it('should not throw for strings that contain but do not start with http', () => {
const config = {
imageUrl: 'some-prefix-https://example.com',
description: 'This text contains http:// but is not a URL',
};
expect(() => validateNoUrlsInConfig(config)).not.toThrow();
});
});
});
});
+42 -4
View File
@@ -24,6 +24,31 @@ import { generateUniqueSeeds } from '@/utils/number';
const log = debug('lobe-image:lambda');
/**
* Recursively validate that no full URLs are present in the config
* This is a defensive check to ensure only keys are stored in database
*/
function validateNoUrlsInConfig(obj: any, path: string = ''): void {
if (typeof obj === 'string') {
if (obj.startsWith('http://') || obj.startsWith('https://')) {
throw new Error(
`Invalid configuration: Found full URL instead of key at ${path || 'root'}. ` +
`URL: "${obj.slice(0, 100)}${obj.length > 100 ? '...' : ''}". ` +
`All URLs must be converted to storage keys before database insertion.`,
);
}
} else if (Array.isArray(obj)) {
obj.forEach((item, index) => {
validateNoUrlsInConfig(item, `${path}[${index}]`);
});
} else if (obj && typeof obj === 'object') {
Object.entries(obj).forEach(([key, value]) => {
const currentPath = path ? `${path}.${key}` : key;
validateNoUrlsInConfig(value, currentPath);
});
}
}
const imageProcedure = authedProcedure
.use(keyVaults)
.use(serverDatabase)
@@ -71,8 +96,9 @@ export const imageRouter = router({
log('Starting image creation process, input: %O', input);
// 如果 params 中包含 imageUrls,将它们转换为 S3 keys 用于数据库存储
// 规范化参考图地址,统一存储 S3 key(避免把会过期的预签名 URL 存进数据库
let configForDatabase = { ...params };
// 1) 处理多图 imageUrls
if (Array.isArray(params.imageUrls) && params.imageUrls.length > 0) {
log('Converting imageUrls to S3 keys for database storage: %O', params.imageUrls);
try {
@@ -82,18 +108,30 @@ export const imageRouter = router({
return key;
});
// 将转换后的 keys 存储为数据库配置
configForDatabase = {
...params,
...configForDatabase,
imageUrls: imageKeys,
};
log('Successfully converted imageUrls to keys for database: %O', imageKeys);
} catch (error) {
log('Error converting imageUrls to keys: %O', error);
// 如果转换失败,保持原始 URLs(可能是本地文件或其他格式)
log('Keeping original imageUrls due to conversion error');
}
}
// 2) 处理单图 imageUrl
if (typeof params.imageUrl === 'string' && params.imageUrl) {
try {
const key = fileService.getKeyFromFullUrl(params.imageUrl);
log('Converted single imageUrl to key: %s -> %s', params.imageUrl, key);
configForDatabase = { ...configForDatabase, imageUrl: key };
} catch (error) {
log('Error converting imageUrl to key: %O', error);
// 转换失败则保留原始值
}
}
// 防御性检测:确保没有完整URL进入数据库
validateNoUrlsInConfig(configForDatabase, 'configForDatabase');
// 步骤 1: 在事务中原子性地创建所有数据库记录
const { batch: createdBatch, generationsWithTasks } = await serverDB.transaction(async (tx) => {
+46 -1
View File
@@ -1,5 +1,4 @@
import { existsSync, readFileSync } from 'node:fs';
import path from 'node:path';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { electronIpcClient } from '@/server/modules/ElectronIPCClient';
@@ -234,6 +233,52 @@ describe('DesktopLocalFileImpl', () => {
// 验证
expect(result).toBe('');
});
// Legacy bug compatibility tests - https://github.com/lobehub/lobe-chat/issues/8994
describe('legacy bug compatibility', () => {
it('should handle full URL input by extracting key', async () => {
const fullUrl = 'http://localhost:3000/desktop-file/documents/test.txt';
// Mock getKeyFromFullUrl and getLocalFileUrl
vi.spyOn(service, 'getKeyFromFullUrl').mockReturnValue('desktop://documents/test.txt');
const getLocalFileUrlSpy = vi.spyOn(service as any, 'getLocalFileUrl');
getLocalFileUrlSpy.mockResolvedValueOnce('data:image/png;base64,test');
const result = await service.getFullFileUrl(fullUrl);
expect(service.getKeyFromFullUrl).toHaveBeenCalledWith(fullUrl);
expect(getLocalFileUrlSpy).toHaveBeenCalledWith('desktop://documents/test.txt');
expect(result).toBe('data:image/png;base64,test');
});
it('should handle normal desktop:// key input without extraction', async () => {
const key = 'desktop://documents/test.txt';
const extractSpy = vi.spyOn(service, 'getKeyFromFullUrl');
const getLocalFileUrlSpy = vi.spyOn(service as any, 'getLocalFileUrl');
getLocalFileUrlSpy.mockResolvedValueOnce('data:image/png;base64,test');
const result = await service.getFullFileUrl(key);
expect(extractSpy).not.toHaveBeenCalled();
expect(getLocalFileUrlSpy).toHaveBeenCalledWith(key);
expect(result).toBe('data:image/png;base64,test');
});
it('should handle https:// URLs for legacy compatibility', async () => {
const httpsUrl = 'https://localhost:3000/desktop-file/images/photo.jpg';
vi.spyOn(service, 'getKeyFromFullUrl').mockReturnValue('desktop://images/photo.jpg');
const getLocalFileUrlSpy = vi.spyOn(service as any, 'getLocalFileUrl');
getLocalFileUrlSpy.mockResolvedValueOnce('data:image/jpeg;base64,test');
const result = await service.getFullFileUrl(httpsUrl);
expect(service.getKeyFromFullUrl).toHaveBeenCalledWith(httpsUrl);
expect(getLocalFileUrlSpy).toHaveBeenCalledWith('desktop://images/photo.jpg');
expect(result).toBe('data:image/jpeg;base64,test');
});
});
});
describe('uploadContent', () => {
+6 -1
View File
@@ -6,6 +6,7 @@ import { electronIpcClient } from '@/server/modules/ElectronIPCClient';
import { inferContentTypeFromImageUrl } from '@/utils/url';
import { FileServiceImpl } from './type';
import { extractKeyFromUrlOrReturnOriginal } from './utils';
/**
* 桌面应用本地文件服务实现
@@ -127,7 +128,11 @@ export class DesktopLocalFileImpl implements FileServiceImpl {
*/
async getFullFileUrl(url?: string | null): Promise<string> {
if (!url) return '';
return this.getLocalFileUrl(url);
// Handle legacy data compatibility using shared utility
const key = extractKeyFromUrlOrReturnOriginal(url, this.getKeyFromFullUrl.bind(this));
return this.getLocalFileUrl(key);
}
/**
+50
View File
@@ -65,6 +65,56 @@ describe('S3StaticFileImpl', () => {
);
config.S3_ENABLE_PATH_STYLE = false;
});
// Legacy bug compatibility tests - https://github.com/lobehub/lobe-chat/issues/8994
describe('legacy bug compatibility', () => {
it('should handle full URL input by extracting key (S3_SET_ACL=false)', async () => {
config.S3_SET_ACL = false;
const fullUrl = 'https://s3.example.com/bucket/path/to/file.jpg?X-Amz-Signature=expired';
// Mock getKeyFromFullUrl to return the extracted key
vi.spyOn(fileService, 'getKeyFromFullUrl').mockReturnValue('path/to/file.jpg');
const result = await fileService.getFullFileUrl(fullUrl);
expect(fileService.getKeyFromFullUrl).toHaveBeenCalledWith(fullUrl);
expect(result).toBe('https://presigned.example.com/test.jpg');
config.S3_SET_ACL = true;
});
it('should handle full URL input by extracting key (S3_SET_ACL=true)', async () => {
const fullUrl = 'https://s3.example.com/bucket/path/to/file.jpg';
vi.spyOn(fileService, 'getKeyFromFullUrl').mockReturnValue('path/to/file.jpg');
const result = await fileService.getFullFileUrl(fullUrl);
expect(fileService.getKeyFromFullUrl).toHaveBeenCalledWith(fullUrl);
expect(result).toBe('https://example.com/path/to/file.jpg');
});
it('should handle normal key input without extraction', async () => {
const key = 'path/to/file.jpg';
const spy = vi.spyOn(fileService, 'getKeyFromFullUrl');
const result = await fileService.getFullFileUrl(key);
expect(spy).not.toHaveBeenCalled();
expect(result).toBe('https://example.com/path/to/file.jpg');
});
it('should handle http:// URLs for legacy compatibility', async () => {
const httpUrl = 'http://s3.example.com/bucket/path/to/file.jpg';
vi.spyOn(fileService, 'getKeyFromFullUrl').mockReturnValue('path/to/file.jpg');
const result = await fileService.getFullFileUrl(httpUrl);
expect(fileService.getKeyFromFullUrl).toHaveBeenCalledWith(httpUrl);
expect(result).toBe('https://example.com/path/to/file.jpg');
});
});
});
describe('getFileContent', () => {
+7 -3
View File
@@ -4,6 +4,7 @@ import { fileEnv } from '@/config/file';
import { S3 } from '@/server/modules/S3';
import { FileServiceImpl } from './type';
import { extractKeyFromUrlOrReturnOriginal } from './utils';
/**
* 基于S3的文件服务实现
@@ -46,16 +47,19 @@ export class S3StaticFileImpl implements FileServiceImpl {
async getFullFileUrl(url?: string | null, expiresIn?: number): Promise<string> {
if (!url) return '';
// Handle legacy data compatibility using shared utility
const key = extractKeyFromUrlOrReturnOriginal(url, this.getKeyFromFullUrl.bind(this));
// If bucket is not set public read, the preview address needs to be regenerated each time
if (!fileEnv.S3_SET_ACL) {
return await this.createPreSignedUrlForPreview(url, expiresIn);
return await this.createPreSignedUrlForPreview(key, expiresIn);
}
if (fileEnv.S3_ENABLE_PATH_STYLE) {
return urlJoin(fileEnv.S3_PUBLIC_DOMAIN!, fileEnv.S3_BUCKET!, url);
return urlJoin(fileEnv.S3_PUBLIC_DOMAIN!, fileEnv.S3_BUCKET!, key);
}
return urlJoin(fileEnv.S3_PUBLIC_DOMAIN!, url);
return urlJoin(fileEnv.S3_PUBLIC_DOMAIN!, key);
}
getKeyFromFullUrl(url: string): string {
@@ -0,0 +1,154 @@
import { describe, expect, it, vi } from 'vitest';
import { extractKeyFromUrlOrReturnOriginal } from './utils';
describe('extractKeyFromUrlOrReturnOriginal', () => {
const mockGetKeyFromFullUrl = vi.fn();
beforeEach(() => {
vi.clearAllMocks();
});
describe('URL detection', () => {
it('should detect https:// URLs and extract key', () => {
const httpsUrl = 'https://s3.example.com/bucket/path/to/file.jpg';
const expectedKey = 'path/to/file.jpg';
mockGetKeyFromFullUrl.mockReturnValue(expectedKey);
const result = extractKeyFromUrlOrReturnOriginal(httpsUrl, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).toHaveBeenCalledWith(httpsUrl);
expect(result).toBe(expectedKey);
});
it('should detect http:// URLs and extract key', () => {
const httpUrl = 'http://s3.example.com/bucket/path/to/file.jpg';
const expectedKey = 'path/to/file.jpg';
mockGetKeyFromFullUrl.mockReturnValue(expectedKey);
const result = extractKeyFromUrlOrReturnOriginal(httpUrl, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).toHaveBeenCalledWith(httpUrl);
expect(result).toBe(expectedKey);
});
it('should handle presigned URLs with query parameters', () => {
const presignedUrl = 'https://s3.amazonaws.com/bucket/file.jpg?X-Amz-Signature=abc&X-Amz-Expires=3600';
const expectedKey = 'file.jpg';
mockGetKeyFromFullUrl.mockReturnValue(expectedKey);
const result = extractKeyFromUrlOrReturnOriginal(presignedUrl, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).toHaveBeenCalledWith(presignedUrl);
expect(result).toBe(expectedKey);
});
});
describe('key passthrough', () => {
it('should return original string for plain keys', () => {
const key = 'path/to/file.jpg';
const result = extractKeyFromUrlOrReturnOriginal(key, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).not.toHaveBeenCalled();
expect(result).toBe(key);
});
it('should return original string for desktop:// keys', () => {
const desktopKey = 'desktop://documents/file.pdf';
const result = extractKeyFromUrlOrReturnOriginal(desktopKey, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).not.toHaveBeenCalled();
expect(result).toBe(desktopKey);
});
it('should return original string for relative paths', () => {
const relativePath = './assets/image.png';
const result = extractKeyFromUrlOrReturnOriginal(relativePath, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).not.toHaveBeenCalled();
expect(result).toBe(relativePath);
});
it('should return original string for file:// protocol', () => {
const fileUrl = 'file:///Users/test/file.txt';
const result = extractKeyFromUrlOrReturnOriginal(fileUrl, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).not.toHaveBeenCalled();
expect(result).toBe(fileUrl);
});
});
describe('edge cases', () => {
it('should handle empty string', () => {
const emptyString = '';
const result = extractKeyFromUrlOrReturnOriginal(emptyString, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).not.toHaveBeenCalled();
expect(result).toBe(emptyString);
});
it('should handle strings that start with http but are not URLs', () => {
const notUrl = 'httpish-string';
const result = extractKeyFromUrlOrReturnOriginal(notUrl, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).not.toHaveBeenCalled();
expect(result).toBe(notUrl);
});
it('should handle case sensitivity correctly', () => {
const upperCaseHttps = 'HTTPS://example.com/file.jpg';
const result = extractKeyFromUrlOrReturnOriginal(upperCaseHttps, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).not.toHaveBeenCalled();
expect(result).toBe(upperCaseHttps);
});
});
describe('legacy bug scenarios', () => {
it('should handle S3 public URLs', () => {
const s3PublicUrl = 'https://mybucket.s3.amazonaws.com/images/photo.jpg';
const expectedKey = 'images/photo.jpg';
mockGetKeyFromFullUrl.mockReturnValue(expectedKey);
const result = extractKeyFromUrlOrReturnOriginal(s3PublicUrl, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).toHaveBeenCalledWith(s3PublicUrl);
expect(result).toBe(expectedKey);
});
it('should handle custom domain S3 URLs', () => {
const customDomainUrl = 'https://cdn.example.com/files/document.pdf';
const expectedKey = 'files/document.pdf';
mockGetKeyFromFullUrl.mockReturnValue(expectedKey);
const result = extractKeyFromUrlOrReturnOriginal(customDomainUrl, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).toHaveBeenCalledWith(customDomainUrl);
expect(result).toBe(expectedKey);
});
it('should handle local development URLs', () => {
const localUrl = 'http://localhost:3000/desktop-file/images/screenshot.png';
const expectedKey = 'desktop://images/screenshot.png';
mockGetKeyFromFullUrl.mockReturnValue(expectedKey);
const result = extractKeyFromUrlOrReturnOriginal(localUrl, mockGetKeyFromFullUrl);
expect(mockGetKeyFromFullUrl).toHaveBeenCalledWith(localUrl);
expect(result).toBe(expectedKey);
});
});
});
+17
View File
@@ -0,0 +1,17 @@
/**
* Handle legacy bug where full URLs were stored instead of keys
* Some historical data stored complete URLs in database instead of just keys
* Related issue: https://github.com/lobehub/lobe-chat/issues/8994
*/
export function extractKeyFromUrlOrReturnOriginal(
url: string,
getKeyFromFullUrl: (url: string) => string,
): string {
// Only process URLs that start with http:// or https://
if (url.startsWith('http://') || url.startsWith('https://')) {
// Extract key from full URL for legacy data compatibility
return getKeyFromFullUrl(url);
}
// Return original input if it's already a key
return url;
}