Fixed missing image dimensions in meta tags with non-local storage

ghost/core/core/server/adapters/storage/S3Storage.ts

@@ -7,6 +7,7 @@ import errors from '@tryghost/errors';
 import logging from '@tryghost/logging';
 import {
     DeleteObjectCommand,
+    GetObjectCommand,
     HeadObjectCommand,
     NotFound,
     NoSuchKey,
@@ -33,7 +34,8 @@ const messages = {
     emptyTargetPath: 'S3Storage.saveRaw requires a non-empty targetPath',
     emptyFileName: 'S3Storage.{method} requires a non-empty fileName',
     emptyRelativePath: 'S3Storage.buildKey requires a non-empty relativePath',
-    readNotSupported: 'read() is not supported by S3Storage. S3Storage is designed for media and files, not images. Use LocalImagesStorage for image storage.',
+    emptyReadPath: 'S3Storage.read requires a non-empty path',
+    readNotFound: 'Could not read file: {path}',
     multipartUploadInitFailed: 'Failed to initiate file upload.',
     multipartUploadPartFailed: 'Failed to upload file part {partNumber}.',
     multipartUploadReadFailed: 'There was an error uploading the file. The file may have been modified or removed during upload.',
@@ -380,12 +382,38 @@ export default class S3Storage extends StorageBase {
     }
 
     /**
-     * Not supported - S3Storage is for media/files only. Images use LocalImagesStorage.
+     * Reads an object's bytes from S3. Used by image dimension lookups, which
+     * fall back to reading from storage for images served via the CDN.
      */
-    async read(): Promise<Buffer> {
-        throw new errors.IncorrectUsageError({
-            message: tpl(messages.readNotSupported)
-        });
+    async read(options: {path?: string} = {}): Promise<Buffer> {
+        const relativePath = options.path;
+
+        if (!relativePath?.trim()) {
+            throw new errors.IncorrectUsageError({
+                message: tpl(messages.emptyReadPath)
+            });
+        }
+
+        const key = this.buildKey(relativePath);
+
+        try {
+            const response = await this.client.send(new GetObjectCommand({
+                Bucket: this.bucket,
+                Key: key
+            }));
+
+            const bytes = await response.Body?.transformToByteArray();
+            return Buffer.from(bytes ?? []);
+        } catch (error) {
+            if (this.isNotFound(error)) {
+                throw new errors.NotFoundError({
+                    err: error,
+                    message: tpl(messages.readNotFound, {path: relativePath})
+                });
+            }
+
+            throw error;
+        }
     }
 
     private buildKey(relativePath: string): string {
@@ -446,7 +474,7 @@ export default class S3Storage extends StorageBase {
         return input.replace(/^\/+/, '');
     }
 
-    private isNotFound(error: unknown): boolean {
+    private isNotFound(error: unknown): error is NotFound | NoSuchKey {
         return error instanceof NotFound || error instanceof NoSuchKey;
     }
 }

ghost/core/test/integration/adapters/storage/s3-storage.test.ts

@@ -0,0 +1,112 @@
+/* eslint-disable ghost/mocha/no-top-level-hooks -- false positive: the hooks are inside the describe, but the lint plugin can't see through the describe.skipIf()() gate below. (PLA-170) */
+import {describe, it, beforeAll, afterEach, afterAll} from 'vitest';
+import assert from 'node:assert/strict';
+
+import S3Storage from '../../../../core/server/adapters/storage/S3Storage';
+import {
+    createTestS3Client,
+    createTestBucket,
+    emptyTestBucket,
+    deleteTestBucket,
+    getMinioConfig,
+    putObject
+} from '../../../utils/minio';
+
+const STATIC_PREFIX = 'content/images';
+const minioConfig = getMinioConfig();
+
+// read() builds the object key as [tenantPrefix/]storagePath/relativePath, so a
+// test fixture must be written at that exact key for read() to find it.
+const objectKey = (relativePath: string, tenantPrefix = '') => [tenantPrefix, STATIC_PREFIX, relativePath].filter(Boolean).join('/');
+
+// Skip when MinIO is unreachable. The flag is set by the integration
+// globalSetup (vitest-globalsetup-services.ts), which probes MinIO once before
+// the forks spawn. (PLA-170)
+describe.skipIf(process.env.GHOST_TEST_MINIO_AVAILABLE !== '1')('Integration: S3Storage.read', function () {
+    let adminClient: ReturnType<typeof createTestS3Client>;
+    let bucket: string;
+
+    const createStorage = (overrides = {}) => new S3Storage({
+        ...minioConfig,
+        bucket,
+        cdnUrl: `${minioConfig.endpoint}/${bucket}`,
+        staticFileURLPrefix: STATIC_PREFIX,
+        multipartUploadThresholdBytes: 10 * 1024 * 1024,
+        multipartChunkSizeBytes: 10 * 1024 * 1024,
+        ...overrides
+    });
+
+    beforeAll(async function () {
+        adminClient = createTestS3Client();
+        bucket = await createTestBucket(adminClient, 'test-storage');
+    });
+
+    afterEach(async function () {
+        await emptyTestBucket(adminClient, bucket);
+    });
+
+    afterAll(async function () {
+        await deleteTestBucket(adminClient, bucket);
+    });
+
+    it('reads back the raw bytes of an object written to storage', async function () {
+        // Include NUL and high bytes to prove read() is binary-safe (images).
+        const body = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0xff, 0x0a, 0x01]);
+        await putObject(adminClient, bucket, objectKey('2024/06/image.jpg'), body);
+
+        const result = await createStorage().read({path: '2024/06/image.jpg'});
+
+        assert.deepEqual(result, body);
+    });
+
+    it('round-trips a file referenced by its absolute storage path', async function () {
+        // image dimension lookups pass the local-style /content/images/ path;
+        // read() must map that back to the same key save() would have written.
+        const body = Buffer.from('saved-via-adapter');
+        await putObject(adminClient, bucket, objectKey('2024/06/saved.png'), body);
+
+        const result = await createStorage().read({path: '/content/images/2024/06/saved.png'});
+
+        assert.deepEqual(result, body);
+    });
+
+    it('throws NotFoundError when the object is missing', async function () {
+        await assert.rejects(
+            () => createStorage().read({path: '2024/06/missing.jpg'}),
+            (err: Error) => {
+                assert.equal((err as {errorType?: string}).errorType, 'NotFoundError');
+                assert.match(err.message, /Could not read file: 2024\/06\/missing\.jpg/);
+                return true;
+            }
+        );
+    });
+
+    it('throws IncorrectUsageError when no path is given', async function () {
+        await assert.rejects(
+            () => createStorage().read(),
+            {errorType: 'IncorrectUsageError', message: /requires a non-empty path/}
+        );
+    });
+
+    describe('tenantPrefix scoping', function () {
+        const TENANT = 'tenant-abc';
+
+        it('reads an object stored under the tenant prefix', async function () {
+            const body = Buffer.from('tenant-scoped-bytes');
+            await putObject(adminClient, bucket, objectKey('2024/06/image.jpg', TENANT), body);
+
+            const result = await createStorage({tenantPrefix: TENANT}).read({path: '2024/06/image.jpg'});
+
+            assert.deepEqual(result, body);
+        });
+
+        it('does not read another tenant\'s object', async function () {
+            await putObject(adminClient, bucket, objectKey('2024/06/image.jpg', 'tenant-other'), Buffer.from('other'));
+
+            await assert.rejects(
+                () => createStorage({tenantPrefix: TENANT}).read({path: '2024/06/image.jpg'}),
+                {errorType: 'NotFoundError'}
+            );
+        });
+    });
+});

ghost/core/test/unit/server/adapters/storage/s3-storage.test.ts

@@ -485,12 +485,42 @@ describe('S3Storage', function () {
         }, /not a valid URL/);
     });
 
