Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion api/server/services/Files/Azure/images.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,20 @@ async function uploadImageToAzure({
buffer: resizedBuffer,
width,
height,
type,
} = await resizeImageBuffer(inputBuffer, resolution, endpoint);
const extension = path.extname(inputFilePath);
const userId = req.user.id;
let webPBuffer;
let fileName = `${file_id}__${path.basename(inputFilePath)}`;
const targetExtension = `.${appConfig.imageOutputType}`;

if (extension.toLowerCase() === targetExtension) {
/* The name is only a claim about the contents; the bytes have to agree before conversion is
* skipped, or the record is typed from a name that misdescribes what was stored. */
if (
extension.toLowerCase() === targetExtension &&
type === `image/${appConfig.imageOutputType}`
) {
webPBuffer = resizedBuffer;
} else {
webPBuffer = await sharp(resizedBuffer).toFormat(appConfig.imageOutputType).toBuffer();
Expand Down
8 changes: 7 additions & 1 deletion api/server/services/Files/Firebase/images.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,20 @@ async function uploadImageToFirebase({ req, file, file_id, endpoint, resolution
buffer: resizedBuffer,
width,
height,
type,
} = await resizeImageBuffer(inputBuffer, resolution, endpoint);
const extension = path.extname(inputFilePath);
const userId = req.user.id;

let webPBuffer;
let fileName = `${file_id}__${path.basename(inputFilePath)}`;
const targetExtension = `.${appConfig.imageOutputType}`;
if (extension.toLowerCase() === targetExtension) {
/* The name is only a claim about the contents; the bytes have to agree before conversion is
* skipped, or the record is typed from a name that misdescribes what was stored. */
if (
extension.toLowerCase() === targetExtension &&
type === `image/${appConfig.imageOutputType}`
) {
webPBuffer = resizedBuffer;
} else {
webPBuffer = await sharp(resizedBuffer).toFormat(appConfig.imageOutputType).toBuffer();
Expand Down
87 changes: 85 additions & 2 deletions api/server/services/Files/Local/__tests__/images.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
jest.mock('sharp', () => ({}));
jest.mock('sharp', () => {
const toBuffer = jest.fn(async () => Buffer.from('converted-bytes'));
const sharpMock = jest.fn(() => ({ toFormat: jest.fn().mockReturnThis(), toBuffer }));
return sharpMock;
});
jest.mock('@librechat/api', () => ({
stripCacheBust: jest.fn((filepath) => filepath.split('?')[0]),
}));
Expand All @@ -11,8 +15,10 @@ jest.mock('~/models', () => ({
const fs = require('fs');
const os = require('os');
const path = require('path');
const sharp = require('sharp');
const { updateFile } = require('~/models');
const { prepareImagesLocal } = require('../images');
const { resizeImageBuffer } = require('../../images/resize');
const { prepareImagesLocal, uploadLocalImage } = require('../images');

describe('prepareImagesLocal', () => {
let tmpDir;
Expand Down Expand Up @@ -62,3 +68,80 @@ describe('prepareImagesLocal', () => {
expect(encoded).toBe(Buffer.from('plain-png-bytes').toString('base64'));
});
});

describe('uploadLocalImage', () => {
let tmpDir;
let imageOutput;

beforeEach(() => {
jest.clearAllMocks();
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'upload-local-image-'));
imageOutput = path.join(tmpDir, 'images');
});

afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
});

const makeReq = (imageOutputType) => ({
user: { id: 'user-1' },
config: { paths: { imageOutput }, imageOutputType },
});

/** The caller records the upload as `image/${imageOutputType}`, so whatever this writes has to
* be in that format β€” the type is handed to providers verbatim as `media_type`. */
const upload = async (name, resized) => {
const inputFilePath = path.join(tmpDir, name);
fs.writeFileSync(inputFilePath, Buffer.from('uploaded-bytes'));
resizeImageBuffer.mockResolvedValue({ width: 8, height: 8, ...resized });

const { filepath } = await uploadLocalImage({
req: makeReq('png'),
file: { path: inputFilePath },
file_id: 'file-1',
endpoint: 'anthropic',
});

return fs.readFileSync(path.join(imageOutput, 'user-1', path.basename(filepath)));
};

it('converts a JPEG that was given a .png name', async () => {
const written = await upload('fake.png', {
buffer: Buffer.from('resized-jpeg-bytes'),
type: 'image/jpeg',
});

expect(sharp).toHaveBeenCalledWith(Buffer.from('resized-jpeg-bytes'));
expect(written).toEqual(Buffer.from('converted-bytes'));
});

it('writes the resized bytes untouched when the name and the bytes both match the target', async () => {
const written = await upload('real.png', {
buffer: Buffer.from('resized-png-bytes'),
type: 'image/png',
});

expect(sharp).not.toHaveBeenCalled();
expect(written).toEqual(Buffer.from('resized-png-bytes'));
});

it('converts when sharp named no format for the resized bytes', async () => {
const written = await upload('unknown.png', {
buffer: Buffer.from('resized-unknown-bytes'),
type: undefined,
});

expect(sharp).toHaveBeenCalled();
expect(written).toEqual(Buffer.from('converted-bytes'));
});

it('still converts a name that does not carry the target extension', async () => {
const written = await upload('photo.jpg', {
buffer: Buffer.from('resized-jpeg-bytes'),
type: 'image/jpeg',
});

expect(sharp).toHaveBeenCalled();
expect(written).toEqual(Buffer.from('converted-bytes'));
});
});
10 changes: 9 additions & 1 deletion api/server/services/Files/Local/images.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ async function uploadLocalImage({ req, file, file_id, endpoint, resolution = 'hi
buffer: resizedBuffer,
width,
height,
type,
} = await resizeImageBuffer(inputBuffer, resolution, endpoint);
const extension = path.extname(inputFilePath);

Expand All @@ -50,7 +51,14 @@ async function uploadLocalImage({ req, file, file_id, endpoint, resolution = 'hi
const newPath = path.join(userPath, fileName);
const targetExtension = `.${appConfig.imageOutputType}`;

if (extension.toLowerCase() === targetExtension) {
/* The name is only a claim about the contents, so conversion is skipped only when the bytes
* agree with it. A JPEG named `.png` otherwise reaches `createFile` typed `image/png`, and that
* type is handed to providers verbatim; Anthropic then refuses the request, and keeps refusing
* it, because the image stays in the conversation's history. */
if (
extension.toLowerCase() === targetExtension &&
type === `image/${appConfig.imageOutputType}`
Comment on lines +58 to +60

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Move the format decision out of legacy CJS

Every upload using the local path shown hereβ€”and the matching Firebase and Azure editsβ€”now executes this new byte/type branch in legacy CJS, while S3 and CloudFront execute a separate copy in packages/api/src/storage/images.ts; convertImage adds another CJS copy. This splits one storage-format invariant across five implementations, so the next format fix can leave some file strategies writing bytes that disagree with their recorded type; move the conversion/skip decision into a shared TypeScript service/helper and leave these CJS modules as delegators.

AGENTS.md reference: AGENTS.md:L77-L81

Useful? React with πŸ‘Β / πŸ‘Ž.

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.

Confirmed against AGENTS.md:77-81 at this head. The added type checks in Local, Firebase, Azure, and convertImage are behavior, while ImageService maintains another copy. A shared TypeScript conversion/skip helper called by those strategies would satisfy the requirement without migrating unrelated storage code. This remains open pending that change.

) {
const bytes = Buffer.byteLength(resizedBuffer);
await fs.promises.writeFile(newPath, resizedBuffer);
const filepath = path.posix.join('/', 'images', req.user.id, path.basename(newPath));
Expand Down
6 changes: 4 additions & 2 deletions api/server/services/Files/images/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ async function convertImage(req, file, resolution = 'high', basename = '') {
buffer: resizedBuffer,
width,
height,
type,
} = await resizeImageBuffer(inputBuffer, resolution);

// Check if the file is already in target format; if it isn't, convert it:
// Check if the file is already in target format; if it isn't, convert it. The name is only a
// claim about the contents, so the bytes have to agree with it before conversion is skipped.
const targetExtension = `.${appConfig.imageOutputType}`;
if (extension === targetExtension) {
if (extension === targetExtension && type === `image/${appConfig.imageOutputType}`) {
outputBuffer = resizedBuffer;
} else {
outputBuffer = await sharp(resizedBuffer).toFormat(appConfig.imageOutputType).toBuffer();
Expand Down
91 changes: 91 additions & 0 deletions packages/api/src/files/encode/image.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import sharp from 'sharp';
import { Readable } from 'node:stream';
import { FileSources, VisionModes } from 'librechat-data-provider';
import type { ServerRequest } from '~/types';
Expand Down Expand Up @@ -202,6 +203,96 @@ describe('encodeAndFormatImages', () => {
expect(result.image_urls).toEqual([expected]);
});

describe('a record whose type does not describe its bytes', () => {
/** A JPEG stored under a `.png` name: the upload was typed from the name, so the record reads
* `image/png`. Sending that as `media_type` is what Anthropic rejects, on this turn and on
* every later one, because the image stays in the conversation's history. */
let jpegBytes: Buffer;
let jpegContent: string;
const mislabeled = () => ({ ...file, bytes: jpegBytes.length, type: 'image/png' });

beforeAll(async () => {
jpegBytes = await sharp({
create: { width: 8, height: 8, channels: 3, background: { r: 1, g: 2, b: 3 } },
})
.jpeg()
.toBuffer();
jpegContent = jpegBytes.toString('base64');
});

beforeEach(() => {
getDownloadStream.mockImplementation(async () => Readable.from(jpegBytes));
});

it('declares the bytes to Anthropic, not the stored type', async () => {
const result = await encodeAndFormatImages(
makeReq(),
[mislabeled()],
{ endpoint: 'anthropic' },
deps,
);

expect(result.image_urls).toEqual([
{ type: 'image', source: { type: 'base64', media_type: 'image/jpeg', data: jpegContent } },
]);
});

it('declares the bytes to Google', async () => {
const result = await encodeAndFormatImages(
makeReq(),
[mislabeled()],
{ provider: 'google' },
deps,
VisionModes.generative,
);

expect(result.image_urls).toEqual([
{ type: 'image_url', inlineData: { mimeType: 'image/jpeg', data: jpegContent } },
]);
});

it('declares the bytes in the data URL', async () => {
const result = await encodeAndFormatImages(makeReq(), [mislabeled()], {}, deps);

expect(result.image_urls).toEqual([
{
type: 'image_url',
image_url: { url: `data:image/jpeg;base64,${jpegContent}`, detail: 'auto' },
},
]);
});

it('keeps the stored type when the bytes name no format of their own', async () => {
getDownloadStream.mockImplementation(async () => Readable.from(imageBytes));

const result = await encodeAndFormatImages(
makeReq(),
[{ ...file, type: 'image/png' }],
{ endpoint: 'anthropic' },
deps,
);

expect(result.image_urls).toEqual([
{ type: 'image', source: { type: 'base64', media_type: 'image/png', data: content } },
]);
});

it('leaves an external URL alone, having no bytes to read', async () => {
prepareImagePayload.mockImplementation(async (_req, image) => [image, file.filepath]);

const result = await encodeAndFormatImages(
makeReq(),
[{ ...mislabeled(), source: FileSources.openai }],
{ endpoint: 'openai' },
deps,
);

expect(result.image_urls).toEqual([
{ type: 'image_url', image_url: { url: file.filepath, detail: 'auto' } },
]);
});
});

it.each([
[undefined, undefined, 'auto'],
['low', undefined, 'low'],
Expand Down
35 changes: 23 additions & 12 deletions packages/api/src/files/encode/image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { AxiosInstance } from 'axios';
import type { ServerRequest, StrategyFunctions } from '~/types';
import { getFileStream, isAttachmentObjectNotFoundError } from './utils';
import { validateImage } from '~/files/validation';
import { sniffImageMimeType } from '~/files/mime';
import { runGuardedEncode } from './memoryGuard';
import { logAxiosError } from '~/utils/axios';

Expand Down Expand Up @@ -185,33 +186,43 @@ export async function encodeAndFormatImages(
}

const isURL = imageContent.startsWith('http');
if (file.height && file.width && !isURL) {
/** The stored type is a claim about bytes it need not describe: an upload is typed from the
* output format it was meant to be converted into, and a file whose name already carries that
* extension skips conversion whatever is inside it. The claim is handed to providers verbatim
* below, and Anthropic refuses a request whose `media_type` disagrees with the bytes β€” every
* later turn of the conversation included, since the image stays in the history. The bytes are
* decoded here anyway, so they are what the request should be built from. */
let mediaType = file.type;
if (!isURL) {
const imageBuffer = Buffer.from(imageContent, 'base64');
const validation = await validateImage(
imageBuffer,
imageBuffer.length,
effectiveEndpoint ?? '',
configuredFileSizeLimit,
);
if (!validation.isValid) {
throw new Error(`Image validation failed for ${file.filename}: ${validation.error}`);
mediaType = sniffImageMimeType(imageBuffer) ?? file.type;
if (file.height && file.width) {
const validation = await validateImage(
imageBuffer,
imageBuffer.length,
effectiveEndpoint ?? '',
configuredFileSizeLimit,
);
if (!validation.isValid) {
throw new Error(`Image validation failed for ${file.filename}: ${validation.error}`);
}
}
}

const url = isURL ? imageContent : `data:${file.type};base64,${imageContent}`;
const url = isURL ? imageContent : `data:${mediaType};base64,${imageContent}`;
if (mode === VisionModes.agents) {
result.image_urls.push({ type: ContentTypes.IMAGE_URL, image_url: { url, detail } });
} else if (effectiveEndpoint === EModelEndpoint.google && mode === VisionModes.generative) {
result.image_urls.push({
type: ContentTypes.IMAGE_URL,
inlineData: { mimeType: file.type, data: imageContent },
inlineData: { mimeType: mediaType, data: imageContent },
});
} else if (effectiveEndpoint === EModelEndpoint.google) {
result.image_urls.push({ type: ContentTypes.IMAGE_URL, image_url: url });
} else if (effectiveEndpoint === EModelEndpoint.anthropic) {
result.image_urls.push({
type: 'image',
source: { type: 'base64', media_type: file.type, data: imageContent },
source: { type: 'base64', media_type: mediaType, data: imageContent },
});
} else {
result.image_urls.push({ type: ContentTypes.IMAGE_URL, image_url: { url, detail } });
Expand Down
Loading
Loading