Lectures: Fix file names for downloads with chromium browsers

src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java

@@ -1,6 +1,8 @@
 package de.tum.cit.aet.artemis.core.web;
 
 import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;
+import static org.apache.velocity.shaded.commons.io.FilenameUtils.getBaseName;
+import static org.apache.velocity.shaded.commons.io.FilenameUtils.getExtension;
 
 import java.io.IOException;
 import java.net.FileNameMap;
@@ -409,20 +411,18 @@ public ResponseEntity<byte[]> getExamUserImage(@PathVariable Long examUserId) {
     /**
      * GET /files/attachments/lecture/:lectureId/:filename : Get the lecture attachment
      *
-     * @param lectureId ID of the lecture, the attachment belongs to
-     * @param filename  the filename of the file
+     * @param lectureId      ID of the lecture, the attachment belongs to
+     * @param attachmentName the filename of the file
      * @return The requested file, 403 if the logged-in user is not allowed to access it, or 404 if the file doesn't exist
      */
-    @GetMapping("files/attachments/lecture/{lectureId}/{filename}")
+    @GetMapping("files/attachments/lecture/{lectureId}/{attachmentName}")
     @EnforceAtLeastStudent
-    public ResponseEntity<byte[]> getLectureAttachment(@PathVariable Long lectureId, @PathVariable String filename) {
-        log.debug("REST request to get lecture attachment : {}", filename);
-        String fileNameWithoutSpaces = filename.replaceAll(" ", "_");
-        sanitizeFilenameElseThrow(fileNameWithoutSpaces);
+    public ResponseEntity<byte[]> getLectureAttachment(@PathVariable Long lectureId, @PathVariable String attachmentName) {
+        log.debug("REST request to get lecture attachment : {}", attachmentName);
 
         List<Attachment> lectureAttachments = attachmentRepository.findAllByLectureId(lectureId);
-        Attachment attachment = lectureAttachments.stream().filter(lectureAttachment -> lectureAttachment.getLink().endsWith(fileNameWithoutSpaces)).findAny()
-                .orElseThrow(() -> new EntityNotFoundException("Attachment", filename));
+        Attachment attachment = lectureAttachments.stream().filter(lectureAttachment -> lectureAttachment.getName().equals(getBaseName(attachmentName))).findAny()
+                .orElseThrow(() -> new EntityNotFoundException("Attachment", attachmentName));
 
         // get the course for a lecture attachment
         Lecture lecture = attachment.getLecture();
@@ -431,7 +431,7 @@ public ResponseEntity<byte[]> getLectureAttachment(@PathVariable Long lectureId,
         // check if the user is authorized to access the requested attachment unit
         checkAttachmentAuthorizationOrThrow(course, attachment);
 
-        return buildFileResponse(getActualPathFromPublicPathString(attachment.getLink()), Optional.of(attachment.getName()));
+        return buildFileResponse(getActualPathFromPublicPathString(attachment.getLink()), Optional.of(attachmentName));
     }
 
     /**
@@ -487,7 +487,7 @@ public ResponseEntity<byte[]> getAttachmentUnitAttachment(@PathVariable Long att
 
         // check if the user is authorized to access the requested attachment unit
         checkAttachmentAuthorizationOrThrow(course, attachment);
-        return buildFileResponse(getActualPathFromPublicPathString(attachment.getLink()), Optional.of(attachment.getName()));
+        return buildFileResponse(getActualPathFromPublicPathString(attachment.getLink()), Optional.of(attachment.getName() + "." + getExtension(attachment.getLink())));
     }
 
     /**

src/main/webapp/app/lecture/lecture-attachments.component.html

@@ -61,7 +61,7 @@ <h4 jhiTranslate="artemisApp.lecture.attachments.attachments"></h4>
                                                 <td>{{ attachment.attachmentType }}</td>
                                                 <td>
                                                     @if (!isDownloadingAttachmentLink) {
-                                                        <a class="text-primary" (click)="downloadAttachment(attachment.link || '')">
+                                                        <a class="text-primary" (click)="downloadAttachment(attachment.name || '', attachment.link || '')">
                                                             {{ attachment.name }}
                                                         </a>
                                                     }

src/main/webapp/app/lecture/lecture-attachments.component.ts

@@ -200,10 +200,10 @@ export class LectureAttachmentsComponent implements OnDestroy {
         return item.id;
     }
 
-    downloadAttachment(downloadUrl: string): void {
+    downloadAttachment(downloadName: string, downloadUrl: string): void {
         if (!this.isDownloadingAttachmentLink) {
             this.isDownloadingAttachmentLink = downloadUrl;
-            this.fileService.downloadFile(this.fileService.replaceLectureAttachmentPrefixAndUnderscores(downloadUrl));
+            this.fileService.downloadFileByAttachmentName(downloadUrl, downloadName);
             this.isDownloadingAttachmentLink = undefined;
         }
     }

src/main/webapp/app/overview/course-lectures/attachment-unit/attachment-unit.component.ts

@@ -51,7 +51,7 @@ export class AttachmentUnitComponent extends LectureUnitDirective<AttachmentUnit
 
         if (this.lectureUnit().attachment?.link) {
             const link = this.lectureUnit().attachment!.link!;
-            this.fileService.downloadFile(this.fileService.replaceAttachmentPrefixAndUnderscores(link));
+            this.fileService.downloadFileByAttachmentName(link, this.lectureUnit().attachment!.name!);
             this.onCompletion.emit({ lectureUnit: this.lectureUnit(), completed: true });
         }
     }

src/main/webapp/app/overview/course-lectures/course-lecture-details.component.html

@@ -112,7 +112,7 @@ <h3 jhiTranslate="artemisApp.courseOverview.lectureDetails.attachments"></h3>
                                     <li class="mb-3">
                                         <h5 class="mb-1">
                                             @if (!isDownloadingLink) {
-                                                <a class="text-primary" (click)="downloadAttachment(attachment.link)">
+                                                <a class="text-primary" (click)="downloadAttachment(attachment.link, attachment.name)">
                                                     {{ attachment.name }}
                                                 </a>
                                             }

src/main/webapp/app/overview/course-lectures/course-lecture-details.component.ts

@@ -128,10 +128,10 @@ export class CourseLectureDetailsComponent extends AbstractScienceComponent impl
         return attachment.link.split('.').pop()!;
     }
 
-    downloadAttachment(downloadUrl?: string): void {
-        if (!this.isDownloadingLink && downloadUrl) {
+    downloadAttachment(downloadUrl?: string, donwloadName?: string): void {
+        if (!this.isDownloadingLink && downloadUrl && donwloadName) {
             this.isDownloadingLink = downloadUrl;
-            this.fileService.downloadFile(this.fileService.replaceLectureAttachmentPrefixAndUnderscores(downloadUrl));
+            this.fileService.downloadFileByAttachmentName(downloadUrl, donwloadName);
             this.isDownloadingLink = undefined;
         }
     }

src/main/webapp/app/shared/http/file.service.ts

@@ -82,6 +82,23 @@ export class FileService {
         return newWindow;
     }
 
+    /**
+     * Downloads the file from the provided downloadUrl and the attachment name
+     *
+     * @param downloadUrl url that is stored in the attachment model
+     * @param downloadName the name given to the attachment
+     */
+    downloadFileByAttachmentName(downloadUrl: string, downloadName: string) {
+        const downloadUrlComponents = downloadUrl.split('/');
+        // take the last element
+        const extension = downloadUrlComponents.pop()!.split('.').pop();
+        const restOfUrl = downloadUrlComponents.join('/');
+        const normalizedDownloadUrl = restOfUrl + '/' + encodeURIComponent(downloadName + '.' + extension);
+        const newWindow = window.open('about:blank');
+        newWindow!.location.href = normalizedDownloadUrl;
+        return newWindow;
+    }
+
     /**
      * Downloads the merged PDF file.
      *
@@ -124,12 +141,4 @@ export class FileService {
     replaceAttachmentPrefixAndUnderscores(link: string): string {
         return link.replace(/AttachmentUnit_\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}_/, '').replace(/_/g, ' ');
     }
-
-    /**
-     * Removes the prefix from the file name, and replaces underscore with spaces
-     * @param link
-     */
-    replaceLectureAttachmentPrefixAndUnderscores(link: string): string {
-        return link.replace(/LectureAttachment_\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}_/, '').replace(/_/g, ' ');
-    }
 }

src/test/javascript/spec/component/lecture/lecture-attachments.component.spec.ts

@@ -361,7 +361,7 @@ describe('LectureAttachmentsComponent', () => {
         fixture.detectChanges();
         comp.isDownloadingAttachmentLink = undefined;
         expect(comp.isDownloadingAttachmentLink).toBeUndefined();
-        comp.downloadAttachment('https://my/own/download/url');
+        comp.downloadAttachment('https://my/own/download/url', 'test');
         expect(comp.isDownloadingAttachmentLink).toBeUndefined();
     }));
 

