Skip to content

fix: add error handling for base64 decode operations #1237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BYK merged 1 commit into from
Jan 8, 2026
+67 −38
Merged

fix: add error handling for base64 decode operations #1237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions packages/spotlight/src/ui/lib/base64.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,26 @@
export function base64Decode(data: string): Uint8Array {
/**
* Safely decode a base64 string using atob().
* Returns null if decoding fails (invalid base64).
*/
export function safeAtob(data: string): string | null {
try {
return atob(data);
} catch {
// atob throws InvalidCharacterError for invalid base64 strings
return null;
}
}

/**
* Decodes a base64-encoded string to a Uint8Array.
* Returns null if the input is not valid base64.
*/
export function base64Decode(data: string): Uint8Array | null {
// TODO: Use Uint8Array.fromBase64 when it becomes available
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array/fromBase64
return Uint8Array.from(atob(data), c => c.charCodeAt(0));
const decoded = safeAtob(data);
if (decoded === null) {
return null;
}
return Uint8Array.from(decoded, c => c.charCodeAt(0));
}
3 changes: 2 additions & 1 deletion packages/spotlight/src/ui/telemetry/components/insights/QuerySummary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ReactComponent as Sort } from "@spotlight/ui/assets/sort.svg";
import { ReactComponent as SortDown } from "@spotlight/ui/assets/sortDown.svg";
import { safeAtob } from "@spotlight/ui/lib/base64";
import { cn } from "@spotlight/ui/lib/cn";
import Breadcrumbs from "@spotlight/ui/ui/breadcrumbs";
import Table from "@spotlight/ui/ui/table";
Expand Down Expand Up @@ -37,7 +38,7 @@ const QuerySummary = () => {
const { sort, toggleSortOrder } = useSort({ defaultSortType: QUERY_SUMMARY_SORT_KEYS.totalTime });

// Ref: https://developer.mozilla.org/en-US/docs/Web/API/Window/atob
const decodedType = type && atob(type);
const decodedType = type ? safeAtob(type) : null;

const filteredDBSpans: Span[] = useMemo(() => {
if (!decodedType) {
Expand Down
77 changes: 42 additions & 35 deletions packages/spotlight/src/ui/telemetry/components/insights/envelopes/Attachment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { EnvelopeItem } from "@sentry/core";
import { ReactComponent as Download } from "@spotlight/ui/assets/download.svg";
import { base64Decode } from "@spotlight/ui/lib/base64";
import { type ReactNode, useCallback, useEffect, useState } from "react";
import { base64Decode, safeAtob } from "@spotlight/ui/lib/base64";
import { type ReactNode, useEffect, useMemo } from "react";
import JsonViewer from "../../shared/JsonViewer";
import { CodeViewer } from "./CodeViewer";
import { inferExtension } from "./contentType";
Expand All @@ -20,53 +20,60 @@ export default function Attachment({
attachment: string;
expanded?: boolean;
}) {
const [downloadUrl, setDownloadUrl] = useState<string | null>(null);
const extension = inferExtension(header.content_type as string | null, header.type as string | null);
const name = (header.filename as string) || `untitled.${extension}`;

const createDownloadUrl = useCallback(() => {
const blob = new Blob(
[
IMAGE_CONTENT_TYPES.has(header.content_type as string) || VIDEO_CONTENT_TYPES.has(header.content_type as string)
? (base64Decode(attachment).buffer as BlobPart)
: extension === "bin"
? atob(attachment)
: attachment,
],
{ type: (header.content_type as string) || "application/octet-stream" },
);
const url = URL.createObjectURL(blob);
setDownloadUrl(current => {
if (current) {
URL.revokeObjectURL(current);
}
return url;
});
return url;
}, [attachment, extension, header.content_type]);

useEffect(() => {
// Create download URL for binary content types
// Returns: string (success), null (decode error)
const downloadUrl = useMemo(() => {
if (!expanded) {
return;
return undefined; // Not needed yet
}
if (!downloadUrl) {
createDownloadUrl();

const contentType = header.content_type as string;
let blobData: BlobPart;

if (IMAGE_CONTENT_TYPES.has(contentType) || VIDEO_CONTENT_TYPES.has(contentType)) {
const decoded = base64Decode(attachment);
if (!decoded) {
return null; // Decode error
}
blobData = decoded.buffer as BlobPart;
} else if (extension === "bin") {
const decoded = safeAtob(attachment);
if (decoded === null) {
return null; // Decode error
}
blobData = decoded;
} else {
return undefined; // Not a binary type, no blob URL needed
Comment on lines +48 to +49
Copy link

@sentry sentry bot Jan 8, 2026

Choose a reason for hiding this comment

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

Bug: The refactoring to useMemo for downloadUrl removed the fallback for text-based attachments, causing the download link to become non-functional for types like .txt, .csv, and .json.
Severity: CRITICAL

🔍 Detailed Analysis

The refactoring of the download URL generation logic within the Attachment component has introduced a regression. Previously, a Blob was created for text-based attachments using the raw attachment string. The new implementation inside useMemo handles image, video, and binary content types but returns undefined for all other types, including common text formats like text/plain, text/csv, and text/json. As a result, the downloadUrl variable becomes undefined for these attachments, rendering the download link inoperable and preventing users from downloading these files.

💡 Suggested Fix

Restore the logic to handle text-based attachments. Inside the useMemo hook, add a final else block that creates a Blob from the raw attachment string for text content types and returns its URL via URL.createObjectURL, similar to the old implementation. This will ensure download links are correctly generated for all supported attachment types.

🤖 Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
packages/spotlight/src/ui/telemetry/components/insights/envelopes/Attachment.tsx#L48-L49

Potential issue: The refactoring of the download URL generation logic within the
`Attachment` component has introduced a regression. Previously, a `Blob` was created for
text-based attachments using the raw attachment string. The new implementation inside
`useMemo` handles image, video, and binary content types but returns `undefined` for all
other types, including common text formats like `text/plain`, `text/csv`, and
`text/json`. As a result, the `downloadUrl` variable becomes `undefined` for these
attachments, rendering the download link inoperable and preventing users from
downloading these files.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8364912

Copy link

@cursor cursor bot Jan 8, 2026

Choose a reason for hiding this comment

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

Download broken for text-based attachment content types

High Severity

The refactored downloadUrl logic only creates blob URLs for image, video, and "bin" extension types. For all other content types like text/plain, text/csv, JSON, and code files, downloadUrl is set to undefined. The old code created a blob URL for ALL content types by using attachment directly for text-based types. This breaks the download link (href={downloadUrl ?? undefined}) for non-binary attachments, as the href will be undefined and clicking it won't trigger a download. The PR description states users should "still allowing raw envelope download" but this functionality is now broken for text-based content types.

🔬 Verification Test

Why verification test was not possible: This is a React component that requires a full browser/DOM environment to test the blob URL creation and download functionality. The bug is evident from code analysis - the old code had a fallback branch that used attachment directly for non-binary types to create blob URLs, while the new code returns undefined for these cases, leaving the download <a> tag without a valid href.

Fix in Cursor Fix in Web

}
}, [expanded, downloadUrl, createDownloadUrl]);

useEffect(
() => () => {
const blob = new Blob([blobData], { type: contentType || "application/octet-stream" });
return URL.createObjectURL(blob);
}, [expanded, attachment, extension, header.content_type]);

// Cleanup blob URL on unmount or when URL changes
useEffect(() => {
return () => {
if (downloadUrl) {
URL.revokeObjectURL(downloadUrl);
}
},
[downloadUrl],
);
};
}, [downloadUrl]);

const decodeError = downloadUrl === null;

let content: ReactNode = null;

if (expanded) {
if (header.content_type === "text/plain" || header.content_type === "text/csv") {
if (decodeError) {
content = (
<pre className="text-destructive-400 whitespace-pre-wrap break-words font-mono text-sm rounded-sm bg-primary-900 p-2">
Failed to decode attachment data. The base64 data may be corrupted or invalid.
</pre>
);
} else if (header.content_type === "text/plain" || header.content_type === "text/csv") {
content = (
<pre className="text-primary-300 whitespace-pre-wrap break-words font-mono text-sm rounded-sm bg-primary-900 p-2">
{attachment}
Expand Down
Loading