fix(file-parsers): stop deleting non-BMP characters when sanitizing parsed text (#6442)

* fix(file-parsers): stop deleting non-BMP characters when sanitizing parsed text

The unpaired-surrogate strip used a bare [\uD800-\uDFFF] class, which matches
UTF-16 code units and so removed both halves of every valid surrogate pair —
deleting all emoji, CJK Extension B, and mathematical alphanumerics from parsed
output. Match only genuinely unpaired surrogates instead.

Also surface PDF truncation inline. Callers read only `content`, so a bounded
PDF was indistinguishable from a complete one; it now carries the same
`[... ... ...]` marker csv and xlsx already use, via a shared helper.

Drop two unused exports, one of which carried the same surrogate bug.

* fix(file-parsers): keep text-free PDFs empty and count only pages that were read

Gate the truncation notice on the sanitized, trimmed body: a text-free
multi-page PDF collapses to a lone separator, so the previous length check let
a notice turn a document callers treat as empty into one that looks like it
holds content.

Also stop counting a page the budget cut off before it yielded anything, so the
notice no longer reports one more page than was shown.

Author: waleedlatif1

apps/sim/lib/file-parsers/csv-parser.ts

@@ -3,7 +3,7 @@ import { Readable } from 'stream'
 import { createLogger } from '@sim/logger'
 import { type Options, parse } from 'csv-parse'
 import type { FileParseResult, FileParser } from '@/lib/file-parsers/types'
-import { sanitizeTextForUTF8 } from '@/lib/file-parsers/utils'
+import { sanitizeTextForUTF8, truncationNotice } from '@/lib/file-parsers/utils'
 
 const logger = createLogger('CsvParser')
 
@@ -123,7 +123,9 @@ export class CsvParser implements FileParser {
       parser.on('end', () => {
         if (!aborted) {
           if (rowCount > CONFIG.MAX_PREVIEW_ROWS) {
-            processedContent += `\n[... ${rowCount.toLocaleString()} total rows, showing first ${CONFIG.MAX_PREVIEW_ROWS} ...]\n`
+            processedContent += truncationNotice(
+              `${rowCount.toLocaleString()} total rows, showing first ${CONFIG.MAX_PREVIEW_ROWS}`
+            )
           }
 
           logger.info(`CSV parsing complete: ${rowCount} rows, ${errorCount} errors`)

apps/sim/lib/file-parsers/pdf-parser.test.ts

@@ -33,6 +33,24 @@ function buildTextBombPdf(repeats: number): Buffer {
     Buffer.from('<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>'),
   ]
 
+  return assemblePdf(objects)
+}
+
+/** Builds a PDF whose pages carry no content stream, so nothing is extractable. */
+function buildTextFreePdf(pageCount: number): Buffer {
+  const pageIds = Array.from({ length: pageCount }, (_, i) => 3 + i)
+
+  return assemblePdf([
+    Buffer.from('<< /Type /Catalog /Pages 2 0 R >>'),
+    Buffer.from(
+      `<< /Type /Pages /Kids [${pageIds.map((id) => `${id} 0 R`).join(' ')}] /Count ${pageCount} >>`
+    ),
+    ...pageIds.map(() => Buffer.from('<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] >>')),
+  ])
+}
+
+/** Serializes numbered objects into a PDF with a matching xref table and trailer. */
+function assemblePdf(objects: Buffer[]): Buffer {
   const chunks: Buffer[] = [Buffer.from('%PDF-1.4\n')]
   const offsets: number[] = []
   let offset = chunks[0].length
@@ -70,7 +88,13 @@ describe('PdfParser', () => {
 
     expect(result.metadata?.truncated).toBe(true)
     expect(result.metadata?.warning).toMatch(/parser limit/i)
-    expect(result.content.length).toBeLessThanOrEqual(MAX_PDF_TEXT_CHARS)
+    expect(result.content.length).toBeLessThanOrEqual(MAX_PDF_TEXT_CHARS + 200)
+  }, 120_000)
+
+  it('marks truncated content inline so callers reading only content can see it', async () => {
+    const result = await new PdfParser().parseBuffer(buildTextBombPdf(200_000))
+
+    expect(result.content).toMatch(/\[\.\.\. PDF text truncated at parser limits.* \.\.\.\]/)
   }, 120_000)
 
   it('extracts a small PDF in full and does not flag it as truncated', async () => {
@@ -80,5 +104,13 @@ describe('PdfParser', () => {
     expect(result.metadata?.warning).toBeUndefined()
     expect(result.metadata?.pageCount).toBe(1)
     expect(result.content).toContain('AAAA')
+    expect(result.content).not.toContain('truncated')
+  }, 30_000)
+
+  it('reports a multi-page PDF with no extractable text as empty', async () => {
+    const result = await new PdfParser().parseBuffer(buildTextFreePdf(3))
+
+    expect(result.content.trim()).toBe('')
+    expect(result.content).not.toContain('[...')
   }, 30_000)
 })

apps/sim/lib/file-parsers/pdf-parser.ts

@@ -1,11 +1,15 @@
 import { readFile } from 'fs/promises'
 import { createLogger } from '@sim/logger'
 import type { FileParseResult, FileParser } from '@/lib/file-parsers/types'
-import { sanitizeTextForUTF8 } from '@/lib/file-parsers/utils'
+import { sanitizeTextForUTF8, truncationNotice } from '@/lib/file-parsers/utils'
 
 const logger = createLogger('PdfParser')
 
-/** Highest page number visited, bounding documents that declare huge page counts. */
+/**
+ * Ceiling on the page loop. The character budget and the deadline already stop
+ * extraction on their own, so this exists purely so the loop bound never comes
+ * straight from the attacker-controlled `numPages` field.
+ */
 const MAX_PDF_PAGES = 10_000
 
 /** Ceiling on extracted characters — roughly 3,000 pages of dense text. */
@@ -35,6 +39,8 @@ interface BoundedExtraction {
   text: string
   /** Page count the document declares, however many pages were actually read. */
   totalPages: number
+  /** Pages actually visited before a budget stopped extraction. */
+  pagesRead: number
   /** True when a budget stopped extraction before the document was exhausted. */
   truncated: boolean
 }
@@ -119,7 +125,12 @@ async function extractTextWithinBudget(pdf: PdfDocumentProxy): Promise<BoundedEx
     const { text, used, completed } = await readPageWithinBudget(page, remainingChars, deadline)
 
     remainingChars -= used
-    pageTexts.push(text)
+
+    // A page the budget cut off before it yielded anything was never really
+    // read, so it must not count toward `pagesRead` or add a blank separator.
+    if (completed || text.length > 0) {
+      pageTexts.push(text)
+    }
     page.cleanup()
 
     if (!completed) {
@@ -128,7 +139,12 @@ async function extractTextWithinBudget(pdf: PdfDocumentProxy): Promise<BoundedEx
     }
   }
 
-  return { text: pageTexts.join('\n').replace(/\s+/g, ' '), totalPages, truncated }
+  return {
+    text: pageTexts.join('\n').replace(/\s+/g, ' '),
+    totalPages,
+    pagesRead: pageTexts.length,
+    truncated,
+  }
 }
 
 export class PdfParser implements FileParser {
@@ -162,16 +178,30 @@ export class PdfParser implements FileParser {
       const pdf = await getDocumentProxy(uint8Array)
 
       try {
-        const { text, totalPages, truncated } = await extractTextWithinBudget(pdf)
+        const { text, totalPages, pagesRead, truncated } = await extractTextWithinBudget(pdf)
 
         logger.info('PDF parsed successfully, pages:', totalPages, 'text length:', text.length)
 
         if (truncated) {
-          logger.warn(PDF_TRUNCATION_WARNING, { totalPages, textLength: text.length })
+          logger.warn(PDF_TRUNCATION_WARNING, { totalPages, pagesRead, textLength: text.length })
         }
 
+        const body = sanitizeTextForUTF8(text)
+
+        // Callers only ever read `content`, so without an inline notice a truncated
+        // document is indistinguishable from a complete one. Tested after sanitizing
+        // and against `trim`, because a text-free multi-page PDF collapses to a lone
+        // separator — appending a notice to that would turn a document callers treat
+        // as empty into one that looks like it holds content.
+        const notice =
+          truncated && body.trim().length > 0
+            ? truncationNotice(
+                `PDF text truncated at parser limits, showing first ${pagesRead} of ${totalPages} pages`
+              )
+            : ''
+
         return {
-          content: sanitizeTextForUTF8(text),
+          content: body + notice,
           metadata: {
             pageCount: totalPages,
             source: 'unpdf',

apps/sim/lib/file-parsers/utils.test.ts

@@ -0,0 +1,54 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import { sanitizeTextForUTF8, truncationNotice } from '@/lib/file-parsers/utils'
+
+const LONE_HIGH = '\uD800'
+const LONE_LOW = '\uDC00'
+
+describe('sanitizeTextForUTF8', () => {
+  it('preserves non-BMP characters built from valid surrogate pairs', () => {
+    const text = 'emoji 😀 cjk-ext-b 𠮷野家 math 𝐀𝐁𝐂'
+
+    expect(sanitizeTextForUTF8(text)).toBe(text)
+  })
+
+  it('removes unpaired surrogates', () => {
+    expect(sanitizeTextForUTF8(`a${LONE_HIGH}b`)).toBe('ab')
+    expect(sanitizeTextForUTF8(`a${LONE_LOW}b`)).toBe('ab')
+  })
+
+  it('removes an unpaired surrogate without disturbing an adjacent valid pair', () => {
+    expect(sanitizeTextForUTF8(`😀${LONE_HIGH}😀`)).toBe('😀😀')
+  })
+
+  it('round-trips through UTF-8 after sanitizing', () => {
+    const sanitized = sanitizeTextForUTF8(`😀${LONE_LOW}𠮷`)
+
+    expect(Buffer.from(sanitized, 'utf8').toString('utf8')).toBe(sanitized)
+    expect(sanitized).toBe('😀𠮷')
+  })
+
+  it('removes control characters but keeps tab, newline, and carriage return', () => {
+    expect(sanitizeTextForUTF8('a\x07b\x7Fc')).toBe('abc')
+    expect(sanitizeTextForUTF8('a\tb\nc\rd')).toBe('a\tb\nc\rd')
+  })
+
+  it('removes null bytes and replacement characters', () => {
+    expect(sanitizeTextForUTF8('a\x00b\uFFFDc')).toBe('abc')
+  })
+
+  it('returns an empty string for empty or non-string input', () => {
+    expect(sanitizeTextForUTF8('')).toBe('')
+    expect(sanitizeTextForUTF8(undefined as unknown as string)).toBe('')
+  })
+})
+
+describe('truncationNotice', () => {
+  it('wraps the detail in the shared inline marker', () => {
+    expect(truncationNotice('42 total rows, showing first 10')).toBe(
+      '\n[... 42 total rows, showing first 10 ...]\n'
+    )
+  })
+})

apps/sim/lib/file-parsers/utils.ts

@@ -1,42 +1,26 @@
 /**
- * Utility functions for file parsing
+ * A bare `[\uD800-\uDFFF]` class would match both halves of a *valid* pair,
+ * deleting every non-BMP character rather than only the malformed ones.
  */
+const UNPAIRED_SURROGATE = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/g
 
 /**
- * Clean text content to ensure it's safe for UTF-8 storage in PostgreSQL
- * Removes null bytes and control characters that can cause encoding errors
+ * Strips control characters, replacement characters, and unpaired surrogates so
+ * the text is safe for UTF-8 storage in PostgreSQL. Tabs, newlines, and carriage
+ * returns are preserved.
  */
 export function sanitizeTextForUTF8(text: string): string {
   if (!text || typeof text !== 'string') {
     return ''
   }
 
   return text
-    .replace(/\0/g, '') // Remove null bytes (0x00)
-    .replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, '') // Remove control characters except \t(0x09), \n(0x0A), \r(0x0D)
-    .replace(/\uFFFD/g, '') // Remove Unicode replacement character
-    .replace(/[\uD800-\uDFFF]/g, '') // Remove unpaired surrogate characters
+    .replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, '')
+    .replace(/\uFFFD/g, '')
+    .replace(UNPAIRED_SURROGATE, '')
 }
 
-/**
- * Sanitize an array of strings
- */
-export function sanitizeTextArray(texts: string[]): string[] {
-  return texts.map((text) => sanitizeTextForUTF8(text))
-}
-
-/**
- * Check if a string contains problematic characters for UTF-8 storage
- */
-export function hasInvalidUTF8Characters(text: string): boolean {
-  if (!text || typeof text !== 'string') {
-    return false
-  }
-
-  // Check for null bytes and control characters
-  return (
-    /[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/.test(text) ||
-    /\uFFFD/.test(text) ||
-    /[\uD800-\uDFFF]/.test(text)
-  )
+/** Formats the inline `[... detail ...]` marker parsers append when a limit stopped extraction early. */
+export function truncationNotice(detail: string): string {
+  return `\n[... ${detail} ...]\n`
 }

apps/sim/lib/file-parsers/xlsx-parser.ts

@@ -4,7 +4,7 @@ import { createLogger } from '@sim/logger'
 import { truncate } from '@sim/utils/string'
 import * as XLSX from 'xlsx'
 import type { FileParseResult, FileParser } from '@/lib/file-parsers/types'
-import { sanitizeTextForUTF8 } from '@/lib/file-parsers/utils'
+import { sanitizeTextForUTF8, truncationNotice } from '@/lib/file-parsers/utils'
 import { assertOoxmlArchiveWithinLimits } from '@/lib/file-parsers/zip-guard'
 
 const logger = createLogger('XlsxParser')
@@ -155,19 +155,18 @@ export class XlsxParser implements FileParser {
           contentSize += chunkContent.length
         }
 
-        // Add truncation notice if needed
         if (actualRowCount > rowsToProcess) {
-          const notice = `\n[... ${actualRowCount.toLocaleString()} total rows, showing first ${rowsToProcess.toLocaleString()} ...]\n`
-          content += notice
+          content += truncationNotice(
+            `${actualRowCount.toLocaleString()} total rows, showing first ${rowsToProcess.toLocaleString()}`
+          )
           truncated = true
         }
       } else {
         content += '[Empty sheet]\n'
       }
 
-      // Stop processing if content is too large
       if (contentSize > CONFIG.MAX_CONTENT_SIZE) {
-        content += '\n[... Content truncated due to size limits ...]\n'
+        content += truncationNotice('Content truncated due to size limits')
         truncated = true
         break
       }