Merge branch 'main' into fix/data-url-encoding-issue-174

Author: rwblair

changelog.d/20250911_171112_markiewicz_file_access_errors.md

@@ -0,0 +1,47 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+For top level release notes, leave all the headers commented out.
+-->
+
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+### Fixed
+
+- File access failures consistently produce `FILE_READ` errors across all file types.
+
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->
+<!--
+### Infrastructure
+
+- A bullet item for the Infrastructure category.
+
+-->

changelog.d/20250915_144507_markiewicz_cache_json.md

@@ -0,0 +1,47 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+For top level release notes, leave all the headers commented out.
+-->
+
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+### Changed
+
+- Parsed JSON files are now cached to reduce I/O and parsing costs.
+
+<!--
+### Fixed
+
+- A bullet item for the Fixed category.
+
+-->
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->
+<!--
+### Infrastructure
+
+- A bullet item for the Infrastructure category.
+
+-->

src/files/access.test.ts

@@ -0,0 +1,40 @@
+import { assert, assertArrayIncludes, assertObjectMatch } from '@std/assert'
+import { basename, dirname } from '@std/path'
+import { BIDSFileDeno } from './deno.ts'
+
+export function testAsyncFileAccess(
+  name: string,
+  fn: (file: BIDSFileDeno, ...args: any[]) => Promise<any>,
+  ...args: any[]
+) {
+  Deno.test({
+    name,
+    ignore: Deno.build.os === 'windows',
+    async fn(t) {
+      await t.step('Dangling symlink', async () => {
+        const file = new BIDSFileDeno('tests/data', '/broken-symlink')
+        try {
+          await fn(file, ...args)
+          assert(false, 'Expected error')
+        } catch (e: any) {
+          assertObjectMatch(e, {
+            code: 'FILE_READ',
+            location: '/broken-symlink',
+          })
+          assertArrayIncludes(['NotFound', 'FilesystemLoop'], [e.subCode])
+        }
+      })
+      await t.step('Insufficient permissions', async () => {
+        const tmpfile = await Deno.makeTempFile()
+        await Deno.chmod(tmpfile, 0o000)
+        const file = new BIDSFileDeno('', tmpfile)
+        try {
+          await fn(file, ...args)
+          assert(false, 'Expected error')
+        } catch (e: any) {
+          assertObjectMatch(e, { code: 'FILE_READ', subCode: 'PermissionDenied' })
+        }
+      })
+    },
+  })
+}

src/files/access.ts

@@ -0,0 +1,35 @@
+import { type BIDSFile } from '../types/filetree.ts'
+import { type Issue } from '../types/issues.ts'
+
+function IOErrorToIssue(err: { code: string; name: string }): Issue {
+  const subcode = err.name
+  let issueMessage: string | undefined = undefined
+  if (err.code === 'ENOENT' || err.code === 'ELOOP') {
+    issueMessage = 'Possible dangling symbolic link'
+  }
+  return { code: 'FILE_READ', subCode: err.name, issueMessage }
+}
+
+export function openStream(file: BIDSFile): ReadableStream<Uint8Array<ArrayBuffer>> {
+  try {
+    return file.stream
+  } catch (err: any) {
+    throw { location: file.path, ...IOErrorToIssue(err) }
+  }
+}
+
+export async function readBytes(
+  file: BIDSFile,
+  size: number,
+  offset = 0,
+): Promise<Uint8Array<ArrayBuffer>> {
+  return file.readBytes(size, offset).catch((err: any) => {
+    throw { location: file.path, ...IOErrorToIssue(err) }
+  })
+}
+
+export async function readText(file: BIDSFile): Promise<string> {
+  return file.text().catch((err: any) => {
+    throw { location: file.path, ...IOErrorToIssue(err) }
+  })
+}

src/files/gzip.test.ts

@@ -1,6 +1,7 @@
 import { assert, assertObjectMatch } from '@std/assert'
 import { parseGzip } from './gzip.ts'
 import { BIDSFileDeno } from './deno.ts'
