refactor(media): add media redirection endpoint

Previous versions of HedgeDoc suffered from the problem
that changing the media backend required manipulation of
the media links in all created notes. We discussed in
#3704 that it's favourable to have an endpoint that
redirects to the image's original URL. When changing the
media backend, the link stays the same but just the
redirect changes.

Signed-off-by: Erik Michelson <github@erik.michelson.eu>

backend/src/api/private/media/media.controller.ts

@@ -7,14 +7,17 @@ import {
   BadRequestException,
   Controller,
   Delete,
+  Get,
   Param,
   Post,
+  Res,
   UploadedFile,
   UseGuards,
   UseInterceptors,
 } from '@nestjs/common';
 import { FileInterceptor } from '@nestjs/platform-express';
 import { ApiBody, ApiConsumes, ApiHeader, ApiTags } from '@nestjs/swagger';
+import { Response } from 'express';
 
 import { PermissionError } from '../../../errors/errors';
 import { SessionGuard } from '../../../identity/session.guard';
@@ -101,6 +104,17 @@ export class MediaController {
     return await this.mediaService.toMediaUploadDto(upload);
   }
 
+  @Get(':filename')
+  @OpenApi(404, 500)
+  async getMedia(
+    @Param('filename') filename: string,
+    @Res() response: Response,
+  ): Promise<void> {
+    const mediaUpload = await this.mediaService.findUploadByFilename(filename);
+    const targetUrl = mediaUpload.fileUrl;
+    response.redirect(targetUrl);
+  }
+
   @Delete(':filename')
   @OpenApi(204, 403, 404, 500)
   async deleteMedia(

backend/src/api/public/media/media.controller.ts

@@ -7,8 +7,10 @@ import {
   BadRequestException,
   Controller,
   Delete,
+  Get,
   Param,
   Post,
+  Res,
   UploadedFile,
   UseGuards,
   UseInterceptors,
@@ -21,6 +23,7 @@ import {
   ApiSecurity,
   ApiTags,
 } from '@nestjs/swagger';
+import { Response } from 'express';
 
 import { TokenAuthGuard } from '../../../auth/token.strategy';
 import { PermissionError } from '../../../errors/errors';
@@ -101,6 +104,17 @@ export class MediaController {
     return await this.mediaService.toMediaUploadDto(upload);
   }
 
+  @Get(':filename')
+  @OpenApi(404, 500)
+  async getMedia(
+    @Param('filename') filename: string,
+    @Res() response: Response,
+  ): Promise<void> {
+    const mediaUpload = await this.mediaService.findUploadByFilename(filename);
+    const targetUrl = mediaUpload.fileUrl;
+    response.redirect(targetUrl);
+  }
+
   @Delete(':filename')
   @OpenApi(204, 403, 404, 500)
   async deleteMedia(

backend/src/media/media-upload-url.dto.ts

@@ -1,15 +0,0 @@
-/*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
- *
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-import { ApiProperty } from '@nestjs/swagger';
-import { IsString } from 'class-validator';
-
-import { BaseDto } from '../utils/base.dto.';
-
-export class MediaUploadUrlDto extends BaseDto {
-  @IsString()
-  @ApiProperty()
-  link: string;
-}

backend/src/media/media-upload.dto.ts

@@ -12,12 +12,12 @@ import { Username } from '../utils/username';
 
 export class MediaUploadDto extends BaseDto {
   /**
-   * The link to the media file.
+   * The id of the media file.
-   * @example "https://example.com/uploads/testfile123.jpg"
+   * @example "testfile123.jpg"
    */
   @IsString()
   @ApiProperty()
-  url: string;
+  id: string;
 
   /**
    * The publicId of the note to which the uploaded file is linked to.

backend/src/media/media.service.ts

@@ -232,7 +232,7 @@ export class MediaService {
   async toMediaUploadDto(mediaUpload: MediaUpload): Promise<MediaUploadDto> {
     const user = await mediaUpload.user;
     return {
-      url: mediaUpload.fileUrl,
+      id: mediaUpload.id,
       notePublicId: (await mediaUpload.note)?.publicId ?? null,
       createdAt: mediaUpload.createdAt,
       username: user?.username ?? null,

backend/test/private-api/me.e2e-spec.ts

@@ -68,18 +68,18 @@ describe('Me', () => {
     expect(responseBefore.body).toHaveLength(0);
 
     const testImage = await fs.readFile('test/public-api/fixtures/test.png');
-    const imageUrls = [];
+    const imageIds = [];
-    imageUrls.push(
+    imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl,
+      (await testSetup.mediaService.saveFile(testImage, user, note1)).id,
     );
-    imageUrls.push(
+    imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl,
+      (await testSetup.mediaService.saveFile(testImage, user, note1)).id,
     );
-    imageUrls.push(
+    imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl,
+      (await testSetup.mediaService.saveFile(testImage, user, note2)).id,
     );
-    imageUrls.push(
+    imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl,
+      (await testSetup.mediaService.saveFile(testImage, user, note2)).id,
     );
 
     const response = await agent
@@ -87,10 +87,10 @@ describe('Me', () => {
       .expect('Content-Type', /json/)
       .expect(200);
     expect(response.body).toHaveLength(4);
-    expect(imageUrls).toContain(response.body[0].url);
+    expect(imageIds).toContain(response.body[0].id);
-    expect(imageUrls).toContain(response.body[1].url);
+    expect(imageIds).toContain(response.body[1].id);
-    expect(imageUrls).toContain(response.body[2].url);
+    expect(imageIds).toContain(response.body[2].id);
-    expect(imageUrls).toContain(response.body[3].url);
+    expect(imageIds).toContain(response.body[3].id);
     const mediaUploads = await testSetup.mediaService.listUploadsByUser(user);
     for (const upload of mediaUploads) {
       await testSetup.mediaService.deleteFile(upload);
@@ -122,7 +122,7 @@ describe('Me', () => {
     expect(dbUser).toBeInstanceOf(User);
     const mediaUploads = await testSetup.mediaService.listUploadsByUser(dbUser);
     expect(mediaUploads).toHaveLength(1);
-    expect(mediaUploads[0].fileUrl).toEqual(upload.fileUrl);
+    expect(mediaUploads[0].id).toEqual(upload.id);
     await agent.delete('/api/private/me').expect(204);
     await expect(
       testSetup.userService.getUserByUsername('hardcoded'),

backend/test/private-api/media.e2e-spec.ts

@@ -71,14 +71,15 @@ describe('Media', () => {
           .set('HedgeDoc-Note', 'test_upload_media')
           .expect('Content-Type', /json/)
           .expect(201);
-        const path: string = uploadResponse.body.url;
+        const fileName: string = uploadResponse.body.id;
         const testImage = await fs.readFile(
           'test/private-api/fixtures/test.png',
         );
-        const downloadResponse = await agent.get(path);
+        const path = '/api/private/media/' + fileName;
+        const apiResponse = await agent.get(path);
+        expect(apiResponse.statusCode).toEqual(302);
+        const downloadResponse = await agent.get(apiResponse.header.location);
         expect(downloadResponse.body).toEqual(testImage);
-        // Remove /uploads/ from path as we just need the filename.
-        const fileName = path.replace('/uploads/', '');
         // delete the file afterwards
         await fs.unlink(join(uploadPath, fileName));
       });
@@ -90,14 +91,15 @@ describe('Media', () => {
           .set('HedgeDoc-Note', 'test_upload_media')
           .expect('Content-Type', /json/)
           .expect(201);
-        const path: string = uploadResponse.body.url;
+        const fileName: string = uploadResponse.body.id;
         const testImage = await fs.readFile(
           'test/private-api/fixtures/test.png',
         );
-        const downloadResponse = await agent.get(path);
+        const path = '/api/private/media/' + fileName;
+        const apiResponse = await agent.get(path);
+        expect(apiResponse.statusCode).toEqual(302);
+        const downloadResponse = await agent.get(apiResponse.header.location);
         expect(downloadResponse.body).toEqual(testImage);
-        // Remove /uploads/ from path as we just need the filename.
-        const fileName = path.replace('/uploads/', '');
         // delete the file afterwards
         await fs.unlink(join(uploadPath, fileName));
       });
@@ -160,7 +162,7 @@ describe('Media', () => {
         user,
         testNote,
       );
-      const filename = upload.fileUrl.split('/').pop() || '';
+      const filename = upload.id;
 
       // login with a different user;
       const agent2 = request.agent(testSetup.app.getHttpServer());

backend/test/private-api/notes.e2e-spec.ts

@@ -412,12 +412,11 @@ describe('Notes', () => {
         .expect('Content-Type', /json/)
         .expect(200);
       expect(responseAfter.body).toHaveLength(1);
-      expect(responseAfter.body[0].url).toEqual(upload0.fileUrl);
+      expect(responseAfter.body[0].id).toEqual(upload0.id);
-      expect(responseAfter.body[0].url).not.toEqual(upload1.fileUrl);
+      expect(responseAfter.body[0].id).not.toEqual(upload1.id);
       for (const upload of [upload0, upload1]) {
-        const fileName = upload.fileUrl.replace('/uploads/', '');
         // delete the file afterwards
-        await fs.unlink(join(uploadPath, fileName));
+        await fs.unlink(join(uploadPath, upload.id));
       }
       await fs.rm(uploadPath, { recursive: true });
     });

backend/test/public-api/me.e2e-spec.ts

@@ -199,18 +199,18 @@ describe('Me', () => {
     expect(response1.body).toHaveLength(0);
 
     const testImage = await fs.readFile('test/public-api/fixtures/test.png');
-    const imageUrls = [];
+    const imageIds = [];
-    imageUrls.push(
+    imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl,
+      (await testSetup.mediaService.saveFile(testImage, user, note1)).id,
     );
-    imageUrls.push(
+    imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note1)).fileUrl,
+      (await testSetup.mediaService.saveFile(testImage, user, note1)).id,
     );
-    imageUrls.push(
+    imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl,
+      (await testSetup.mediaService.saveFile(testImage, user, note2)).id,
     );
-    imageUrls.push(
+    imageIds.push(
-      (await testSetup.mediaService.saveFile(testImage, user, note2)).fileUrl,
+      (await testSetup.mediaService.saveFile(testImage, user, note2)).id,
     );
 
     const response = await request(httpServer)
@@ -218,14 +218,13 @@ describe('Me', () => {
       .expect('Content-Type', /json/)
       .expect(200);
     expect(response.body).toHaveLength(4);
-    expect(imageUrls).toContain(response.body[0].url);
+    expect(imageIds).toContain(response.body[0].id);
-    expect(imageUrls).toContain(response.body[1].url);
+    expect(imageIds).toContain(response.body[1].id);
-    expect(imageUrls).toContain(response.body[2].url);
+    expect(imageIds).toContain(response.body[2].id);
-    expect(imageUrls).toContain(response.body[3].url);
+    expect(imageIds).toContain(response.body[3].id);
-    for (const fileUrl of imageUrls) {
+    for (const imageId of imageIds) {
-      const fileName = fileUrl.replace('/uploads/', '');
       // delete the file afterwards
-      await fs.unlink(join(uploadPath, fileName));
+      await fs.unlink(join(uploadPath, imageId));
     }
     await fs.rm(uploadPath, { recursive: true });
   });

backend/test/public-api/media.e2e-spec.ts

@@ -41,21 +41,23 @@ describe('Media', () => {
 
   describe('POST /media', () => {
     it('works', async () => {
-      const uploadResponse = await request(testSetup.app.getHttpServer())
+      const agent = request.agent(testSetup.app.getHttpServer());
+      const uploadResponse = await agent
         .post('/api/v2/media')
         .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`)
         .attach('file', 'test/public-api/fixtures/test.png')
         .set('HedgeDoc-Note', 'testAlias1')
         .expect('Content-Type', /json/)
         .expect(201);
-      const path: string = uploadResponse.body.url;
+      const fileName = uploadResponse.body.id;
+      const path: string = '/api/v2/media/' + fileName;
       const testImage = await fs.readFile('test/public-api/fixtures/test.png');
-      const downloadResponse = await request(testSetup.app.getHttpServer()).get(
+      const apiResponse = await agent
-        path,
+        .get(path)
-      );
+        .set('Authorization', `Bearer ${testSetup.authTokens[0].secret}`);
+      expect(apiResponse.statusCode).toEqual(302);
+      const downloadResponse = await agent.get(apiResponse.header.location);
       expect(downloadResponse.body).toEqual(testImage);
-      // Remove /uploads/ from path as we just need the filename.
-      const fileName = path.replace('/uploads/', '');
       // delete the file afterwards
       await fs.unlink(join(uploadPath, fileName));
     });

backend/test/public-api/notes.e2e-spec.ts

@@ -495,12 +495,11 @@ describe('Notes', () => {
         .expect('Content-Type', /json/)
         .expect(200);
       expect(responseAfter.body).toHaveLength(1);
-      expect(responseAfter.body[0].url).toEqual(upload0.fileUrl);
+      expect(responseAfter.body[0].id).toEqual(upload0.id);
-      expect(responseAfter.body[0].url).not.toEqual(upload1.fileUrl);
+      expect(responseAfter.body[0].id).not.toEqual(upload1.id);
       for (const upload of [upload0, upload1]) {
-        const fileName = upload.fileUrl.replace('/uploads/', '');
         // delete the file afterwards
-        await fs.unlink(join(uploadPath, fileName));
+        await fs.unlink(join(uploadPath, upload.id));
       }
       await fs.rm(uploadPath, { recursive: true });
     });

frontend/cypress/e2e/fileUpload.spec.ts

@@ -1,10 +1,10 @@
 /*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 
-const imageUrl = 'http://example.com/non-existing.png'
+const imageId = 'non-existing.png'
 
 describe('File upload', () => {
   beforeEach(() => {
@@ -22,7 +22,7 @@ describe('File upload', () => {
         {
           statusCode: 201,
           body: {
-            url: imageUrl
+            id: imageId
           }
         }
       )
@@ -38,7 +38,7 @@ describe('File upload', () => {
         },
         { force: true }
       )
-      cy.get('.cm-line').contains(`![demo.png](${imageUrl})`)
+      cy.get('.cm-line').contains(`![demo.png](http://127.0.0.1:3001/api/private/media/${imageId})`)
     })
 
     it('via paste', () => {
@@ -51,7 +51,7 @@ describe('File upload', () => {
           }
         }
         cy.get('.cm-content').trigger('paste', pasteEvent)
-        cy.get('.cm-line').contains(`![](${imageUrl})`)
+        cy.get('.cm-line').contains(`![](http://127.0.0.1:3001/api/private/media/${imageId})`)
       })
     })
 
@@ -65,7 +65,7 @@ describe('File upload', () => {
         },
         { action: 'drag-drop', force: true }
       )
-      cy.get('.cm-line').contains(`![demo.png](${imageUrl})`)
+      cy.get('.cm-line').contains(`![demo.png](http://127.0.0.1:3001/api/private/media/${imageId})`)
     })
   })
 

frontend/src/api/media/types.ts

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 export interface MediaUpload {
-  url: string
+  id: string
   noteId: string | null
   createdAt: string
   username: string

frontend/src/components/editor-page/editor-pane/hooks/use-handle-upload.tsx

@@ -15,6 +15,7 @@ import type { CursorSelection } from '../tool-bar/formatters/types/cursor-select
 import type { EditorView } from '@codemirror/view'
 import { useCallback } from 'react'
 import { useTranslation } from 'react-i18next'
+import { useBaseUrl } from '../../../../hooks/common/use-base-url'
 
 /**
  * @param view the codemirror instance that is used to insert the Markdown code
@@ -37,6 +38,7 @@ type handleUploadSignature = (
 export const useHandleUpload = (): handleUploadSignature => {
   const { t } = useTranslation()
   const { showErrorNotification } = useUiNotifications()
+  const baseUrl = useBaseUrl()
 
   return useCallback(
     (view, file, cursorSelection, description, additionalUrlText) => {
@@ -58,8 +60,9 @@ export const useHandleUpload = (): handleUploadSignature => {
         return replaceSelection(cursorSelection ?? currentSelection, uploadPlaceholder, false)
       })
       uploadFile(noteId, file)
-        .then(({ url }) => {
+        .then(({ id }) => {
-          const replacement = `![${description ?? file.name ?? ''}](${url}${additionalUrlText ?? ''})`
+          const fullUrl = `${baseUrl}api/private/media/${id}`
+          const replacement = `![${description ?? file.name ?? ''}](${fullUrl}${additionalUrlText ?? ''})`
           changeContent(({ markdownContent }) => [
             replaceInContent(markdownContent, uploadPlaceholder, replacement),
             undefined
@@ -74,6 +77,6 @@ export const useHandleUpload = (): handleUploadSignature => {
           ])
         })
     },
-    [showErrorNotification, t]
+    [showErrorNotification, t, baseUrl]
   )
 }

frontend/src/pages/api/private/me/media.ts

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file)
+ * SPDX-FileCopyrightText: 2024 The HedgeDoc developers (see AUTHORS file)
  *
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -12,13 +12,13 @@ const handler = (req: NextApiRequest, res: NextApiResponse) => {
     {
       username: 'tilman',
       createdAt: '2022-03-20T20:36:32Z',
-      url: 'https://dummyimage.com/256/f00',
+      id: 'dummy.png',
       noteId: 'features'
     },
     {
       username: 'tilman',
       createdAt: '2022-03-20T20:36:57+0000',
-      url: 'https://dummyimage.com/256/00f',
+      id: 'dummy.png',
       noteId: null
     }
   ])

frontend/src/pages/api/private/media.ts

@@ -20,7 +20,7 @@ const handler = async (req: NextApiRequest, res: NextApiResponse): Promise<void>
     req,
     res,
     {
-      url: '/public/img/avatar.png',
+      id: '/public/img/avatar.png',
       noteId: null,
       username: 'test',
       createdAt: '2022-02-27T21:54:23.856Z'