Improve logging in VoiceNotesPlaybackContext

Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com>

ts/components/VoiceNotesPlaybackContext.tsx

@@ -7,6 +7,7 @@ import LRU from 'lru-cache';
 
 import type { WaveformCache } from '../types/Audio';
 import * as log from '../logging/log';
+import { redactAttachmentUrl } from '../util/privacy';
 
 const MAX_WAVEFORM_COUNT = 1000;
 const MAX_PARALLEL_COMPUTE = 8;
@@ -43,7 +44,7 @@ async function getAudioDuration(
 ): Promise<number> {
   const blob = new Blob([buffer]);
   const blobURL = URL.createObjectURL(blob);
-
+  const urlForLogging = redactAttachmentUrl(url);
   const audio = new Audio();
   audio.muted = true;
   audio.src = blobURL;
@@ -55,14 +56,14 @@ async function getAudioDuration(
 
     audio.addEventListener('error', event => {
       const error = new Error(
-        `Failed to load audio from: ${url} due to error: ${event.type}`
+        `Failed to load audio from: ${urlForLogging} due to error: ${event.type}`
       );
       reject(error);
     });
   });
 
   if (Number.isNaN(audio.duration)) {
-    throw new Error(`Invalid audio duration for: ${url}`);
+    throw new Error(`Invalid audio duration for: ${urlForLogging}`);
   }
   return audio.duration;
 }
