Browse Source

fix(replays): Fix console crashing when using color codes (#35477)

`console.log` accepts a placeholder `%c` to style your console messages. This conflicts with `sprintf-js` library which expects it be an ascii char code. For now we will strip `%c` as I do not want to deal with any potential XSS.

Also adds error boundaries so that the entire page does not crash when a subcomponent throws an error.
Billy Vong 2 years ago
parent
commit
8ae97d07da

+ 16 - 3
static/app/views/replays/detail/console/consoleMessage.tsx

@@ -3,6 +3,7 @@ import styled from '@emotion/styled';
 import {sprintf, vsprintf} from 'sprintf-js';
 
 import DateTime from 'sentry/components/dateTime';
+import ErrorBoundary from 'sentry/components/errorBoundary';
 import AnnotatedText from 'sentry/components/events/meta/annotatedText';
 import {getMeta} from 'sentry/components/events/meta/metaProxy';
 import {useReplayContext} from 'sentry/components/replays/replayContext';
@@ -40,19 +41,29 @@ function MessageFormatter({breadcrumb}: MessageFormatterProps) {
 
   const isMessageString = typeof message === 'string';
 
-  // Assumption here is that Array types are == placeholders, which means that's the number of arguments we need
   const placeholders = isMessageString
     ? sprintf.parse(message).filter(parsed => Array.isArray(parsed))
     : [];
 
+  // TODO `%c` is console specific, it applies colors to messages
+  // for now we are stripping it as this is potentially risky to implement due to xss
+  const consoleColorPlaceholderIndexes = placeholders
+    .filter(([placeholder]) => placeholder === '%c')
+    .map((_, i) => i);
+
   // Retrieve message formatting args
   const messageArgs = args.slice(0, placeholders.length);
 
+  // Filter out args that were for %c
+  for (const colorIndex of consoleColorPlaceholderIndexes) {
+    messageArgs.splice(colorIndex, 1);
+  }
+
   // Attempt to stringify the rest of the args
   const restArgs = args.slice(placeholders.length).map(renderString);
 
   const formattedMessage = isMessageString
-    ? vsprintf(message, messageArgs)
+    ? vsprintf(message.replaceAll('%c', ''), messageArgs)
     : renderString(message);
 
   // TODO(replays): Add better support for AnnotatedText (e.g. we use message
@@ -94,7 +105,9 @@ function ConsoleMessage({
         {ICONS[breadcrumb.level]}
       </Icon>
       <Message isLast={isLast} level={breadcrumb.level}>
-        <MessageFormatter breadcrumb={breadcrumb} />
+        <ErrorBoundary mini>
+          <MessageFormatter breadcrumb={breadcrumb} />
+        </ErrorBoundary>
       </Message>
       <ConsoleTimestamp isLast={isLast} level={breadcrumb.level}>
         <Tooltip title={<DateTime date={breadcrumb.timestamp} seconds />}>

+ 13 - 6
static/app/views/replays/details.tsx

@@ -1,6 +1,7 @@
 import React from 'react';
 import styled from '@emotion/styled';
 
+import ErrorBoundary from 'sentry/components/errorBoundary';
 import DetailedError from 'sentry/components/errors/detailedError';
 import NotFound from 'sentry/components/errors/notFound';
 import {HeaderContainer} from 'sentry/components/events/interfaces/spans/header';
@@ -84,19 +85,25 @@ function ReplayDetails() {
           </Layout.Main>
 
           <Layout.Side>
-            <UserActionsNavigator
-              crumbs={replay?.getRawCrumbs()}
-              event={replay?.getEvent()}
-            />
+            <ErrorBoundary>
+              <UserActionsNavigator
+                crumbs={replay?.getRawCrumbs()}
+                event={replay?.getEvent()}
+              />
+            </ErrorBoundary>
           </Layout.Side>
 
           <StickyMain fullWidth>
-            <ReplayTimeline />
+            <ErrorBoundary>
+              <ReplayTimeline />
+            </ErrorBoundary>
             <FocusTabs />
           </StickyMain>
 
           <StyledLayoutMain fullWidth>
-            <FocusArea replay={replay} />
+            <ErrorBoundary>
+              <FocusArea replay={replay} />
+            </ErrorBoundary>
           </StyledLayoutMain>
         </Layout.Body>
       </DetailLayout>