-    it('read() throws as it is not supported', async function () {
+    it('read() returns the object bytes for a relative path', async function () {
+        const {storage, sendStub} = createStorage();
+        const {GetObjectCommand: GetObjectCmd} = await import('@aws-sdk/client-s3');
+
+        sendStub.resolves({
+            Body: {
+                transformToByteArray: async () => new Uint8Array(Buffer.from('image-bytes'))
+            }
+        });
+
+        const result = await storage.read({path: '2024/06/image.jpg'});
+
+        assert.deepEqual(result, Buffer.from('image-bytes'));
+
+        const command = sendStub.firstCall.args[0] as InstanceType<typeof GetObjectCmd>;
+        assert.equal(command.input.Bucket, 'test-bucket');
+        assert.equal(command.input.Key, 'configurable/prefix/content/files/2024/06/image.jpg');
+    });
+
+    it('read() throws NotFoundError when the object is missing', async function () {
+        const {storage, sendStub} = createStorage();
+
+        sendStub.rejects(createNotFoundError());
+
+        await assert.rejects(
+            storage.read({path: '2024/06/missing.jpg'}),
+            /Could not read file: 2024\/06\/missing.jpg/
+        );
+    });
+
+    it('read() throws when no path is given', async function () {
         const {storage} = createStorage();
 
         await assert.rejects(
             storage.read(),
-            /read\(\) is not supported by S3Storage/
+            /requires a non-empty path/
         );
     });