fix(manifest): check upload-encoded storage keys

scripts/backfill_manifest_file_sizes.mjs

@@ -104,6 +104,7 @@ const FAILED_CSV_HEADERS = [
   'file_name',
   's3_path',
   'attempted_s3_path',
+  'attempted_s3_paths',
   'status',
   'method',
   'reason',
@@ -239,34 +240,47 @@ function decodeStoragePathSegments(s3Path) {
   }
 }
 
-function getLegacyDecodedStoragePath(s3Path) {
-  if (!PERCENT_ENCODED_OCTET_RE.test(s3Path))
-    return null
+function encodeStoragePathSegments(s3Path) {
+  return s3Path.split('/').map(segment => encodeURIComponent(segment)).join('/')
+}
+
+function getStorageCandidatePaths(s3Path) {
+  const candidates = [s3Path]
+  if (PERCENT_ENCODED_OCTET_RE.test(s3Path)) {
+    const decoded = decodeStoragePathSegments(s3Path)
+    if (decoded && decoded !== s3Path)
+      candidates.push(decoded)
 
-  const decoded = decodeStoragePathSegments(s3Path)
-  return decoded && decoded !== s3Path ? decoded : null
+    const encoded = encodeStoragePathSegments(s3Path)
+    if (encoded !== s3Path)
+      candidates.push(encoded)
+  }
+  return [...new Set(candidates)]
 }
 
-function markDecodedStorageResult(result, attemptedS3Path) {
+function markStorageCandidateResult(result, attemptedS3Path, attemptedS3Paths) {
+  const suffix = attemptedS3Path === attemptedS3Paths[0] ? '' : '_candidate'
   return {
     ...result,
     attempted_s3_path: attemptedS3Path,
-    method: `${result.method ?? 'unknown'}_decoded`,
-    reason: result.reason ? `${result.reason}_decoded_path` : 'decoded_path',
+    attempted_s3_paths: attemptedS3Paths,
+    method: `${result.method ?? 'unknown'}${suffix}`,
+    reason: suffix && result.reason ? `${result.reason}${suffix}` : result.reason,
   }
 }
 
 async function getObjectSizeWithLegacyFallback(s3, s3Path) {
-  const primary = await getObjectSize(s3, s3Path)
-  if (primary.size > 0)
-    return primary
+  const candidatePaths = getStorageCandidatePaths(s3Path)
+  let lastResult = null
 
-  const fallbackPath = primary.status === 404 ? getLegacyDecodedStoragePath(s3Path) : null
-  if (!fallbackPath)
-    return primary
+  for (const candidatePath of candidatePaths) {
+    const result = markStorageCandidateResult(await getObjectSize(s3, candidatePath), candidatePath, candidatePaths)
+    lastResult = result
+    if (result.size > 0 || shouldRetryStorageResult(result))
+      return result
+  }
 
-  const fallback = await getObjectSize(s3, fallbackPath)
-  return markDecodedStorageResult(fallback, fallbackPath)
+  return lastResult
 }
 
 async function getObjectSize(s3, s3Path) {
@@ -564,6 +578,7 @@ async function processCandidates({ apply, dbAttempts, pool, s3, storageAttempts,
       app_id: result.row.app_id,
       app_version_id: result.row.app_version_id,
       attempted_s3_path: result.storage.attempted_s3_path,
+      attempted_s3_paths: result.storage.attempted_s3_paths,
       attempts: result.storage.attempts,
       error: result.storage.error,
       file_name: result.row.file_name,

supabase/functions/_backend/files/util.ts

@@ -72,10 +72,16 @@ export function encodeR2KeyForUploadLocation(r2Key: string): string {
 }
 
 export function getAttachmentReadCandidateKeys(decodedKey: string, rawRouteKey: string | null | undefined): string[] {
-  if (rawRouteKey && rawRouteKey !== decodedKey)
-    return [decodedKey, rawRouteKey]
+  const candidates = [decodedKey]
+  if (rawRouteKey && rawRouteKey !== decodedKey) {
+    candidates.push(rawRouteKey)
 
-  return [decodedKey]
+    const encodedRawRouteKey = encodeR2KeyForUploadLocation(rawRouteKey)
+    if (encodedRawRouteKey !== rawRouteKey && encodedRawRouteKey !== decodedKey)
+      candidates.push(encodedRawRouteKey)
+  }
+
+  return [...new Set(candidates)]
 }
 
 export function parseAppScopedAttachmentPath(fileId: unknown): AppScopedAttachmentPath | null {

supabase/functions/_backend/utils/manifest_encoding.ts

@@ -4,7 +4,7 @@ export function encodeManifestPathSegments(path: string): string {
   return path.split('/').map(segment => encodeURIComponent(segment)).join('/')
 }
 
-function decodeManifestPathSegments(path: string): string | null {
+export function decodeManifestPathSegments(path: string): string | null {
   try {
     return path.split('/').map(segment => decodeURIComponent(segment)).join('/')
   }
@@ -36,3 +36,19 @@ export function normalizeLegacyEncodedManifestFileName(fileName: string | null |
 
   return decodedFileName
 }
+
+export function getManifestStorageCandidateKeys(s3Path: string): string[] {
+  const candidates = [s3Path]
+
+  if (PERCENT_ENCODED_OCTET_RE.test(s3Path)) {
+    const decodedPath = decodeManifestPathSegments(s3Path)
+    if (decodedPath && decodedPath !== s3Path)
+      candidates.push(decodedPath)
+
+    const encodedPath = encodeManifestPathSegments(s3Path)
+    if (encodedPath !== s3Path)
+      candidates.push(encodedPath)
+  }
+
+  return [...new Set(candidates)]
+}

supabase/functions/_backend/utils/s3.ts

@@ -2,6 +2,7 @@ import type { Context } from 'hono'
 import type { Database } from '../utils/supabase.types.ts'
 import { S3Client } from '@bradenmacdonald/s3-lite-client'
 import { cloudlog, cloudlogErr, serializeError } from './logging.ts'
+import { getManifestStorageCandidateKeys } from './manifest_encoding.ts'
 import { getEnv } from './utils.ts'
 
 function firstForwardedHeaderValue(value: string | undefined): string | undefined {
@@ -312,8 +313,7 @@ async function getSizeFromRangeFallback(
   }
 }
 
-async function getSize(c: Context, fileId: string) {
-  const client = initS3(c)
+async function getSizeForKey(c: Context, client: ReturnType<typeof initS3>, fileId: string) {
   let size = 0
   let headError: unknown
   let usedFallback = false
@@ -362,6 +362,41 @@ async function getSize(c: Context, fileId: string) {
   return size
 }
 
+async function getSize(c: Context, fileId: string) {
+  const client = initS3(c)
+  const candidateKeys = getManifestStorageCandidateKeys(fileId)
+
+  for (const candidateKey of candidateKeys) {
+    const size = await getSizeForKey(c, client, candidateKey)
+    if (size > 0) {
+      if (candidateKey !== fileId) {
+        cloudlog({
+          requestId: c.get('requestId'),
+          message: 'getSize recovered from manifest storage candidate',
+          fileId,
+          candidateKey,
+          candidateKeys,
+          size,
+        })
+      }
+      return size
+    }
+  }
+
+  if (candidateKeys.length > 1) {
+    cloudlogErr({
+      requestId: c.get('requestId'),
+      message: 'getSize failed all manifest storage candidates',
+      fileId,
+      candidateKeys,
+      bucket: getEnv(c, 'S3_BUCKET'),
+      endpoint: getEnv(c, 'S3_ENDPOINT'),
+    })
+  }
+
+  return 0
+}
+
 async function getObject(c: Context, fileId: string): Promise<Response | null> {
   const client = initS3(c)
   try {

tests/manifest-path-encoding.unit.test.ts

@@ -1,7 +1,7 @@
 import { describe, expect, it } from 'vitest'
 
 import { getManifestUrl } from '../supabase/functions/_backend/utils/downloadUrl.ts'
-import { normalizeLegacyEncodedManifestFileName } from '../supabase/functions/_backend/utils/manifest_encoding.ts'
+import { getManifestStorageCandidateKeys, normalizeLegacyEncodedManifestFileName } from '../supabase/functions/_backend/utils/manifest_encoding.ts'
 
 describe('manifest path encoding', () => {
   it.concurrent.each([
@@ -54,4 +54,14 @@ describe('manifest path encoding', () => {
       'orgs/org-id/apps/com.test.app/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey%25402x.png',
     )).toBe('assets/suite-marketing/images/social-media/sad_post_grey%402x.png')
   })
+
+  it.concurrent('checks legacy percent, decoded, and upload-location encoded storage keys', () => {
+    expect(getManifestStorageCandidateKeys(
+      'orgs/org-id/apps/com.test.app/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey%402x.png',
+    )).toEqual([
+      'orgs/org-id/apps/com.test.app/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey%402x.png',
+      'orgs/org-id/apps/com.test.app/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey@2x.png',
+      'orgs/org-id/apps/com.test.app/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey%25402x.png',
+    ])
+  })
 })

tests/upload-path-encoding.unit.test.ts

@@ -16,6 +16,7 @@ describe('upload path encoding', () => {
     )).toEqual([
       'orgs/org-id/apps/app-id/delta/hash_sad_post_grey@2x.png',
       'orgs/org-id/apps/app-id/delta/hash_sad_post_grey%402x.png',
+      'orgs/org-id/apps/app-id/delta/hash_sad_post_grey%25402x.png',
     ])
   })
 
@@ -28,24 +29,26 @@ describe('upload path encoding', () => {
     expect(getSafeAttachmentReadCandidateKeys(decodedRouteKey, encodedStoragePath)).toEqual([
       decodedRouteKey,
       encodedStoragePath,
+      'orgs/org-id/apps/app-id/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey%25402x.png',
     ])
   })
 
-  it.concurrent('heads legacy raw percent candidates when decoded object keys are missing', async () => {
+  it.concurrent('heads upload-location encoded candidates when decoded and raw percent object keys are missing', async () => {
     const checkedKeys: string[] = []
     const decodedRouteKey = 'orgs/org-id/apps/app-id/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey@2x.png'
     const encodedStoragePath = 'orgs/org-id/apps/app-id/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey%402x.png'
+    const uploadLocationEncodedStoragePath = 'orgs/org-id/apps/app-id/delta/hash_assets/suite-marketing/images/social-media/sad_post_grey%25402x.png'
     const legacyObjectInfo = { size: 42 }
 
     const objectInfo = await headFirstExistingAttachmentCandidate({
       async head(key: string) {
         checkedKeys.push(key)
-        return key === encodedStoragePath ? legacyObjectInfo : null
+        return key === uploadLocationEncodedStoragePath ? legacyObjectInfo : null
       },
     }, getSafeAttachmentReadCandidateKeys(decodedRouteKey, encodedStoragePath))
 
     expect(objectInfo).toBe(legacyObjectInfo)
-    expect(checkedKeys).toEqual([decodedRouteKey, encodedStoragePath])
+    expect(checkedKeys).toEqual([decodedRouteKey, encodedStoragePath, uploadLocationEncodedStoragePath])
   })
 
   it.concurrent('does not try raw percent route keys outside the authorized app scope', () => {