src/test/javascript/spec/component/overview/course-lectures/course-lecture-details.component.spec.ts

@@ -279,13 +279,13 @@ describe('CourseLectureDetailsComponent', () => {
 
     it('should download file for attachment', fakeAsync(() => {
         const fileService = TestBed.inject(FileService);
-        const downloadFileSpy = jest.spyOn(fileService, 'downloadFile');
+        const downloadFileSpy = jest.spyOn(fileService, 'downloadFileByAttachmentName');
         const attachment = getAttachmentUnit(lecture, 1, dayjs()).attachment!;
 
-        courseLecturesDetailsComponent.downloadAttachment(attachment.link);
+        courseLecturesDetailsComponent.downloadAttachment(attachment.link, attachment.name);
 
         expect(downloadFileSpy).toHaveBeenCalledOnce();
-        expect(downloadFileSpy).toHaveBeenCalledWith(attachment.link);
+        expect(downloadFileSpy).toHaveBeenCalledWith(attachment.link, attachment.name);
         expect(courseLecturesDetailsComponent.isDownloadingLink).toBeUndefined();
     }));
 

src/test/javascript/spec/helpers/mocks/service/mock-file.service.ts

@@ -8,6 +8,9 @@ export class MockFileService {
     downloadFile = () => {
         return { subscribe: (fn: (value: any) => void) => fn({ body: new Window() }) };
     };
+    downloadFileByAttachmentName = () => {
+        return { subscribe: (fn: (value: any) => void) => fn({ body: new Window() }) };
+    };
 
     getTemplateFile = () => {
         return of();