Apply review suggestions (#9256)

Author: xeinherjer-dev

b1.sh

@@ -0,0 +1,4 @@
+
+pnpm install && npx nx build
+
+

b2.sh

@@ -0,0 +1,2 @@
+
+pnpm test:unit src/scripts/metadata/ogg.test.ts --coverage --reporter=verbose

b3.sh

@@ -0,0 +1,2 @@
+
+cp -Rf dist/* ../ComfyUI_windows_portable/python_embeded/Lib/site-packages/comfyui_frontend_package/static/

src/scripts/metadata/ogg.test.ts

@@ -187,6 +187,18 @@ describe('getOggMetadata', () => {
           return buffer.buffer
         }
       },
+      {
+        name: 'segment data exceeds buffer boundary',
+        createBuffer: () => {
+          // Page header declares 1 segment of length 200, but only 10 bytes of data follow
+          const buffer = new Uint8Array(27 + 1 + 10)
+          buffer.set(new TextEncoder().encode('OggS'), 0)
+          buffer[26] = 1 // 1 segment
+          buffer[27] = 200 // segment length = 200 (exceeds remaining 10 bytes)
+          // Only 10 bytes of data follow; segment exceeds boundary
+          return buffer.buffer
+        }
+      },
       {
         name: 'truncated user comment in OpusTags',
         createBuffer: () => {
@@ -207,7 +219,6 @@ describe('getOggMetadata', () => {
         name: 'file smaller than the Ogg magic number',
         createBuffer: () => new Uint8Array([1, 2]).buffer
       },
-
       {
         name: 'page has an invalid magic number (stop parsing)',
         createBuffer: () => {
@@ -222,6 +233,73 @@ describe('getOggMetadata', () => {
           combined.set(page2, page1.length)
           return combined.buffer
         }
+      },
+      {
+        name: 'OpusTags packet truncated before vendorLength field',
+        createBuffer: () => {
+          // Packet is only 10 bytes: 'OpusTags'(8) + 2 extra bytes (not enough for 4-byte vendorLength)
+          const packet = new Uint8Array(10)
+          packet.set(new TextEncoder().encode('OpusTags'), 0)
+          const buffer = new Uint8Array(27 + 1 + packet.length)
+          buffer.set(new TextEncoder().encode('OggS'), 0)
+          buffer[26] = 1
+          buffer[27] = packet.length
+          buffer.set(packet, 28)
+          return buffer.buffer
+        }
+      },
+      {
+        name: 'OpusTags packet with oversized vendorLength',
+        createBuffer: () => {
+          // Packet: 'OpusTags'(8) + vendorLength=9999(4) + no actual vendor bytes
+          const packet = new Uint8Array(12)
+          packet.set(new TextEncoder().encode('OpusTags'), 0)
+          new DataView(packet.buffer).setUint32(8, 9999, true) // vendor string claims 9999 bytes
+          const buffer = new Uint8Array(27 + 1 + packet.length)
+          buffer.set(new TextEncoder().encode('OggS'), 0)
+          buffer[26] = 1
+          buffer[27] = packet.length
+          buffer.set(packet, 28)
+          return buffer.buffer
+        }
+      },
+      {
+        name: 'OpusTags packet truncated before userCommentListLength',
+        createBuffer: () => {
+          // Packet: 'OpusTags'(8) + vendorLength=0(4) + no userCommentListLength
+          const packet = new Uint8Array(12) // exactly 8 + 4, no bytes for userCommentListLength
+          packet.set(new TextEncoder().encode('OpusTags'), 0)
+          new DataView(packet.buffer).setUint32(8, 0, true) // vendorLength=0
+          // packet ends here — no room for userCommentListLength (needs 4 more bytes)
+          const buffer = new Uint8Array(27 + 1 + packet.length)
+          buffer.set(new TextEncoder().encode('OggS'), 0)
+          buffer[26] = 1
+          buffer[27] = packet.length
+          buffer.set(packet, 28)
+          return buffer.buffer
+        }
+      },
+      {
+        name: 'OpusTags packet truncated mid-comment-list (missing comment length field)',
+        createBuffer: () => {
+          const commentBytes = new TextEncoder().encode('key=value')
+          // Packet: 'OpusTags'(8) + vendorLength=0(4) + userCommentListLength=2(4) +
+          //         commentLength=9(4) + commentData(9) — second comment length field is missing
+          const packet = new Uint8Array(8 + 4 + 4 + 4 + commentBytes.length)
+          const view = new DataView(packet.buffer)
+          packet.set(new TextEncoder().encode('OpusTags'), 0)
+          view.setUint32(8, 0, true) // vendorLength = 0
+          view.setUint32(12, 2, true) // userCommentListLength = 2 (but only 1 follows)
+          view.setUint32(16, commentBytes.length, true) // first comment length
+          packet.set(commentBytes, 20) // first comment data
+          // second comment's length field is absent — packet ends here
+          const buffer = new Uint8Array(27 + 1 + packet.length)
+          buffer.set(new TextEncoder().encode('OggS'), 0)
+          buffer[26] = 1
+          buffer[27] = packet.length
+          buffer.set(packet, 28)
+          return buffer.buffer
+        }
       }
     ]
 
@@ -231,14 +309,12 @@ describe('getOggMetadata', () => {
           [createBuffer()],
           `test_${name.replace(/\s+/g, '_')}.ogg`
         )
-        try {
-          const result = await getOggMetadata(file)
-          expect(result.prompt).toBeUndefined()
-          expect(result.workflow).toBeUndefined()
-        } catch (e) {
-          // If it throws (e.g. RangeError from DataView out of bounds), it's successfully handled gracefully by the caller
-          expect(e).toBeInstanceOf(Error)
-        }
+        await expect(
+          getOggMetadata(file).then((result) => {
+            expect(result.prompt).toBeUndefined()
+            expect(result.workflow).toBeUndefined()
+          })
+        ).resolves.not.toThrow()
       })
     }
   })
@@ -247,7 +323,7 @@ describe('getOggMetadata', () => {
     const scenarios: {
       name: string
       createBuffer: () => ArrayBuffer
-      expectedPrompt?: any
+      expectedPrompt?: Record<string, unknown>
     }[] = [
       {
         name: 'unrelated Vorbis comments',
@@ -258,6 +334,14 @@ describe('getOggMetadata', () => {
           }),
         expectedPrompt: { valid: true } // Workflow is undefined, Prompt is valid
       },
+      {
+        name: 'invalid JSON in prompt/workflow comments (plain text)',
+        createBuffer: () =>
+          createOggWithOpusTags({
+            prompt: 'hello world',
+            workflow: 'plain text workflow'
+          })
+      },
       {
         name: 'user comments without an equal sign',
         createBuffer: () => {
@@ -287,18 +371,15 @@ describe('getOggMetadata', () => {
           [createBuffer()],
           `test_${name.replace(/\s+/g, '_')}.ogg`
         )
-        try {
-          const result = await getOggMetadata(file)
+        await expect(getOggMetadata(file)).resolves.toSatisfy((result) => {
           if (expectedPrompt) {
-            expect(result.prompt).toEqual(expectedPrompt) // either undefined or { valid: true }
+            expect(result.prompt).toEqual(expectedPrompt)
           } else {
             expect(result.prompt).toBeUndefined()
           }
           expect(result.workflow).toBeUndefined()
-        } catch (e) {
-          // Handled via thrown exception
-          expect(e).toBeInstanceOf(Error)
-        }
+          return true
+        })
       })
     }
   })

src/scripts/metadata/ogg.ts

@@ -50,6 +50,10 @@ function extractOpusTags(data: Uint8Array, decoder: TextDecoder): Uint8Array[] {
 
     for (let i = 0; i < pageSegmentsCount; i++) {
       const segmentLength = data[lengthsOffset + i]
+
+      // Bounds check: ensure the segment data lies within the available data
+      if (dataOffset + segmentLength > data.length) break
+
       const segment = data.subarray(dataOffset, dataOffset + segmentLength)
       dataOffset += segmentLength
 
@@ -86,20 +90,39 @@ function parseVorbisComments(packetData: Uint8Array, decoder: TextDecoder) {
     packetData.byteLength
   )
 
+  // Bounds check: ensure vendor length field is within packet
+  if (readIndex + 4 > packetData.length)
+    return { prompt: undefined, workflow: undefined }
+
   // Skip vendor string length (4 bytes) + vendor string
   const vendorLength = packetView.getUint32(readIndex, true)
-  readIndex += 4 + vendorLength
+  readIndex += 4
+
+  // Bounds check: ensure vendor string is within packet
+  if (readIndex + vendorLength > packetData.length)
+    return { prompt: undefined, workflow: undefined }
+  readIndex += vendorLength
+
+  // Bounds check: ensure user comment list length field is within packet
+  if (readIndex + 4 > packetData.length)
+    return { prompt: undefined, workflow: undefined }
 
   // Get the number of user comments
   const userCommentListLength = packetView.getUint32(readIndex, true)
   readIndex += 4
 
   let prompt, workflow
   for (let i = 0; i < userCommentListLength; i++) {
+    // Bounds check: ensure comment length field is within packet
+    if (readIndex + 4 > packetData.length) break
+
     // Vorbis Comments spec: Get comment length (32-bit little-endian)
     const commentLength = packetView.getUint32(readIndex, true)
     readIndex += 4
 
+    // Bounds check: ensure comment data is within packet
+    if (readIndex + commentLength > packetData.length) break
+
     // Extract and decode the comment string (UTF-8)
     const text = decoder.decode(
       packetData.subarray(readIndex, readIndex + commentLength)
@@ -111,9 +134,17 @@ function parseVorbisComments(packetData: Uint8Array, decoder: TextDecoder) {
       const key = text.substring(0, separatorIndex)
       const value = text.substring(separatorIndex + 1)
       if (key === 'prompt') {
-        prompt = JSON.parse(value)
+        try {
+          prompt = JSON.parse(value)
+        } catch (e) {
+          console.warn('Ogg metadata parsing failed for prompt:', e)
+        }
       } else if (key === 'workflow') {
-        workflow = JSON.parse(value)
+        try {
+          workflow = JSON.parse(value)
+        } catch (e) {
+          console.warn('Ogg metadata parsing failed for workflow:', e)
+        }
       }
     }