Skip to content

Commit bad009a

Browse files
tjx666tjx666
authored
🐛 fix(ai-image): save config.imageUrl with fullUrl instead of key (lobehub#9016)
1 parent 987e87a commit bad009a

File tree

10 files changed

+466
-15
lines changed

10 files changed

+466
-15
lines changed

‎.vscode/settings.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@
5959
"**/src/**/route.ts": "${dirname(1)}/${dirname} • route",
6060
"**/src/**/index.tsx": "${dirname} • component",
6161

62-
"**/src/database/repositories/*/index.ts": "${dirname} • db repository",
63-
"**/src/database/models/*.ts": "${filename} • db model",
64-
"**/src/database/schemas/*.ts": "${filename} • db schema",
62+
"**/packages/database/src/repositories/*/index.ts": "${dirname} • db repository",
63+
"**/packages/database/src/models/*.ts": "${filename} • db model",
64+
"**/packages/database/src/schemas/*.ts": "${filename} • db schema",
6565

6666
"**/src/services/*.ts": "${filename} • service",
6767
"**/src/services/*/client.ts": "${dirname} • client service",
@@ -81,7 +81,7 @@
8181
"**/src/store/*/slices/*/reducer.ts": "${dirname(2)}/${dirname} • reducer",
8282

8383
"**/src/config/modelProviders/*.ts": "${filename} • provider",
84-
"**/packages/model-bank/src/aiModels/aiModels/*.ts": "${filename} • model",
84+
"**/packages/model-bank/src/aiModels/*.ts": "${filename} • model",
8585
"**/packages/model-runtime/src/*/index.ts": "${dirname} • runtime",
8686

8787
"**/src/server/services/*/index.ts": "${dirname} • server/service",

‎packages/model-bank/src/standard-parameters/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ export const DEFAULT_DIMENSION_CONSTRAINTS = {
4545
} as const;
4646

4747
export const CHAT_MODEL_IMAGE_GENERATION_PARAMS: ModelParamsSchema = {
48-
imageUrl: {
49-
default: null,
48+
imageUrls: {
49+
default: [],
5050
},
5151
prompt: { default: '' },
5252
};

‎src/server/routers/lambda/__tests__/image.test.ts‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
// Copy of the validation function from image.ts for testing
4+
function validateNoUrlsInConfig(obj: any, path: string = ''): void {
5+
if (typeof obj === 'string') {
6+
if (obj.startsWith('http://') || obj.startsWith('https://')) {
7+
throw new Error(
8+
`Invalid configuration: Found full URL instead of key at ${path || 'root'}. ` +
9+
`URL: "${obj.slice(0, 100)}${obj.length > 100 ? '...' : ''}". ` +
10+
`All URLs must be converted to storage keys before database insertion.`,
11+
);
12+
}
13+
} else if (Array.isArray(obj)) {
14+
obj.forEach((item, index) => {
15+
validateNoUrlsInConfig(item, `${path}[${index}]`);
16+
});
17+
} else if (obj && typeof obj === 'object') {
18+
Object.entries(obj).forEach(([key, value]) => {
19+
const currentPath = path ? `${path}.${key}` : key;
20+
validateNoUrlsInConfig(value, currentPath);
21+
});
22+
}
23+
}
24+
25+
describe('imageRouter', () => {
26+
describe('validateNoUrlsInConfig utility', () => {
27+
describe('valid configurations', () => {
28+
it('should pass with normal keys', () => {
29+
const config = {
30+
imageUrl: 'images/photo.jpg',
31+
imageUrls: ['files/doc.pdf', 'assets/video.mp4'],
32+
prompt: 'Generate an image',
33+
};
34+
35+
expect(() => validateNoUrlsInConfig(config)).not.toThrow();
36+
});
37+
38+
it('should pass with empty strings', () => {
39+
const config = {
40+
imageUrl: '',
41+
imageUrls: [],
42+
prompt: 'Generate an image',
43+
};
44+
45+
expect(() => validateNoUrlsInConfig(config)).not.toThrow();
46+
});
47+
48+
it('should pass with null/undefined values', () => {
49+
const config = {
50+
imageUrl: null,
51+
imageUrls: undefined,
52+
prompt: 'Generate an image',
53+
};
54+
55+
expect(() => validateNoUrlsInConfig(config)).not.toThrow();
56+
});
57+
});
58+
59+
describe('invalid configurations', () => {
60+
it('should throw for https URL in imageUrl', () => {
61+
const config = {
62+
imageUrl: 'https://s3.amazonaws.com/bucket/image.jpg',
63+
prompt: 'Generate an image',
64+
};
65+
66+
expect(() => validateNoUrlsInConfig(config)).toThrow(
67+
'Invalid configuration: Found full URL instead of key at imageUrl',
68+
);
69+
});
70+
71+
it('should throw for http URL in imageUrls array', () => {
72+
const config = {
73+
imageUrls: ['files/doc.pdf', 'http://example.com/image.jpg'],
74+
prompt: 'Generate an image',
75+
};
76+
77+
expect(() => validateNoUrlsInConfig(config)).toThrow(
78+
'Invalid configuration: Found full URL instead of key at imageUrls[1]',
79+
);
80+
});
81+
82+
it('should throw for nested URL in complex object', () => {
83+
const config = {
84+
settings: {
85+
imageConfig: {
86+
url: 'https://cdn.example.com/very-long-url-that-exceeds-100-characters-to-test-truncation-functionality.jpg',
87+
},
88+
},
89+
};
90+
91+
expect(() => validateNoUrlsInConfig(config)).toThrow(
92+
'Invalid configuration: Found full URL instead of key at settings.imageConfig.url',
93+
);
94+
expect(() => validateNoUrlsInConfig(config)).toThrow(
95+
'https://cdn.example.com/very-long-url-that-exceeds-100-characters-to-test-truncation-func',
96+
);
97+
});
98+
99+
it('should throw for presigned URL with query parameters', () => {
100+
const config = {
101+
imageUrl:
102+
'https://s3.amazonaws.com/bucket/file.jpg?X-Amz-Signature=abc&X-Amz-Expires=3600',
103+
};
104+
105+
expect(() => validateNoUrlsInConfig(config)).toThrow(
106+
'All URLs must be converted to storage keys before database insertion',
107+
);
108+
});
109+
});
110+
111+
describe('edge cases', () => {
112+
it('should handle deeply nested structures', () => {
113+
const config = {
114+
level1: {
115+
level2: {
116+
level3: {
117+
level4: ['normal-key', 'https://bad-url.com'],
118+
},
119+
},
120+
},
121+
};
122+
123+
expect(() => validateNoUrlsInConfig(config)).toThrow(
124+
'Invalid configuration: Found full URL instead of key at level1.level2.level3.level4[1]',
125+
);
126+
});
127+
128+
it('should not throw for strings that contain but do not start with http', () => {
129+
const config = {
130+
imageUrl: 'some-prefix-https://example.com',
131+
description: 'This text contains http:// but is not a URL',
132+
};
133+
134+
expect(() => validateNoUrlsInConfig(config)).not.toThrow();
135+
});
136+
});
137+
});
138+
});

‎src/server/routers/lambda/image.ts‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,31 @@ import { generateUniqueSeeds } from '@/utils/number';
2424

2525
const log = debug('lobe-image:lambda');
2626

27+
/**
28+
* Recursively validate that no full URLs are present in the config
29+
* This is a defensive check to ensure only keys are stored in database
30+
*/
31+
function validateNoUrlsInConfig(obj: any, path: string = ''): void {
32+
if (typeof obj === 'string') {
33+
if (obj.startsWith('http://') || obj.startsWith('https://')) {
34+
throw new Error(
35+
`Invalid configuration: Found full URL instead of key at ${path || 'root'}. ` +
36+
`URL: "${obj.slice(0, 100)}${obj.length > 100 ? '...' : ''}". ` +
37+
`All URLs must be converted to storage keys before database insertion.`,
38+
);
39+
}
40+
} else if (Array.isArray(obj)) {
41+
obj.forEach((item, index) => {
42+
validateNoUrlsInConfig(item, `${path}[${index}]`);
43+
});
44+
} else if (obj && typeof obj === 'object') {
45+
Object.entries(obj).forEach(([key, value]) => {
46+
const currentPath = path ? `${path}.${key}` : key;
47+
validateNoUrlsInConfig(value, currentPath);
48+
});
49+
}
50+
}
51+
2752
const imageProcedure = authedProcedure
2853
.use(keyVaults)
2954
.use(serverDatabase)
@@ -71,8 +96,9 @@ export const imageRouter = router({
7196

7297
log('Starting image creation process, input: %O', input);
7398

74-
// 如果 params 中包含 imageUrls，将它们转换为 S3 keys 用于数据库存储
99+
// 规范化参考图地址，统一存储 S3 key（避免把会过期的预签名 URL 存进数据库）
75100
let configForDatabase = { ...params };
101+
// 1) 处理多图 imageUrls
76102
if (Array.isArray(params.imageUrls) && params.imageUrls.length > 0) {
77103
log('Converting imageUrls to S3 keys for database storage: %O', params.imageUrls);
78104
try {
@@ -82,18 +108,30 @@ export const imageRouter = router({
82108
return key;
83109
});
84110

85-
// 将转换后的 keys 存储为数据库配置
86111
configForDatabase = {
87-
...params,
112+
...configForDatabase,
88113
imageUrls: imageKeys,
89114
};
90115
log('Successfully converted imageUrls to keys for database: %O', imageKeys);
91116
} catch (error) {
92117
log('Error converting imageUrls to keys: %O', error);
93-
// 如果转换失败，保持原始 URLs（可能是本地文件或其他格式）
94118
log('Keeping original imageUrls due to conversion error');
95119
}
96120
}
121+
// 2) 处理单图 imageUrl
122+
if (typeof params.imageUrl === 'string' && params.imageUrl) {
123+
try {
124+
const key = fileService.getKeyFromFullUrl(params.imageUrl);
125+
log('Converted single imageUrl to key: %s -> %s', params.imageUrl, key);
126+
configForDatabase = { ...configForDatabase, imageUrl: key };
127+
} catch (error) {
128+
log('Error converting imageUrl to key: %O', error);
129+
// 转换失败则保留原始值
130+
}
131+
}
132+
133+
// 防御性检测：确保没有完整URL进入数据库
134+
validateNoUrlsInConfig(configForDatabase, 'configForDatabase');
97135

98136
// 步骤 1: 在事务中原子性地创建所有数据库记录
99137
const { batch: createdBatch, generationsWithTasks } = await serverDB.transaction(async (tx) => {

‎src/server/services/file/impls/local.test.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { existsSync, readFileSync } from 'node:fs';
2-
import path from 'node:path';
32
import { beforeEach, describe, expect, it, vi } from 'vitest';
43

54
import { electronIpcClient } from '@/server/modules/ElectronIPCClient';
@@ -234,6 +233,52 @@ describe('DesktopLocalFileImpl', () => {
234233
// 验证
235234
expect(result).toBe('');
236235
});
236+
237+
// Legacy bug compatibility tests - https://github.com/lobehub/lobe-chat/issues/8994
238+
describe('legacy bug compatibility', () => {
239+
it('should handle full URL input by extracting key', async () => {
240+
const fullUrl = 'http://localhost:3000/desktop-file/documents/test.txt';
241+
242+
// Mock getKeyFromFullUrl and getLocalFileUrl
243+
vi.spyOn(service, 'getKeyFromFullUrl').mockReturnValue('desktop://documents/test.txt');
244+
const getLocalFileUrlSpy = vi.spyOn(service as any, 'getLocalFileUrl');
245+
getLocalFileUrlSpy.mockResolvedValueOnce('data:image/png;base64,test');
246+
247+
const result = await service.getFullFileUrl(fullUrl);
248+
249+
expect(service.getKeyFromFullUrl).toHaveBeenCalledWith(fullUrl);
250+
expect(getLocalFileUrlSpy).toHaveBeenCalledWith('desktop://documents/test.txt');
251+
expect(result).toBe('data:image/png;base64,test');
252+
});
253+
254+
it('should handle normal desktop:// key input without extraction', async () => {
255+
const key = 'desktop://documents/test.txt';
256+
257+
const extractSpy = vi.spyOn(service, 'getKeyFromFullUrl');
258+
const getLocalFileUrlSpy = vi.spyOn(service as any, 'getLocalFileUrl');
259+
getLocalFileUrlSpy.mockResolvedValueOnce('data:image/png;base64,test');
260+
261+
const result = await service.getFullFileUrl(key);
262+
263+
expect(extractSpy).not.toHaveBeenCalled();
264+
expect(getLocalFileUrlSpy).toHaveBeenCalledWith(key);
265+
expect(result).toBe('data:image/png;base64,test');
266+
});
267+
268+
it('should handle https:// URLs for legacy compatibility', async () => {
269+
const httpsUrl = 'https://localhost:3000/desktop-file/images/photo.jpg';
270+
271+
vi.spyOn(service, 'getKeyFromFullUrl').mockReturnValue('desktop://images/photo.jpg');
272+
const getLocalFileUrlSpy = vi.spyOn(service as any, 'getLocalFileUrl');
273+
getLocalFileUrlSpy.mockResolvedValueOnce('data:image/jpeg;base64,test');
274+
275+
const result = await service.getFullFileUrl(httpsUrl);
276+
277+
expect(service.getKeyFromFullUrl).toHaveBeenCalledWith(httpsUrl);
278+
expect(getLocalFileUrlSpy).toHaveBeenCalledWith('desktop://images/photo.jpg');
279+
expect(result).toBe('data:image/jpeg;base64,test');
280+
});
281+
});
237282
});
238283

239284
describe('uploadContent', () => {

‎src/server/services/file/impls/local.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { electronIpcClient } from '@/server/modules/ElectronIPCClient';
66
import { inferContentTypeFromImageUrl } from '@/utils/url';
77

88
import { FileServiceImpl } from './type';
9+
import { extractKeyFromUrlOrReturnOriginal } from './utils';
910

1011
/**
1112
* 桌面应用本地文件服务实现
@@ -127,7 +128,11 @@ export class DesktopLocalFileImpl implements FileServiceImpl {
127128
*/
128129
async getFullFileUrl(url?: string | null): Promise<string> {
129130
if (!url) return '';
130-
return this.getLocalFileUrl(url);
131+
132+
// Handle legacy data compatibility using shared utility
133+
const key = extractKeyFromUrlOrReturnOriginal(url, this.getKeyFromFullUrl.bind(this));
134+
135+
return this.getLocalFileUrl(key);
131136
}
132137

133138
/**

‎src/server/services/file/impls/s3.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,56 @@ describe('S3StaticFileImpl', () => {
6565
);
6666
config.S3_ENABLE_PATH_STYLE = false;
6767
});
68+
69+
// Legacy bug compatibility tests - https://github.com/lobehub/lobe-chat/issues/8994
70+
describe('legacy bug compatibility', () => {
71+
it('should handle full URL input by extracting key (S3_SET_ACL=false)', async () => {
72+
config.S3_SET_ACL = false;
73+
const fullUrl = 'https://s3.example.com/bucket/path/to/file.jpg?X-Amz-Signature=expired';
74+
75+
// Mock getKeyFromFullUrl to return the extracted key
76+
vi.spyOn(fileService, 'getKeyFromFullUrl').mockReturnValue('path/to/file.jpg');
77+
78+
const result = await fileService.getFullFileUrl(fullUrl);
79+
80+
expect(fileService.getKeyFromFullUrl).toHaveBeenCalledWith(fullUrl);
81+
expect(result).toBe('https://presigned.example.com/test.jpg');
82+
config.S3_SET_ACL = true;
83+
});
84+
85+
it('should handle full URL input by extracting key (S3_SET_ACL=true)', async () => {
86+
const fullUrl = 'https://s3.example.com/bucket/path/to/file.jpg';
87+
88+
vi.spyOn(fileService, 'getKeyFromFullUrl').mockReturnValue('path/to/file.jpg');
89+
90+
const result = await fileService.getFullFileUrl(fullUrl);
91+
92+
expect(fileService.getKeyFromFullUrl).toHaveBeenCalledWith(fullUrl);
93+
expect(result).toBe('https://example.com/path/to/file.jpg');
94+
});
95+
96+
it('should handle normal key input without extraction', async () => {
97+
const key = 'path/to/file.jpg';
98+
99+
const spy = vi.spyOn(fileService, 'getKeyFromFullUrl');
100+
101+
const result = await fileService.getFullFileUrl(key);
102+
103+
expect(spy).not.toHaveBeenCalled();
104+
expect(result).toBe('https://example.com/path/to/file.jpg');
105+
});
106+
107+
it('should handle http:// URLs for legacy compatibility', async () => {
108+
const httpUrl = 'http://s3.example.com/bucket/path/to/file.jpg';
109+
110+
vi.spyOn(fileService, 'getKeyFromFullUrl').mockReturnValue('path/to/file.jpg');
111+
112+
const result = await fileService.getFullFileUrl(httpUrl);
113+
114+
expect(fileService.getKeyFromFullUrl).toHaveBeenCalledWith(httpUrl);
115+
expect(result).toBe('https://example.com/path/to/file.jpg');
116+
});
117+
});
68118
});
69119

70120
describe('getFileContent', () => {

0 commit comments

Comments
 (0)