Skip to content

Feature/open file in UI#66

Merged
MartinStenhagen merged 6 commits into
mainfrom
feature/open-file-in-ui
Apr 28, 2026
Merged

Feature/open file in UI#66
MartinStenhagen merged 6 commits into
mainfrom
feature/open-file-in-ui

Conversation

@Rickank

@Rickank Rickank commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Inline file preview: open documents inside the app via a per-file “Open file” button; previews load into an embedded preview frame.
    • Preview buttons added to document lists and meeting agendas for quick access.
  • Bug Fixes

    • Improved robustness when determining file content types so previews and downloads continue to work with malformed/missing media-type values.
  • Security

    • Tightened framing policy (same-origin) to allow safe embedding of previews.

@coderabbitai

coderabbitai Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@MartinStenhagen has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 43 minutes and 4 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f558480f-b66c-4625-bef6-e8fc04402c4e

📥 Commits

Reviewing files that changed from the base of the PR and between b9b512f and 58ce1ae.

📒 Files selected for processing (1)
  • src/main/java/backendlab/team4you/casefile/CaseFileController.java
📝 Walkthrough

Walkthrough

Adds an inline file preview flow: a new preview streaming endpoint, a fragment endpoint that returns an iframe, UI buttons and client-side toggle to load/unload the iframe, and a same-origin frame security header; download/preview media-type parsing is made resilient to invalid media type strings.

Changes

Cohort / File(s) Summary
Backend: preview streaming & controller
src/main/java/backendlab/team4you/casefile/CaseFileController.java
Added previewFile GET endpoint that streams the stored file using Content-Disposition: inline, derives/falls back content type, and guards against invalid media type parsing.
Backend: preview fragment view
src/main/java/backendlab/team4you/casefile/ui/CaseFileViewController.java
Added previewFrame GET endpoint that returns the Thymeleaf fragment for an iframe (file-preview-frame :: frame) with caseId and fileId in the model.
Security
src/main/java/backendlab/team4you/config/SecurityConfig.java
Sets X-Frame-Options to sameOrigin to permit same-origin iframe embedding.
UI: template fragment
src/main/resources/templates/fragments/case-management/file-preview-frame.html
New Thymeleaf fragment frame rendering an <iframe> whose src targets the preview endpoint (fixed width/height).
UI: file lists and meetings
src/main/resources/templates/fragments/case-management/case-file-list.html, src/main/resources/templates/fragments/admin-meetings.html
Added per-file preview toggle buttons (shown when file.canDownload) and client-side togglePreview(btn) logic to fetch/insert or remove the preview frame HTML.

Sequence Diagram(s)

sequenceDiagram
  participant User as Client (browser)
  participant UI as Case file list (JS)
  participant ViewCtrl as CaseFileViewController
  participant IFrame as Browser iframe
  participant PreviewCtrl as CaseFileController
  participant Storage as File storage

  User->>UI: click "Preview" button
  UI->>ViewCtrl: GET /dashboard/.../preview-frame (fetch fragment)
  ViewCtrl-->>UI: 200 HTML fragment (iframe src=.../preview)
  UI-->>UI: insert fragment into DOM (iframe added)
  IFrame->>PreviewCtrl: GET /api/cases/{caseId}/files/{fileId}/preview (navigation)
  PreviewCtrl->>Storage: retrieve file bytes & metadata
  PreviewCtrl-->>IFrame: 200 StreamingResponseBody (Content-Type, Content-Disposition:inline)
  IFrame-->>User: browser renders streamed content inline
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • MartinStenhagen
  • JohanHiths

Poem

🐰 I opened a frame with a curious eye,
Files flowed inline beneath the same-origin sky.
A toggle, a fetch, an iframe in bloom,
Rabbits peek softly — no downloads, just zoom. 👁️🐇

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Feature/open file in UI' is vague and uses a generic naming convention (Feature/) without clearly conveying what the actual change accomplishes. Replace with a more specific, descriptive title that clearly indicates the main feature, e.g., 'Add file preview capability with inline display' or 'Implement file preview in case management UI'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/open-file-in-ui

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
src/main/resources/templates/fragments/case-management/case-file-list.html (1)

81-95: Duplicate togglePreview function exists in admin-meetings.html.

This function is nearly identical to the one in admin-meetings.html (lines 359-372), differing only in the parent selector (.case-file-item vs .agenda-document-row). Consider extracting to a shared JavaScript file with a configurable selector parameter.

