Fix rekoni temp files clean up (#11000)

* Fix rekoni files clean up Signed-off-by: Artyom Savchenko <armisav@gmail.com> * Fix formatting Signed-off-by: Artyom Savchenko <armisav@gmail.com> * Fix copyright Signed-off-by: Artyom Savchenko <armisav@gmail.com> --------- Signed-off-by: Artyom Savchenko <armisav@gmail.com>
2026-08-17 18:05:42 +02:00 · 2026-08-01 07:35:24 +07:00
parent 9b607d36ea
commit b056667372
5 changed files with 144 additions and 64 deletions
@@ -0,0 +1,53 @@
+//
+// Copyright © 2026 Hardcore Engineering Inc.
+//
+// Licensed under the Eclipse Public License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License. You may
+// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+import { stat } from 'fs/promises'
+import { withTempFile } from '../tempfile'
+
+async function exists (path: string): Promise<boolean> {
+  try {
+    await stat(path)
+    return true
+  } catch {
+    return false
+  }
+}
+
+describe('withTempFile', () => {
+  it('removes the temp dir after a successful run', async () => {
+    let capturedDir = ''
+    const result = await withTempFile('content.txt', Buffer.from('hello'), async (filePath, tempDir) => {
+      capturedDir = tempDir
+      expect(await exists(filePath)).toBe(true)
+      return 'ok'
+    })
+
+    expect(result).toBe('ok')
+    expect(await exists(capturedDir)).toBe(false)
+  })
+
+  it('removes the temp dir even when the run callback throws', async () => {
+    let capturedDir = ''
+
+    await expect(
+      withTempFile('content.txt', Buffer.from('hello'), async (_filePath, tempDir) => {
+        capturedDir = tempDir
+        throw new Error('boom')
+      })
+    ).rejects.toThrow('boom')
+
+    expect(capturedDir).not.toBe('')
+    expect(await exists(capturedDir)).toBe(false)
+  })
+})
@@ -1,8 +1,6 @@
 import { exec } from 'child_process'
-import { mkdtemp, rm, rmdir, writeFile } from 'fs/promises'
 import { contentType } from 'mime-types'
-import { tmpdir } from 'os'
-import { join } from 'path'
+import { withTempFile } from '../tempfile'
 import { rtfExtractor } from './rtf'
 import { DocumentExtractor } from './types'

@@ -18,36 +16,32 @@ export const docExtractor: DocumentExtractor = {
  },

  async extract (fileName: string, type: string, data): Promise<string> {
-    const tempDir = await mkdtemp(join(tmpdir(), 'rekoni-'))
-    const distFileName = join(tempDir, 'content.doc')
-    await writeFile(distFileName, data)
-    const text = await new Promise<string>((resolve, reject) => {
-      exec(
-        `antiword -i 1 -f -m UTF-8 "${distFileName}"`,
-        { encoding: 'utf-8', cwd: tempDir },
-        (error, stdout, stderr) => {
-          if (error != null) {
-            if (stderr.includes('is not a Word Document. It is probably a Rich Text Format file')) {
-              rtfExtractor
-                .extract(fileName, type, data)
-                .then((value) => {
-                  resolve(value)
-                })
-                .catch((err) => {
-                  reject(err)
-                })
-              return
+    return await withTempFile('content.doc', data, async (distFileName, tempDir) => {
+      return await new Promise<string>((resolve, reject) => {
+        exec(
+          `antiword -i 1 -f -m UTF-8 "${distFileName}"`,
+          { encoding: 'utf-8', cwd: tempDir },
+          (error, stdout, stderr) => {
+            if (error != null) {
+              if (stderr.includes('is not a Word Document. It is probably a Rich Text Format file')) {
+                rtfExtractor
+                  .extract(fileName, type, data)
+                  .then((value) => {
+                    resolve(value)
+                  })
+                  .catch((err) => {
+                    reject(err)
+                  })
+                return
+              }
+              reject(new Error(`Error ${JSON.stringify(error)} ${stderr}`))
+            } else {
+              resolve(stdout)
            }
-            reject(new Error(`Error ${JSON.stringify(error)} ${stderr}`))
-          } else {
-            resolve(stdout)
          }
-        }
-      )
+        )
+      })
    })
-    await rm(distFileName)
-    await rmdir(tempDir)
-    return text
  }
 }
 function isType (type: string): boolean {
@@ -1,8 +1,6 @@
 import { exec } from 'child_process'
-import { mkdtemp, rm, rmdir, writeFile } from 'fs/promises'
 import { contentType } from 'mime-types'
-import { tmpdir } from 'os'
-import { join } from 'path'
+import { withTempFile } from '../tempfile'
 import { DocumentExtractor } from './types'

 export const pdfExtractor: DocumentExtractor = {
@@ -22,21 +20,16 @@ export const pdfExtractor: DocumentExtractor = {
  },

  async extract (fileName: string, type: string, data): Promise<string> {
-    const tempDir = await mkdtemp(join(tmpdir(), 'rekoni-'))
-    const distFileName = join(tempDir, 'content.pdf')
-    await writeFile(distFileName, data)
-
-    const text = await new Promise<string>((resolve, reject) => {
-      exec(`pdftotext -layout "${distFileName}" -`, { encoding: 'utf-8', cwd: tempDir }, (error, stdout, stderr) => {
-        if (error != null) {
-          reject(new Error(`Error ${JSON.stringify(error)} ${stderr}`))
-        } else {
-          resolve(stdout)
-        }
+    return await withTempFile('content.pdf', data, async (distFileName, tempDir) => {
+      return await new Promise<string>((resolve, reject) => {
+        exec(`pdftotext -layout "${distFileName}" -`, { encoding: 'utf-8', cwd: tempDir }, (error, stdout, stderr) => {
+          if (error != null) {
+            reject(new Error(`Error ${JSON.stringify(error)} ${stderr}`))
+          } else {
+            resolve(stdout)
+          }
+        })
      })
    })
-    await rm(distFileName)
-    await rmdir(tempDir)
-    return text
  }
 }
@@ -1,8 +1,6 @@
 import { exec } from 'child_process'
-import { mkdtemp, rm, rmdir, writeFile } from 'fs/promises'
 import { contentType } from 'mime-types'
-import { tmpdir } from 'os'
-import { join } from 'path'
+import { withTempFile } from '../tempfile'
 import { convertString } from './html'
 import { DocumentExtractor } from './types'

@@ -18,23 +16,23 @@ export const rtfExtractor: DocumentExtractor = {
  },

  async extract (fileName: string, type: string, data): Promise<string> {
-    const tempDir = await mkdtemp(join(tmpdir(), 'rekoni-'))
-    const distFileName = join(tempDir, 'content.rtf')
-    await writeFile(distFileName, data)
-    const htmlText = await new Promise<string>((resolve, reject) => {
-      exec(`unrtf --nopict --html "${distFileName}"`, { encoding: 'utf-8', cwd: tempDir }, (error, stdout, stderr) => {
-        if (error != null) {
-          reject(new Error(`Error ${JSON.stringify(error)} ${stderr}`))
-        } else {
-          resolve(stdout)
-        }
+    const htmlText = await withTempFile('content.rtf', data, async (distFileName, tempDir) => {
+      return await new Promise<string>((resolve, reject) => {
+        exec(
+          `unrtf --nopict --html "${distFileName}"`,
+          { encoding: 'utf-8', cwd: tempDir },
+          (error, stdout, stderr) => {
+            if (error != null) {
+              reject(new Error(`Error ${JSON.stringify(error)} ${stderr}`))
+            } else {
+              resolve(stdout)
+            }
+          }
+        )
      })
    })

-    const text = convertString(htmlText)
-    await rm(distFileName)
-    await rmdir(tempDir)
-    return text
+    return convertString(htmlText)
  }
 }
 function isType (type: string): boolean {
@@ -0,0 +1,42 @@
+//
+// Copyright © 2026 Hardcore Engineering Inc.
+//
+// Licensed under the Eclipse Public License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License. You may
+// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+import { mkdtemp, rm, writeFile } from 'fs/promises'
+import { tmpdir } from 'os'
+import { join } from 'path'
+
+/**
+ * Writes `data` into a fresh temp directory under a file named `fileName`, runs `run` against it,
+ * and guarantees the whole temp directory is removed afterwards — whether `run` succeeds or throws.
+ *
+ * Extractors shell out to external binaries (pdftotext, antiword, unrtf) that can fail on malformed
+ * or unsupported input. Without a `finally`-guaranteed cleanup, a failed extraction leaks its temp
+ * directory (including the uploaded file content) for the lifetime of the host.
+ *
+ * @public
+ */
+export async function withTempFile<T> (
+  fileName: string,
+  data: Buffer,
+  run: (filePath: string, tempDir: string) => Promise<T>
+): Promise<T> {
+  const tempDir = await mkdtemp(join(tmpdir(), 'rekoni-'))
+  try {
+    const filePath = join(tempDir, fileName)
+    await writeFile(filePath, data)
+    return await run(filePath, tempDir)
+  } finally {
+    await rm(tempDir, { recursive: true, force: true })
+  }
+}