fix(share): answer place-photo cache miss with 204 like the maps proxy

server/src/nest/share/share.controller.ts

@@ -89,14 +89,17 @@ export class SharedController {
    * image URLs to this route so thumbnails load without a session cookie (the
    * /api/maps bytes endpoint is JwtAuthGuard'd). The service validates the token
    * and that the place belongs to its trip; a miss streams nothing and answers
-   * 404. Declared before the bare ':token' read route. Streaming mirrors
-   * MapsController.placePhotoBytes (cached photos are always JPEG).
+   * an empty 204, mirroring MapsController.placePhotoBytes (#1727's rationale
+   * extended to shared pages: shared payloads keep this URL in place
+   * image_urls, so evicted cache entries would otherwise 404 once per place
+   * per render). Declared before the bare ':token' read route. Cached photos
+   * are always JPEG.
    */
   @Get(':token/place-photo/:placeId/bytes')
   async placePhotoBytes(@Param('token') token: string, @Param('placeId') placeId: string, @Res() res: Response): Promise<void> {
     const key = this.share.getSharedPlacePhotoKey(token, placeId);
     if (!key) {
-      res.status(404).json({ error: 'Photo not cached' });
+      this.emptyPhoto(res);
       return;
     }
     // Bytes are stream-piped like the maps handler; headers go on before the
@@ -108,16 +111,24 @@ export class SharedController {
     try {
       ({ stream } = await this.storage.getStream('photos-google', key));
     } catch {
-      // Cache-delete race — same terminal state as the old stream error path.
-      if (!res.headersSent) res.status(404).json({ error: 'Photo not cached' });
+      // Cache-delete race — same terminal state as the stream error path.
+      if (!res.headersSent) this.emptyPhoto(res);
       return;
     }
     stream.on('error', () => {
-      if (!res.headersSent) res.status(404).json({ error: 'Photo not cached' });
+      if (!res.headersSent) this.emptyPhoto(res);
     });
     stream.pipe(res);
   }
 
+  // 204 for "no bytes to serve". Overrides the immutable Cache-Control the hit
+  // path already set — a photo that reappears in the cache must not stay hidden
+  // behind a month-old empty response.
+  private emptyPhoto(res: Response): void {
+    res.set('Cache-Control', 'no-store');
+    res.status(204).end();
+  }
+
   @Get(':token')
   read(@Param('token') token: string) {
     const data = this.share.getSharedTripData(token);

server/tests/integration/share.test.ts

@@ -570,24 +570,30 @@ describe('Shared trip — place photos in shared links (issue #1100)', () => {
     expect(Buffer.from(res.body)).toEqual(photoBytes);
   });
 
-  it('SHARE-019 — public proxy 404s for a placeId not in the shared trip', async () => {
+  // Every miss shape answers the same empty 204 (fix: mirrors the maps proxy,
+  // #1727 extended to shared pages). The service still collapses "not in
+  // trip" / "bad token" / "map disabled" into one indistinguishable null, so
+  // no authorization detail leaks through the status change.
+  it('SHARE-019 — public proxy answers an empty 204 for a placeId not in the shared trip', async () => {
     const { token } = await setupSharedPlaceWithPhoto();
     const res = await request(app).get(`/api/shared/${token}/place-photo/ChIJnotInTrip/bytes`);
-    expect(res.status).toBe(404);
-    expect(res.body).toEqual({ error: 'Photo not cached' });
+    expect(res.status).toBe(204);
+    expect(res.headers['cache-control']).toBe('no-store');
+    expect(res.text ?? '').toBe('');
   });
 
-  it('SHARE-020 — public proxy 404s for an invalid token', async () => {
+  it('SHARE-020 — public proxy answers an empty 204 for an invalid token', async () => {
     await setupSharedPlaceWithPhoto();
     const res = await request(app).get(`/api/shared/bad-token/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`);
-    expect(res.status).toBe(404);
-    expect(res.body).toEqual({ error: 'Photo not cached' });
+    expect(res.status).toBe(204);
+    expect(res.headers['cache-control']).toBe('no-store');
+    expect(res.text ?? '').toBe('');
   });
 
   // Regression — GHSA-9hc8 sibling: place photos are part of the map/itinerary,
-  // so the proxy must 404 when the owner disabled the map, even with a valid
-  // token + cached bytes.
-  it('SHARE-025 — public place-photo proxy 404s when share_map=false', async () => {
+  // so the proxy must stream nothing when the owner disabled the map, even
+  // with a valid token + cached bytes.
+  it('SHARE-025 — public place-photo proxy streams nothing when share_map=false', async () => {
     const { user } = createUser(testDb);
     const trip = createTrip(testDb, user.id);
     const place = createPlace(testDb, trip.id, { name: 'Hidden Photo Place' });
@@ -600,7 +606,7 @@ describe('Shared trip — place photos in shared links (issue #1100)', () => {
     cachedFilePath = cached.filePath;
 
     const res = await request(app).get(`/api/shared/${token}/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`);
-    expect(res.status).toBe(404);
-    expect(res.body).toEqual({ error: 'Photo not cached' });
+    expect(res.status).toBe(204);
+    expect(res.text ?? '').toBe('');
   });
 });

server/tests/unit/nest/share.controller.test.ts

@@ -89,8 +89,9 @@ describe('SharedController', () => {
         json: vi.fn(),
         set: vi.fn(),
         type: vi.fn(),
+        end: vi.fn(),
       };
-      return r as unknown as Response & { status: ReturnType<typeof vi.fn>; json: ReturnType<typeof vi.fn>; set: ReturnType<typeof vi.fn>; type: ReturnType<typeof vi.fn> };
+      return r as unknown as Response & { status: ReturnType<typeof vi.fn>; json: ReturnType<typeof vi.fn>; set: ReturnType<typeof vi.fn>; type: ReturnType<typeof vi.fn>; end: ReturnType<typeof vi.fn> };
     }
 
     // Braced body on purpose: mockReset() returns the mock, and a function
@@ -105,11 +106,16 @@ describe('SharedController', () => {
       return new SharedController(svc({ getSharedPlacePhotoKey: vi.fn().mockReturnValue(key) } as Partial<ShareService>), storageStub);
     }
 
-    it('404 without streaming when the photo is not cached for the token', async () => {
+    // #1727's rationale extended to public share pages: shared payloads keep
+    // this URL in place image_urls, so an evicted cache entry means one request
+    // per place per shared-page render — an empty 204 instead of 404 noise.
+    it('204 without a body when the photo is not cached for the token', async () => {
       const res = photoRes();
       await controller(null).placePhotoBytes('tok', 'p1', res);
-      expect(res.status).toHaveBeenCalledWith(404);
-      expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
+      expect(res.status).toHaveBeenCalledWith(204);
+      expect(res.end).toHaveBeenCalled();
+      expect(res.json).not.toHaveBeenCalled();
+      expect(res.set).toHaveBeenLastCalledWith('Cache-Control', 'no-store');
       expect(getStream).not.toHaveBeenCalled();
     });
 
@@ -124,26 +130,30 @@ describe('SharedController', () => {
       expect(stream.pipe).toHaveBeenCalledWith(res);
     });
 
-    it('falls back to 404 when the stream cannot be opened (cache-delete race)', async () => {
+    it('falls back to an empty 204 when the stream cannot be opened (cache-delete race)', async () => {
       getStream.mockRejectedValue(new Error('storage object not found'));
       const res = photoRes();
       await controller('abc.jpg').placePhotoBytes('tok', 'p1', res);
-      expect(res.status).toHaveBeenCalledWith(404);
-      expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
+      expect(res.status).toHaveBeenCalledWith(204);
+      expect(res.end).toHaveBeenCalled();
+      // The hit path already asked for a month of immutable caching — that
+      // header must not survive onto the empty answer.
+      expect(res.set).toHaveBeenLastCalledWith('Cache-Control', 'no-store');
     });
 
-    it('falls back to 404 when the stream errors before headers were sent', async () => {
+    it('falls back to an empty 204 when the stream errors before headers were sent', async () => {
       let onError: () => void = () => {};
       const stream = { on: vi.fn((ev: string, cb: () => void) => { if (ev === 'error') onError = cb; return stream; }), pipe: vi.fn() };
       getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } });
       const res = photoRes();
       await controller('abc.jpg').placePhotoBytes('tok', 'p1', res);
       onError();
-      expect(res.status).toHaveBeenCalledWith(404);
-      expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
+      expect(res.status).toHaveBeenCalledWith(204);
+      expect(res.end).toHaveBeenCalled();
+      expect(res.set).toHaveBeenLastCalledWith('Cache-Control', 'no-store');
     });
 
-    it('does not re-send a 404 when the stream errors after headers were flushed', async () => {
+    it('does not re-send a 204 when the stream errors after headers were flushed', async () => {
       let onError: () => void = () => {};
       const stream = { on: vi.fn((ev: string, cb: () => void) => { if (ev === 'error') onError = cb; return stream; }), pipe: vi.fn() };
       getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } });
@@ -153,6 +163,7 @@ describe('SharedController', () => {
       onError();
       expect(res.status).not.toHaveBeenCalled();
       expect(res.json).not.toHaveBeenCalled();
+      expect(res.end).not.toHaveBeenCalled();
     });
   });
 });