♻️ Example shared utility
// In a shared JS file, e.g., /js/file-preview.js
function togglePreview(btn, parentSelector) {
    const item = btn.closest(parentSelector);
    const existing = item.querySelector('.file-preview-frame');
    if (existing) {
        existing.remove();
        return;
    }
    const url = btn.getAttribute('data-preview-url');
    fetch(url)
        .then(res => {
            if (!res.ok) throw new Error('Preview failed');
            return res.text();
        })
        .then(html => {
            item.insertAdjacentHTML('beforeend', html);
        })
        .catch(err => console.error('Failed to load preview:', err));
}

Then call with: onclick="togglePreview(this, '.case-file-item')"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/templates/fragments/case-management/case-file-list.html`
around lines 81 - 95, Duplicate togglePreview implementations exist (one in this
file and another in admin-meetings.html); extract to a shared JS utility (e.g.,
/js/file-preview.js) as a single function togglePreview(btn, parentSelector)
that uses btn.closest(parentSelector), looks for '.file-preview-frame', reads
'data-preview-url', checks response.ok, handles fetch errors, and inserts the
HTML; then replace the inline functions and update calls to
onclick="togglePreview(this, '.case-file-item')" and
onclick="togglePreview(this, '.agenda-document-row')" respectively.
src/main/java/backendlab/team4you/casefile/ui/CaseFileViewController.java (1)

40-49: Consider adding authorization check for consistency.

This endpoint serves the preview frame without verifying whether the user has permission to view the file. While the actual file content is protected by the /api/cases/.../preview endpoint (which does check authorization), adding a check here would:

  1. Provide consistent authorization behavior across endpoints
  2. Fail fast with a proper error instead of rendering an iframe that will fail to load
  3. Prevent potential enumeration of valid caseId/fileId combinations
🛡️ Suggested authorization check
     `@GetMapping`("/case-records/{caseId}/files/{fileId}/preview-frame")
     public String previewFrame(
             `@PathVariable` Long caseId,
             `@PathVariable` Long fileId,
-            Model model
+            Model model,
+            Principal principal
     ) {
+        UserEntity currentUser = userService.getCurrentUser(principal);
+        // Verify user can view this file before rendering the frame
+        caseFileService.getCaseFileForViewer(caseId, fileId, currentUser);
+
         model.addAttribute("caseId", caseId);
         model.addAttribute("fileId", fileId);
         return "fragments/case-management/file-preview-frame :: frame";
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/backendlab/team4you/casefile/ui/CaseFileViewController.java`
around lines 40 - 49, The previewFrame method currently renders the iframe
without permission checks; update previewFrame in CaseFileViewController to
validate access before adding attributes by calling your authorization utility
(e.g., CasePermissionService.canViewFile(caseId, fileId) or
SecurityService.hasAccessToCaseFile) and, on failure, throw
AccessDeniedException or return a 403/forbidden view; ensure you reference
previewFrame and the chosen auth helper (CasePermissionService or
SecurityService) so the endpoint fails fast and prevents rendering the iframe
when the user is not authorized.
src/main/java/backendlab/team4you/casefile/CaseFileController.java (1)

89-119: Consider extracting shared logic with downloadFile method.

The previewFile and downloadFile methods share nearly identical code (authorization, media type parsing, streaming body construction). The only difference is ContentDisposition.inline() vs ContentDisposition.attachment(). Consider extracting a helper method to reduce duplication.

♻️ Suggested refactoring
private ResponseEntity<StreamingResponseBody> streamFile(
        Long caseRecordId, Long fileId, Principal principal, boolean inline) {
    UserEntity currentUser = userService.getCurrentUser(principal);
    CaseFile caseFile = caseFileService.getCaseFileForViewer(caseRecordId, fileId, currentUser);

    MediaType mediaType = MediaType.APPLICATION_OCTET_STREAM;
    if (caseFile.getContentType() != null && !caseFile.getContentType().isBlank()) {
        mediaType = MediaType.parseMediaType(caseFile.getContentType());
    }

    StreamingResponseBody body = outputStream -> {
        try (InputStream stream = caseFileService.downloadFile(caseRecordId, fileId, currentUser)) {
            stream.transferTo(outputStream);
        }
    };

    ContentDisposition disposition = inline
            ? ContentDisposition.inline().filename(caseFile.getOriginalFilename(), StandardCharsets.UTF_8).build()
            : ContentDisposition.attachment().filename(caseFile.getOriginalFilename(), StandardCharsets.UTF_8).build();

    return ResponseEntity.ok()
            .header(HttpHeaders.CONTENT_DISPOSITION, disposition.toString())
            .contentType(mediaType)
            .body(body);
}

