Fix broken ICO + restore animation for WebP and APNG sources

source/container/src/services/image-processing/image-processor.service.test.ts

@@ -10,16 +10,28 @@ import sharp from 'sharp';
 
 let TEST_JPEG_BUFFER: Buffer;
 let TEST_GIF_BUFFER: Buffer;
+let TEST_WEBP_BUFFER: Buffer;
+let TEST_PNG_BUFFER: Buffer;
 
 beforeAll(async () => {
   // Generate valid test images using Sharp
   TEST_JPEG_BUFFER = await sharp({
     create: { width: 100, height: 100, channels: 3, background: { r: 255, g: 0, b: 0 } }
   }).jpeg().toBuffer();
-  
+
   TEST_GIF_BUFFER = await sharp({
     create: { width: 100, height: 100, channels: 3, background: { r: 0, g: 0, b: 255 } }
   }).gif().toBuffer();
+
+  // Single-frame WebP/PNG — used to exercise the "animation-capable source type
+  // but pages<=1" reset path under the broadened isExpectedToBeAnimated check.
+  TEST_WEBP_BUFFER = await sharp({
+    create: { width: 100, height: 100, channels: 3, background: { r: 0, g: 255, b: 0 } }
+  }).webp().toBuffer();
+
+  TEST_PNG_BUFFER = await sharp({
+    create: { width: 100, height: 100, channels: 3, background: { r: 128, g: 128, b: 128 } }
+  }).png().toBuffer();
 });
 
 describe('ImageProcessorService', () => {
@@ -302,6 +314,282 @@ describe('ImageProcessorService', () => {
     });
   });
 
+  describe('ICO passthrough', () => {
+    // Minimal valid ICONDIR: reserved(2)=0, type(2)=1, count(2)=0 — plus a couple
+    // trailing bytes to be over the 4-byte minimum. The processor must not call
+    // Sharp on this, so the buffer never needs to be a real ICO.
+    const ICO_HEADER = Buffer.from([0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xAB, 0xCD]);
+
+    it('should pass ICO buffer through unchanged when transformations are present', async () => {
+      jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({
+        buffer: ICO_HEADER,
+        metadata: { size: ICO_HEADER.length, format: 'x-icon' }
+      });
+
+      const request: ImageProcessingRequest = {
+        requestId: 'test-ico-passthrough',
+        timestamp: Date.now(),
+        origin: { url: 'https://example.com/favicon.ico' },
+        sourceImageContentType: 'image/x-icon',
+        // Policy might have asked for resize/format conversion — must be ignored for ICO.
+        transformations: [{ type: 'resize', value: { width: 50 }, source: 'url' }],
+        response: { headers: {} }
+      };
+
+      const result = await service.process(request);
+
+      expect(result).toBe(ICO_HEADER);
+      expect(request.response.contentType).toBe('image/x-icon');
+      expect(request.timings.imageProcessing.transformationApplicationMs).toBe(0);
+    });
+
+    it('should pass ICO buffer through unchanged for image/vnd.microsoft.icon', async () => {
+      jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({
+        buffer: ICO_HEADER,
+        metadata: { size: ICO_HEADER.length, format: 'vnd.microsoft.icon' }
+      });
+
+      const request: ImageProcessingRequest = {
+        requestId: 'test-ico-vnd-passthrough',
+        timestamp: Date.now(),
+        origin: { url: 'https://example.com/favicon.ico' },
+        sourceImageContentType: 'image/vnd.microsoft.icon',
+        transformations: [{ type: 'resize', value: { width: 50 }, source: 'url' }],
+        response: { headers: {} }
+      };
+
+      const result = await service.process(request);
+
+      expect(result).toBe(ICO_HEADER);
+      expect(request.response.contentType).toBe('image/vnd.microsoft.icon');
+    });
+
+    it('should still process non-ICO sources normally', async () => {
+      // Regression guard: the ICO short-circuit must only fire for ICO content types.
+      jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({
+        buffer: TEST_JPEG_BUFFER,
+        metadata: { size: TEST_JPEG_BUFFER.length, format: 'jpeg' }
+      });
+
+      const request: ImageProcessingRequest = {
+        requestId: 'test-not-ico',
+        timestamp: Date.now(),
+        origin: { url: 'https://example.com/image.jpg' },
+        sourceImageContentType: 'image/jpeg',
+        transformations: [{ type: 'resize', value: { width: 50 }, source: 'url' }],
+        response: { headers: {} }
+      };
+
+      const result = await service.process(request);
+
+      // Must be a transformed Sharp output, not the original buffer.
+      expect(result).not.toBe(TEST_JPEG_BUFFER);
+      expect(request.response.contentType).toMatch(/^image\//);
+    });
+  });
+
+  describe('APNG passthrough', () => {
+    // Minimal-but-valid leading PNG signature + IHDR + acTL chunk. The pipeline
+    // only sniffs the prefix — the chunks past acTL don't have to decode.
+    // PNG signature: 89 50 4E 47 0D 0A 1A 0A
+    // IHDR chunk: length(4)=0x0D type(4)='IHDR' data(13) crc(4) — 25 bytes
+    // acTL chunk: length(4)=0x08 type(4)='acTL' data(8) crc(4) — 20 bytes
+    const APNG_HEADER = Buffer.concat([
+      Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), // PNG sig
+      Buffer.from([0x00, 0x00, 0x00, 0x0d]), // IHDR length
+      Buffer.from('IHDR', 'ascii'),
+      Buffer.alloc(13 + 4), // IHDR data + crc (dummy)
+      Buffer.from([0x00, 0x00, 0x00, 0x08]), // acTL length
+      Buffer.from('acTL', 'ascii'),
+      Buffer.alloc(8 + 4), // acTL data + crc (dummy)
+    ]);
+    const STATIC_PNG_HEADER = Buffer.concat([
+      Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
+      Buffer.from([0x00, 0x00, 0x00, 0x0d]),
+      Buffer.from('IHDR', 'ascii'),
+      Buffer.alloc(13 + 4),
+      // No acTL chunk — straight to IDAT in real files
+    ]);
+
+    // PNG with a tEXt chunk that uses 'acTL' as its keyword content. A naive
+    // substring scan would false-positive here; the chunk parser must not.
+    // tEXt chunk: length(4) + 'tEXt' + data(length) + crc(4)
+    const PNG_WITH_TEXT_ACTL_KEYWORD = Buffer.concat([
+      Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
+      Buffer.from([0x00, 0x00, 0x00, 0x0d]),
+      Buffer.from('IHDR', 'ascii'),
+      Buffer.alloc(13 + 4),
+      // tEXt chunk carrying the literal bytes 'acTL' as data
+      Buffer.from([0x00, 0x00, 0x00, 0x04]), // tEXt length = 4 bytes
+      Buffer.from('tEXt', 'ascii'),
+      Buffer.from('acTL', 'ascii'), // 4 bytes of data — the false-positive trap
+      Buffer.alloc(4), // crc
+    ]);
+
+    // IHDR -> IDAT -> trailing 'acTL' bytes. APNG spec requires acTL to appear
+    // BEFORE the first IDAT — the parser must stop scanning at IDAT.
+    const PNG_WITH_ACTL_AFTER_IDAT = Buffer.concat([
+      Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
+      Buffer.from([0x00, 0x00, 0x00, 0x0d]),
+      Buffer.from('IHDR', 'ascii'),
+      Buffer.alloc(13 + 4),
+      Buffer.from([0x00, 0x00, 0x00, 0x08]),
+      Buffer.from('IDAT', 'ascii'),
+      Buffer.alloc(8 + 4),
+      Buffer.from([0x00, 0x00, 0x00, 0x08]),
+      Buffer.from('acTL', 'ascii'),
+      Buffer.alloc(8 + 4),
+    ]);
+
+    it('should detect acTL chunk in APNG buffer', () => {
+      expect(ImageProcessorService['isAnimatedPng'](APNG_HEADER)).toBe(true);
+    });
+
+    it('should return false for a static PNG buffer (no acTL chunk)', () => {
+      expect(ImageProcessorService['isAnimatedPng'](STATIC_PNG_HEADER)).toBe(false);
+    });
+
+    it('should not false-positive when acTL appears as tEXt chunk data', () => {
+      // A naive substring scan over the buffer would find "acTL" and incorrectly
+      // mark the file as animated.
+      expect(ImageProcessorService['isAnimatedPng'](PNG_WITH_TEXT_ACTL_KEYWORD)).toBe(false);
+    });
+
+    it('should stop scanning at IDAT (acTL after IDAT is spec-invalid)', () => {
+      expect(ImageProcessorService['isAnimatedPng'](PNG_WITH_ACTL_AFTER_IDAT)).toBe(false);
+    });
+
+    it('should handle truncated buffers without infinite looping', () => {
+      // Length field claims more bytes than exist — must terminate, not loop or throw.
+      const truncated = Buffer.concat([
+        Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
+        Buffer.from([0xff, 0xff, 0xff, 0xff]), // length claims 4GB
+        Buffer.from('IHDR', 'ascii'),
+        Buffer.alloc(2), // truncated data
+      ]);
+      expect(ImageProcessorService['isAnimatedPng'](truncated)).toBe(false);
+    });
+
+    it('should pass APNG buffer through unchanged when transformations are present', async () => {
+      jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({
+        buffer: APNG_HEADER,
+        metadata: { size: APNG_HEADER.length, format: 'png' }
+      });
+
+      const request: ImageProcessingRequest = {
+        requestId: 'test-apng-passthrough',
+        timestamp: Date.now(),
+        origin: { url: 'https://example.com/animated.png' },
+        sourceImageContentType: 'image/png',
+        // Even with a resize requested, we serve the source verbatim — Sharp
+        // would strip animation on output otherwise.
+        transformations: [{ type: 'resize', value: { width: 50 }, source: 'url' }],
+        response: { headers: {} }
+      };
+
+      const result = await service.process(request);
+
+      expect(result).toBe(APNG_HEADER);
+      expect(request.response.contentType).toBe('image/png');
+      expect(request.timings.imageProcessing.transformationApplicationMs).toBe(0);
+    });
+
+    it('should still process static PNG through Sharp', async () => {
+      // Regression guard: only APNG short-circuits; static PNG must reach the
+      // Sharp pipeline.
+      jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({
+        buffer: TEST_PNG_BUFFER,
+        metadata: { size: TEST_PNG_BUFFER.length, format: 'png' }
+      });
+
+      const request: ImageProcessingRequest = {
+        requestId: 'test-static-png-not-passthrough',
+        timestamp: Date.now(),
+        origin: { url: 'https://example.com/static.png' },
+        sourceImageContentType: 'image/png',
+        transformations: [{ type: 'resize', value: { width: 50 }, source: 'url' }],
+        response: { headers: {} }
+      };
+
+      const result = await service.process(request);
+
+      // Must be the Sharp output, not the original buffer.
+      expect(result).not.toBe(TEST_PNG_BUFFER);
+      expect(request.response.contentType).toBe('image/png');
+    });
+  });
+
+  describe('animation-capable source detection', () => {
+    // isAnimationCapableSource is the gate that decides whether Sharp is
+    // instantiated with animated:true. Without animated:true, the ANIM (WebP)
+    // and acTL (APNG) chunks are ignored and animation is silently lost.
+    it('should classify GIF as animation-capable', () => {
+      expect(ImageProcessorService['isAnimationCapableSource']('image/gif')).toBe(true);
+    });
+
+    it('should classify WebP as animation-capable (covers animated WebP)', () => {
+      expect(ImageProcessorService['isAnimationCapableSource']('image/webp')).toBe(true);
+    });
+
+    it('should classify PNG as animation-capable (covers APNG)', () => {
+      expect(ImageProcessorService['isAnimationCapableSource']('image/png')).toBe(true);
+    });
+
+    it('should classify JPEG as not animation-capable', () => {
+      expect(ImageProcessorService['isAnimationCapableSource']('image/jpeg')).toBe(false);
+    });
+
+    it('should be case-insensitive', () => {
+      expect(ImageProcessorService['isAnimationCapableSource']('IMAGE/WEBP')).toBe(true);
+    });
+
+    it('should handle undefined content type', () => {
+      expect(ImageProcessorService['isAnimationCapableSource'](undefined)).toBe(false);
+    });
+
+    it('should process a single-frame WebP source (animated reset path)', async () => {
+      // Regression guard: with isExpectedToBeAnimated=true for WebP, a static
+      // WebP must still reach the reset path (pages<=1) and produce output.
+      jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({
+        buffer: TEST_WEBP_BUFFER,
+        metadata: { size: TEST_WEBP_BUFFER.length, format: 'webp' }
+      });
+
+      const request: ImageProcessingRequest = {
+        requestId: 'test-static-webp',
+        timestamp: Date.now(),
+        origin: { url: 'https://example.com/image.webp' },
+        sourceImageContentType: 'image/webp',
+        transformations: [{ type: 'resize', value: { width: 50 }, source: 'url' }],
+        response: { headers: {} }
+      };
+
+      const result = await service.process(request);
+      expect(result).toBeInstanceOf(Buffer);
+      expect(request.response.contentType).toMatch(/^image\//);
+    });
+
+    it('should process a single-frame PNG source (animated reset path)', async () => {
+      jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({
+        buffer: TEST_PNG_BUFFER,
+        metadata: { size: TEST_PNG_BUFFER.length, format: 'png' }
+      });
+
+      const request: ImageProcessingRequest = {
+        requestId: 'test-static-png',
+        timestamp: Date.now(),
+        origin: { url: 'https://example.com/image.png' },
+        sourceImageContentType: 'image/png',
+        transformations: [{ type: 'resize', value: { width: 50 }, source: 'url' }],
+        response: { headers: {} }
+      };
+
+      const result = await service.process(request);
+      expect(result).toBeInstanceOf(Buffer);
+      expect(request.response.contentType).toMatch(/^image\//);
+    });
+  });
+
   describe('animated GIF handling', () => {
     it('should re-instantiate with animated=false for single-frame GIF', async () => {
       jest.spyOn(service['originFetcher'], 'fetchImage').mockResolvedValue({