+import { testAsyncFileAccess } from './access.test.ts'
 
 Deno.test('parseGzip', async (t) => {
   await t.step('parses anonymized file', async () => {
@@ -40,3 +41,5 @@ Deno.test('parseGzip', async (t) => {
     assert(!gzip)
   })
 })
+
+testAsyncFileAccess('Test file access errors for parseGzip', parseGzip)

src/files/gzip.ts

@@ -4,6 +4,7 @@
  */
 import type { Gzip } from '@bids/schema/context'
 import type { BIDSFile } from '../types/filetree.ts'
+import { readBytes } from './access.ts'
 
 /**
  * Parse a gzip header from a file
@@ -19,7 +20,7 @@ export async function parseGzip(
   file: BIDSFile,
   maxBytes: number = 512,
 ): Promise<Gzip | undefined> {
-  const buf = await file.readBytes(maxBytes)
+  const buf = await readBytes(file, maxBytes)
   const view = new DataView(buf.buffer, buf.byteOffset, buf.byteLength)
   if (view.byteLength < 2 || view.getUint16(0, false) !== 0x1f8b) return undefined
 

src/files/json.test.ts

@@ -1,7 +1,9 @@
 import { type assert, assertObjectMatch } from '@std/assert'
 import type { BIDSFile } from '../types/filetree.ts'
 import type { FileIgnoreRules } from './ignore.ts'
+import { testAsyncFileAccess } from './access.test.ts'
 
+import { pathsToTree } from '../files/filetree.ts'
 import { loadJSON } from './json.ts'
 
 function encodeUTF16(text: string) {
@@ -17,9 +19,12 @@ function encodeUTF16(text: string) {
   return buffer
 }
 
-function makeFile(text: string, encoding: string): BIDSFile {
+function makeFile(path: string, text: string, encoding: string): BIDSFile {
   const bytes = encoding === 'utf-8' ? new TextEncoder().encode(text) : encodeUTF16(text)
+  const file = pathsToTree([path]).get(path) as BIDSFile
   return {
+    path: file.path,
+    parent: file.parent,
     readBytes: async (size: number) => {
       return new Uint8Array(bytes)
     },
@@ -29,13 +34,13 @@ function makeFile(text: string, encoding: string): BIDSFile {
 
 Deno.test('Test JSON error conditions', async (t) => {
   await t.step('Load valid JSON', async () => {
-    const JSONfile = makeFile('{"a": 1}', 'utf-8')
+    const JSONfile = makeFile('/valid-contents.json', '{"a": 1}', 'utf-8')
     const result = await loadJSON(JSONfile)
     assertObjectMatch(result, { a: 1 })
   })
 
   await t.step('Error on BOM', async () => {
-    const BOMfile = makeFile('\uFEFF{"a": 1}', 'utf-8')
+    const BOMfile = makeFile('/BOM.json', '\uFEFF{"a": 1}', 'utf-8')
     let error: any = undefined
     await loadJSON(BOMfile).catch((e) => {
       error = e
@@ -44,7 +49,7 @@ Deno.test('Test JSON error conditions', async (t) => {
   })
 
   await t.step('Error on UTF-16', async () => {
-    const UTF16file = makeFile('{"a": 1}', 'utf-16')
+    const UTF16file = makeFile('/utf16.json', '{"a": 1}', 'utf-16')
     let error: any = undefined
     await loadJSON(UTF16file).catch((e) => {
       error = e
@@ -53,11 +58,14 @@ Deno.test('Test JSON error conditions', async (t) => {
   })
 
   await t.step('Error on invalid JSON syntax', async () => {
-    const badJSON = makeFile('{"a": 1]', 'utf-8')
+    const badJSON = makeFile('/bad-syntax.json', '{"a": 1]', 'utf-8')
     let error: any = undefined
     await loadJSON(badJSON).catch((e) => {
       error = e
     })
     assertObjectMatch(error, { code: 'JSON_INVALID' })
   })
+  loadJSON.cache.clear()
 })
+
+testAsyncFileAccess('Test file access errors for loadJSON', loadJSON)

src/files/json.ts

@@ -1,12 +1,14 @@
+import { filememoizeAsync } from '../utils/memoize.ts'
 import type { BIDSFile } from '../types/filetree.ts'
+import { readBytes } from './access.ts'
 
 async function readJSONText(file: BIDSFile): Promise<string> {
   // Read JSON text from a file
   // JSON must be encoded in UTF-8 without a byte order mark (BOM)
   const decoder = new TextDecoder('utf-8', { fatal: true, ignoreBOM: true })
   // Streaming TextDecoders are buggy in Deno and Chrome, so read the
   // entire file into memory before decoding and parsing
-  const data = await file.readBytes(file.size)
+  const data = await readBytes(file, file.size)
   try {
     const text = decoder.decode(data)
     if (text.startsWith('\uFEFF')) {
@@ -20,7 +22,7 @@ async function readJSONText(file: BIDSFile): Promise<string> {
   }
 }
 
-export async function loadJSON(file: BIDSFile): Promise<Record<string, unknown>> {
+async function _loadJSON(file: BIDSFile): Promise<Record<string, unknown>> {
   const text = await readJSONText(file) // Raise encoding errors
   let parsedText
   try {
@@ -36,3 +38,5 @@ export async function loadJSON(file: BIDSFile): Promise<Record<string, unknown>>
   }
   return parsedText
 }
+
+export const loadJSON = filememoizeAsync(_loadJSON)

src/files/nifti.test.ts

@@ -1,6 +1,7 @@
 import { assert, assertEquals, assertObjectMatch } from '@std/assert'
 import { FileIgnoreRules } from './ignore.ts'
 import { BIDSFileDeno } from './deno.ts'
+import { testAsyncFileAccess } from './access.test.ts'
 
 import { axisCodes, loadHeader } from './nifti.ts'
 
@@ -96,3 +97,5 @@ Deno.test('Test extracting axis codes', async (t) => {
     assertEquals(axisCodes(affine), ['A', 'S', 'R'])
   })
 })
+
+testAsyncFileAccess('Test file access errors for loadHeader', loadHeader)

src/files/nifti.ts

@@ -2,6 +2,7 @@ import { isCompressed, isNIFTI1, isNIFTI2, NIFTI1, NIFTI2 } from '@mango/nifti'
 import type { BIDSFile } from '../types/filetree.ts'
 import { logger } from '../utils/logger.ts'
 import type { NiftiHeader } from '@bids/schema/context'
+import { readBytes } from './access.ts'
 
 async function extract(buffer: Uint8Array, nbytes: number): Promise<Uint8Array<ArrayBuffer>> {
   // The fflate decompression that is used in nifti-reader does not like
@@ -32,8 +33,8 @@ async function extract(buffer: Uint8Array, nbytes: number): Promise<Uint8Array<A
 }
 
 export async function loadHeader(file: BIDSFile): Promise<NiftiHeader> {
+  const buf = await readBytes(file, 1024)
   try {
-    const buf = await file.readBytes(1024)
     const data = isCompressed(buf.buffer) ? await extract(buf, 540) : buf.slice(0, 540)
     let header
     if (isNIFTI1(data.buffer)) {