fix: 馃悰 Fix download PDF, to be a Open in new tab - #9
fix: 馃悰 Fix download PDF, to be a Open in new tab#9yagoquesadafloriach wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modifies the PDF viewing functionality to open PDFs in a new tab using a secure shareId-based URL instead of directly exposing Firebase storage URLs. The implementation adds a new API route that validates the shareId and serves the PDF content while hiding internal identifiers (userId and recordingId) from the client.
Changes:
- Added a new API endpoint
/api/pdf-downloadthat accepts a shareId parameter and serves PDF content after validating access - Updated the PDF viewer component to fetch the PDF via the new API endpoint and open it using a blob URL
- Passed the shareId prop from the Tabs component to PdfViewerNew to enable the secure download flow
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/api/pdf-download/route.ts | New API route that validates shareId and serves PDF content without exposing internal Firebase URLs |
| app/[id]/Tabs.tsx | Passes shareId prop to PdfViewerNew component |
| app/[id]/PdfViewerNew.tsx | Updates handleOpenInNewTab to fetch PDF via the new API endpoint and open using blob URL |
馃挕 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleOpenInNewTab = async () => { | ||
| try { | ||
| const res = await fetch(`/api/pdf-download?shareId=${encodeURIComponent(shareId)}`); | ||
| if (!res.ok) throw new Error('Failed to load PDF'); | ||
| const blob = await res.blob(); | ||
| const blobUrl = URL.createObjectURL(blob); | ||
| window.open(blobUrl, '_blank', 'noopener,noreferrer'); | ||
| } catch (e) { | ||
| console.error('Download PDF error:', e); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The blob URL created with URL.createObjectURL is not being revoked, which can lead to memory leaks. Consider revoking the blob URL after it's no longer needed, or at minimum add a comment explaining why it's intentionally not revoked (e.g., if the new tab needs persistent access to the blob).
Changed download PDF, to open it in a new tab, where the url is a new id (it doesn't show the user the recording id, nor the user id)