@@ -83,12 +84,15 @@ async function doComputePeaks(
 ): Promise<ComputePeaksResult> {
   const cacheKey = `${url}:${barCount}`;
   const existing = waveformCache.get(cacheKey);
+  const urlForLogging = redactAttachmentUrl(url);
+
+  const logId = `GlobalAudioContext(${urlForLogging})`;
   if (existing) {
-    log.info('GlobalAudioContext: waveform cache hit', url);
+    log.info(`${logId}: waveform cache hit`);
     return Promise.resolve(existing);
   }
 
-  log.info('GlobalAudioContext: waveform cache miss', url);
+  log.info(`${logId}: waveform cache miss`);
 
   // Load and decode `url` into a raw PCM
   const response = await fetch(url);
@@ -98,9 +102,7 @@ async function doComputePeaks(
 
   const peaks = new Array(barCount).fill(0);
   if (duration > MAX_AUDIO_DURATION) {
-    log.info(
+    log.info(`${logId}: duration ${duration}s is too long`);
-      `GlobalAudioContext: audio ${url} duration ${duration}s is too long`
-    );
     const emptyResult = { peaks, duration };
     waveformCache.set(cacheKey, emptyResult);
     return emptyResult;
@@ -153,17 +155,15 @@ export async function computePeaks(
   barCount: number
 ): Promise<ComputePeaksResult> {
   const computeKey = `${url}:${barCount}`;
+  const logId = `VoiceNotesPlaybackContext(${redactAttachmentUrl(url)})`;
 
   const pending = inProgressMap.get(computeKey);
   if (pending) {
-    log.info(
+    log.info(`${logId}: already computing peaks`);
-      'VoiceNotesPlaybackContext: already computing peaks for',
-      computeKey
-    );
     return pending;
   }
 
-  log.info('VoiceNotesPlaybackContext: queue computing peaks for', computeKey);
+  log.info(`${logId}: queueing computing peaks`);
   const promise = computeQueue.add(() => doComputePeaks(url, barCount));
 
   inProgressMap.set(computeKey, promise);

ts/test-both/util/privacy_test.ts

@@ -103,6 +103,30 @@ describe('Privacy', () => {
     });
   });
 
+  describe('redactAttachmentUrlKeys', () => {
+    it('should redact key= values ', () => {
+      const text =
+        'Log line with url attachment://v2/e6/abcdee64?key=hxKJ9cTfK0v3KEsnzJ2j%2F4Crwe0yu&size=39360&contentType=png ' +
+        'and another already partially redacted attachment://v2/e6/[REDACTED]?LOCALKEY=hxKJ9cTfK0v3KEsnzJ2j%2F4Crwe0yu&size=39360&contentType=png';
+
+      const actual = Privacy.redactAttachmentUrlKeys(text);
+      const expected =
+        'Log line with url attachment://v2/e6/abcdee64?key=[REDACTED] ' +
+        'and another already partially redacted attachment://v2/e6/[REDACTED]?LOCALKEY=[REDACTED]';
+      assert.equal(actual, expected);
+    });
+  });
+
+  describe('redactAttachmentUrl', () => {
+    it('should remove search params ', () => {
+      const url =
+        'attachment://v2/e6/abcdee64?key=hxKJ9cTfK0v3KEsnzJ2j%2F4Crwe0yu&size=39360&contentType=png';
+      const actual = Privacy.redactAttachmentUrl(url);
+      const expected = 'attachment://v2/e6/abcdee64';
+      assert.equal(actual, expected);
+    });
+  });
+
   describe('redactAll', () => {
     it('should redact all sensitive information', () => {
       const encodedAppRootPath = APP_ROOT_PATH.replace(/ /g, '%20');
@@ -114,7 +138,8 @@ describe('Privacy', () => {
         `path2 file:///${encodedAppRootPath}/js/background.js.` +
         'phone2 +13334445566 lorem\n' +
         'group2 group(abcdefghij) doloret\n' +
-        'path3 sensitive-path/attachment.noindex\n';
+        'path3 sensitive-path/attachment.noindex\n' +
+        'attachment://v2/ab/abcde?key=specialkey\n';
 
       const actual = Privacy.redactAll(text);
       const expected =
@@ -125,7 +150,8 @@ describe('Privacy', () => {
         'path2 [REDACTED]/js/background.js.' +
         'phone2 +[REDACTED]566 lorem\n' +
         'group2 group([REDACTED]hij) doloret\n' +
-        'path3 [REDACTED]/attachment.noindex\n';
+        'path3 [REDACTED]/attachment.noindex\n' +
+        'attachment://v2/ab/abcde?key=[REDACTED]\n';
       assert.equal(actual, expected);
     });
   });

ts/util/privacy.ts

@@ -22,6 +22,7 @@ const GROUP_V2_ID_PATTERN = /(groupv2\()([^=)]+)(=?=?\))/g;
 const CALL_LINK_ROOM_ID_PATTERN = /[0-9A-F]{61}([0-9A-F]{3})/gi;
 const CALL_LINK_ROOT_KEY_PATTERN =
   /([A-Z]{4})-[A-Z]{4}-[A-Z]{4}-[A-Z]{4}-[A-Z]{4}-[A-Z]{4}-[A-Z]{4}-[A-Z]{4}/gi;
+const ATTACHMENT_URL_KEY_PATTERN = /(attachment:\/\/[^\s]+key=)([^\s]+)/gi;
 const REDACTION_PLACEHOLDER = '[REDACTED]';
 
 export type RedactFunction = (value: string) => string;
@@ -163,6 +164,14 @@ export const redactCallLinkRootKeys = (text: string): string => {
   return text.replace(CALL_LINK_ROOT_KEY_PATTERN, `${REDACTION_PLACEHOLDER}$1`);
 };
 
+export const redactAttachmentUrlKeys = (text: string): string => {
+  if (!isString(text)) {
+    throw new TypeError("'text' must be a string");
+  }
+
+  return text.replace(ATTACHMENT_URL_KEY_PATTERN, `$1${REDACTION_PLACEHOLDER}`);
+};
+
 export const redactCdnKey = (cdnKey: string): string => {
   return `${REDACTION_PLACEHOLDER}${cdnKey.slice(-3)}`;
 };
@@ -171,6 +180,16 @@ export const redactGenericText = (text: string): string => {
   return `${REDACTION_PLACEHOLDER}${text.slice(-3)}`;
 };
 
+export const redactAttachmentUrl = (urlString: string): string => {
+  try {
+    const url = new URL(urlString);
+    url.search = '';
+    return url.toString();
+  } catch {
+    return REDACTION_PLACEHOLDER;
+  }
+};
+
 const createRedactSensitivePaths = (
   paths: ReadonlyArray<string>
 ): RedactFunction => {
@@ -194,7 +213,8 @@ export const redactAll: RedactFunction = compose(
   redactPhoneNumbers,
   redactUuids,
   redactCallLinkRoomIds,
-  redactCallLinkRootKeys
+  redactCallLinkRootKeys,
+  redactAttachmentUrlKeys
 );
 
 const removeNewlines: RedactFunction = text => text.replace(/\r?\n|\r/g, '');