Then simplify both endpoints:

`@GetMapping`("/{fileId}")
public ResponseEntity<StreamingResponseBody> downloadFile(...) {
    return streamFile(caseRecordId, fileId, principal, false);
}

`@GetMapping`("/{fileId}/preview")
public ResponseEntity<StreamingResponseBody> previewFile(...) {
    return streamFile(caseRecordId, fileId, principal, true);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/backendlab/team4you/casefile/CaseFileController.java` around
lines 89 - 119, Extract the duplicated streaming/authorization logic from
previewFile and downloadFile into a new private helper (e.g., streamFile) that
accepts (Long caseRecordId, Long fileId, Principal principal, boolean inline);
inside it call userService.getCurrentUser(principal) and
caseFileService.getCaseFileForViewer(...), compute MediaType once (using
MediaType.parseMediaType when contentType present), build the
StreamingResponseBody by calling caseFileService.downloadFile(...). Use the
inline flag to choose ContentDisposition.inline() vs
ContentDisposition.attachment() with caseFile.getOriginalFilename() and return
the ResponseEntity; then simplify previewFile and downloadFile to delegate to
streamFile with inline=true/false.
src/main/resources/templates/fragments/case-management/file-preview-frame.html (1)

1-8: Add sandbox attribute to the iframe for defense-in-depth.

The iframe loads user-uploaded files of any type via the preview endpoint, which uses ContentDisposition.inline() to display them directly. Since users can upload HTML files with embedded scripts, adding a sandbox would prevent script execution while maintaining functionality for PDFs, images, and other content.

🛡️ Suggested security improvement
 <div th:fragment="frame" class="file-preview-frame">
     <iframe
             th:src="@{/api/cases/{caseId}/files/{fileId}/preview(caseId=${caseId}, fileId=${fileId})}"
             width="100%"
             height="600px"
-            style="border: none;">
+            style="border: none;"
+            sandbox="allow-same-origin">
     </iframe>
 </div>

The sandbox="allow-same-origin" attribute restricts the iframe's capabilities (disables scripts, forms, plugins, etc.) while preserving same-origin access needed for the preview endpoint requests.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/resources/templates/fragments/case-management/file-preview-frame.html`
around lines 1 - 8, The iframe in the "frame" fragment (class
file-preview-frame) renders user-uploaded content via the
/api/cases/{caseId}/files/{fileId}/preview endpoint and currently allows script
execution; add a sandbox attribute to the iframe element (e.g.,
sandbox="allow-same-origin") to disable scripts, forms and plugins while
preserving same-origin access for the preview endpoint to mitigate XSS from
uploaded HTML files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/backendlab/team4you/casefile/CaseFileController.java`:
- Around line 98-101: CaseFileController currently calls
MediaType.parseMediaType(...) (in the block that sets mediaType and also in
downloadFile) which can throw InvalidMediaTypeException; wrap those
parseMediaType calls in a try-catch that catches InvalidMediaTypeException, log
a warning (including the bad content type and caseFile id) and fall back to
MediaType.APPLICATION_OCTET_STREAM so malformed content types do not propagate
an exception.

In `@src/main/resources/templates/fragments/admin-meetings.html`:
- Around line 358-372: The togglePreview function performs a fetch without any
error handling or response status checks, so preview failures are silent; update
togglePreview (the function that queries .agenda-document-row and inserts
.file-preview-frame) to check response.ok and throw on non-2xx, add a .catch
handler on the promise chain to handle network/parse errors, and on error either
insert a small error message element into the item (e.g., a .file-preview-error
node) or display an alert so users get visible feedback when the preview fails
to load.

In `@src/main/resources/templates/fragments/case-management/case-file-list.html`:
- Around line 81-95: The fetch in function togglePreview lacks error handling;
update togglePreview to check response.ok after fetch(url) and throw or handle
non-2xx statuses, and add a .catch handler for network or parsing errors that
injects a visible error node into the .case-file-item (or uses alert) so users
get feedback; keep the existing logic that removes existing .file-preview-frame,
ensure you reference the data-preview-url attribute and existing
.file-preview-frame element when inserting the error message.

---

Nitpick comments:
In `@src/main/java/backendlab/team4you/casefile/CaseFileController.java`:
- Around line 89-119: Extract the duplicated streaming/authorization logic from
previewFile and downloadFile into a new private helper (e.g., streamFile) that
accepts (Long caseRecordId, Long fileId, Principal principal, boolean inline);
inside it call userService.getCurrentUser(principal) and
caseFileService.getCaseFileForViewer(...), compute MediaType once (using
MediaType.parseMediaType when contentType present), build the
StreamingResponseBody by calling caseFileService.downloadFile(...). Use the
inline flag to choose ContentDisposition.inline() vs
ContentDisposition.attachment() with caseFile.getOriginalFilename() and return
the ResponseEntity; then simplify previewFile and downloadFile to delegate to
streamFile with inline=true/false.

In `@src/main/java/backendlab/team4you/casefile/ui/CaseFileViewController.java`:
- Around line 40-49: The previewFrame method currently renders the iframe
without permission checks; update previewFrame in CaseFileViewController to
validate access before adding attributes by calling your authorization utility
(e.g., CasePermissionService.canViewFile(caseId, fileId) or
SecurityService.hasAccessToCaseFile) and, on failure, throw
AccessDeniedException or return a 403/forbidden view; ensure you reference
previewFrame and the chosen auth helper (CasePermissionService or
SecurityService) so the endpoint fails fast and prevents rendering the iframe
when the user is not authorized.

In `@src/main/resources/templates/fragments/case-management/case-file-list.html`:
- Around line 81-95: Duplicate togglePreview implementations exist (one in this
file and another in admin-meetings.html); extract to a shared JS utility (e.g.,
/js/file-preview.js) as a single function togglePreview(btn, parentSelector)
that uses btn.closest(parentSelector), looks for '.file-preview-frame', reads
'data-preview-url', checks response.ok, handles fetch errors, and inserts the
HTML; then replace the inline functions and update calls to
onclick="togglePreview(this, '.case-file-item')" and
onclick="togglePreview(this, '.agenda-document-row')" respectively.

In
`@src/main/resources/templates/fragments/case-management/file-preview-frame.html`:
- Around line 1-8: The iframe in the "frame" fragment (class file-preview-frame)
renders user-uploaded content via the /api/cases/{caseId}/files/{fileId}/preview
endpoint and currently allows script execution; add a sandbox attribute to the
iframe element (e.g., sandbox="allow-same-origin") to disable scripts, forms and
plugins while preserving same-origin access for the preview endpoint to mitigate
XSS from uploaded HTML files.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4c34a111-adda-4419-b221-962f181b634c

📥 Commits

Reviewing files that changed from the base of the PR and between a93784a and 9b43a59.

📒 Files selected for processing (6)
  • src/main/java/backendlab/team4you/casefile/CaseFileController.java
  • src/main/java/backendlab/team4you/casefile/ui/CaseFileViewController.java
  • src/main/java/backendlab/team4you/config/SecurityConfig.java
  • src/main/resources/templates/fragments/admin-meetings.html
  • src/main/resources/templates/fragments/case-management/case-file-list.html
  • src/main/resources/templates/fragments/case-management/file-preview-frame.html

Comment thread src/main/java/backendlab/team4you/casefile/CaseFileController.java
Comment thread src/main/resources/templates/fragments/admin-meetings.html
Comment on lines +81 to +95
<script>
function togglePreview(btn) {
const item = btn.closest('.case-file-item');
const existing = item.querySelector('.file-preview-frame');
if (existing) {
existing.remove();
return;
}
const url = btn.getAttribute('data-preview-url');
fetch(url)
.then(res => res.text())
.then(html => {
item.insertAdjacentHTML('beforeend', html);
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add error handling for the fetch request.

The togglePreview function doesn't handle fetch failures. If the network request fails or returns an error status, users won't receive feedback.

🐛 Proposed fix with error handling
     <script>
         function togglePreview(btn) {
             const item = btn.closest('.case-file-item');
             const existing = item.querySelector('.file-preview-frame');
             if (existing) {
                 existing.remove();
                 return;
             }
             const url = btn.getAttribute('data-preview-url');
             fetch(url)
-                .then(res => res.text())
+                .then(res => {
+                    if (!res.ok) throw new Error('Preview failed');
+                    return res.text();
+                })
                 .then(html => {
                     item.insertAdjacentHTML('beforeend', html);
+                })
+                .catch(err => {
+                    console.error('Failed to load preview:', err);
                 });
         }
     </script>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<script>
function togglePreview(btn) {
const item = btn.closest('.case-file-item');
const existing = item.querySelector('.file-preview-frame');
if (existing) {
existing.remove();
return;
}
const url = btn.getAttribute('data-preview-url');
fetch(url)
.then(res => res.text())
.then(html => {
item.insertAdjacentHTML('beforeend', html);
});
}
<script>
function togglePreview(btn) {
const item = btn.closest('.case-file-item');
const existing = item.querySelector('.file-preview-frame');
if (existing) {
existing.remove();
return;
}
const url = btn.getAttribute('data-preview-url');
fetch(url)
.then(res => {
if (!res.ok) throw new Error('Preview failed');
return res.text();
})
.then(html => {
item.insertAdjacentHTML('beforeend', html);
})
.catch(err => {
console.error('Failed to load preview:', err);
});
}
</script>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/templates/fragments/case-management/case-file-list.html`
around lines 81 - 95, The fetch in function togglePreview lacks error handling;
update togglePreview to check response.ok after fetch(url) and throw or handle
non-2xx statuses, and add a .catch handler for network or parsing errors that
injects a visible error node into the .case-file-item (or uses alert) so users
get feedback; keep the existing logic that removes existing .file-preview-frame,
ensure you reference the data-preview-url attribute and existing
.file-preview-frame element when inserting the error message.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/main/java/backendlab/team4you/casefile/CaseFileController.java (1)

70-73: ⚠️ Potential issue | 🟡 Minor

Don’t swallow InvalidMediaTypeException silently.

Line 72 and Line 107 catch and ignore invalid content types with no trace. Please log a warning (include case file id + bad value) before falling back, so malformed metadata is diagnosable.

🔧 Suggested patch
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
...
 public class CaseFileController {
+    private static final Logger log = LoggerFactory.getLogger(CaseFileController.class);
...
         if (caseFile.getContentType() != null && !caseFile.getContentType().isBlank()) {
             try {
                 mediaType = MediaType.parseMediaType(caseFile.getContentType());
             } catch (InvalidMediaTypeException e) {
+                log.warn("Invalid content type '{}' for caseFileId={}; fallback to application/octet-stream",
+                        caseFile.getContentType(), caseFile.getId());
             }
         }
...
         if (caseFile.getContentType() != null && !caseFile.getContentType().isBlank()) {
             try {
                 mediaType = MediaType.parseMediaType(caseFile.getContentType());
             } catch (InvalidMediaTypeException e) {
+                log.warn("Invalid content type '{}' for caseFileId={}; fallback to application/octet-stream",
+                        caseFile.getContentType(), caseFile.getId());
             }
         }
#!/bin/bash
# Verify empty InvalidMediaTypeException catches and logging presence.
rg -nPU 'catch\s*\(\s*InvalidMediaTypeException\s+\w+\s*\)\s*\{\s*\}' src/main/java
rg -nP 'InvalidMediaTypeException|LoggerFactory|log\.warn' src/main/java/backendlab/team4you/casefile/CaseFileController.java

Also applies to: 105-108

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/backendlab/team4you/casefile/CaseFileController.java` around
lines 70 - 73, The code swallows InvalidMediaTypeException in the
MediaType.parseMediaType call; update the catch blocks around
MediaType.parseMediaType(caseFile.getContentType()) to log a warning including
the case file id (caseFile.getId() or caseFile.getCaseFileId()) and the
malformed contentType value before falling back to a default MediaType; use the
existing logger (LoggerFactory/LOGGER) and include the exception
message/stacktrace in the log so both the bad value and the exception are
recorded (apply the same change to both occurrences near parseMediaType).
🧹 Nitpick comments (1)
src/main/java/backendlab/team4you/casefile/CaseFileController.java (1)

59-92: Extract shared stream-response logic used by download + preview.

downloadFile and previewFile duplicate media-type parsing and stream body construction. Pull this into one private helper and parameterize disposition (attachment vs inline) to reduce drift and keep security fixes centralized.

♻️ Refactor sketch
+    private ResponseEntity<StreamingResponseBody> buildFileResponse(
+            Long caseRecordId,
+            Long fileId,
+            UserEntity currentUser,
+            boolean inline
+    ) {
+        CaseFile caseFile = caseFileService.getCaseFileForViewer(caseRecordId, fileId, currentUser);
+        MediaType mediaType = MediaType.APPLICATION_OCTET_STREAM;
+        if (caseFile.getContentType() != null && !caseFile.getContentType().isBlank()) {
+            try {
+                mediaType = MediaType.parseMediaType(caseFile.getContentType());
+            } catch (InvalidMediaTypeException e) {
+                // log fallback
+            }
+        }
+        StreamingResponseBody body = outputStream -> {
+            try (InputStream stream = caseFileService.downloadFile(caseRecordId, fileId, currentUser)) {
+                stream.transferTo(outputStream);
+            }
+        };
+        ContentDisposition disposition = (inline ? ContentDisposition.inline() : ContentDisposition.attachment())
+                .filename(caseFile.getOriginalFilename(), StandardCharsets.UTF_8)
+                .build();
+        return ResponseEntity.ok()
+                .header(HttpHeaders.CONTENT_DISPOSITION, disposition.toString())
+                .contentType(mediaType)
+                .body(body);
+    }

Also applies to: 94-127

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/backendlab/team4you/casefile/CaseFileController.java` around
lines 59 - 92, Refactor duplicated media-type parsing and stream-response
construction used by downloadFile and previewFile into a single private helper
(e.g., buildStreamResponse or createStreamResponse) that accepts caseRecordId,
fileId, Principal/UserEntity (or currentUser), and a disposition type parameter
("attachment" vs "inline"); move the MediaType parsing logic (using
caseFile.getContentType() and MediaType.parseMediaType with
InvalidMediaTypeException handling) and the StreamingResponseBody creation
(calling caseFileService.downloadFile and transferTo) into that helper, and have
downloadFile and previewFile call the helper to build the ResponseEntity with
appropriate ContentDisposition (attachment or inline) while preserving filename
encoding via StandardCharsets.UTF_8 and existing headers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/backendlab/team4you/casefile/CaseFileController.java`:
- Around line 94-127: The previewFile handler currently serves arbitrary
uploaded Content-Type inline; restrict inline rendering by implementing a strict
allowlist (e.g., "application/pdf", "image/*", "text/plain") when reading
caseFile.getContentType() inside previewFile in CaseFileController.java: if the
detected media type is not in the allowlist, change the response to force a
download (Content-Disposition: attachment with original filename) or return
415/403 per policy; always add the header "X-Content-Type-Options: nosniff" to
the ResponseEntity; ensure MediaType parsing still falls back safely and that
ContentDisposition.inline() is only used for allowed types.

---

Duplicate comments:
In `@src/main/java/backendlab/team4you/casefile/CaseFileController.java`:
- Around line 70-73: The code swallows InvalidMediaTypeException in the
MediaType.parseMediaType call; update the catch blocks around
MediaType.parseMediaType(caseFile.getContentType()) to log a warning including
the case file id (caseFile.getId() or caseFile.getCaseFileId()) and the
malformed contentType value before falling back to a default MediaType; use the
existing logger (LoggerFactory/LOGGER) and include the exception
message/stacktrace in the log so both the bad value and the exception are
recorded (apply the same change to both occurrences near parseMediaType).

---

Nitpick comments:
In `@src/main/java/backendlab/team4you/casefile/CaseFileController.java`:
- Around line 59-92: Refactor duplicated media-type parsing and stream-response
construction used by downloadFile and previewFile into a single private helper
(e.g., buildStreamResponse or createStreamResponse) that accepts caseRecordId,
fileId, Principal/UserEntity (or currentUser), and a disposition type parameter
("attachment" vs "inline"); move the MediaType parsing logic (using
caseFile.getContentType() and MediaType.parseMediaType with
InvalidMediaTypeException handling) and the StreamingResponseBody creation
(calling caseFileService.downloadFile and transferTo) into that helper, and have
downloadFile and previewFile call the helper to build the ResponseEntity with
appropriate ContentDisposition (attachment or inline) while preserving filename
encoding via StandardCharsets.UTF_8 and existing headers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 09e0c251-7b7f-4585-9f6a-bef3edcf5131

📥 Commits

Reviewing files that changed from the base of the PR and between 9b43a59 and b9b512f.

📒 Files selected for processing (4)
  • src/main/java/backendlab/team4you/casefile/CaseFileController.java
  • src/main/java/backendlab/team4you/config/SecurityConfig.java
  • src/main/resources/templates/fragments/admin-meetings.html
  • src/main/resources/templates/fragments/case-management/case-file-list.html
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/main/java/backendlab/team4you/config/SecurityConfig.java
  • src/main/resources/templates/fragments/case-management/case-file-list.html
  • src/main/resources/templates/fragments/admin-meetings.html

Comment thread src/main/java/backendlab/team4you/casefile/CaseFileController.java
@MartinStenhagen
MartinStenhagen merged commit 60985f8 into main Apr 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants