Merge pull request #101 from yagni/vr-callback-undefined-fix

Fix bug where peeking to determine whether a field is an SQ would be …
cornerstonejs · Sep 19, 2022 · eb51626 · eb51626
2 parents 04fcbbe + 7e504ce
commit eb51626
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 10 deletions.
diff --git a/src/readDicomElementImplicit.js b/src/readDicomElementImplicit.js
@@ -9,8 +9,11 @@ import { isPrivateTag } from './util/util.js';
 
 const isSequence = (element, byteStream, vrCallback) => {
   // if a data dictionary callback was provided, use that to verify that the element is a sequence.
-  if (typeof vrCallback !== 'undefined') {
-    return (vrCallback(element.tag) === 'SQ');
+  if (vrCallback !== undefined) {
+    const callbackValue = vrCallback(element.tag);
+    if (callbackValue !== undefined) {
+      return (callbackValue === 'SQ');
+    }
   }
 
   if ((byteStream.position + 4) <= byteStream.byteArray.length) {

diff --git a/test/parseDicomDataSet_test.js b/test/parseDicomDataSet_test.js
@@ -90,7 +90,7 @@ describe('parseDicomDataSet', () => {
         0xfe, 0xff, 0x00, 0xe0, 0x0A, 0x00, 0x00, 0x00,
       ];
       const callback = (tag) => {
-        return undefined; // nothing should be interpreted as an SQ
+        return (tag === 'x7fe00010') ? 'OW' : undefined;
       };
       const byteArray = convertToByteArray(bytes);
       const byteStream = new ByteStream(littleEndianByteArrayParser, byteArray);

diff --git a/test/readDicomElementImplicit_test.js b/test/readDicomElementImplicit_test.js
@@ -142,15 +142,33 @@ describe('readDicomElementImplicit', () => {
     expect(invoker).to.throw();
   });
 
-  it('bytes resembling an item tag are not treated like an SQ item when using a callback', () => {
+  it('bytes resembling an item tag look like an implicit SQ item when using a callback that returns undefined', () => {
     // Arrange
     // (7fe0,0010)                               8
     const bytes = [0xe0, 0x7f, 0x10, 0x00, 0x08, 0x00, 0x00, 0x00,
       // Looks like an item tag, but isn't since it's within pixel data
       0xfe, 0xff, 0x00, 0xe0, 0x0A, 0x00, 0x00, 0x00,
     ];
     const callback = (tag) => {
-      return undefined; // nothing should be interpreted as an SQ
+      return undefined;
+    };
+    const byteStream = new ByteStream(littleEndianByteArrayParser, convertToByteArray(bytes));
+    const invoker = () => readDicomElementImplicit(byteStream, undefined, callback);
+
+    // Act/Assert
+    // invalid value for parameter 'maxPosition'
+    expect(invoker).to.throw();
+  });
+
+  it('bytes resembling an item tag are not treated like an SQ item when using a callback (callback overrides peeking)', () => {
+    // Arrange
+    // (7fe0,0010)                               8
+    const bytes = [0xe0, 0x7f, 0x10, 0x00, 0x08, 0x00, 0x00, 0x00,
+      // Looks like an item tag, but isn't since it's within pixel data
+      0xfe, 0xff, 0x00, 0xe0, 0x0A, 0x00, 0x00, 0x00,
+    ];
+    const callback = (tag) => {
+      return (tag === 'x7fe00010') ? 'OW' : undefined;
     };
     const byteStream = new ByteStream(littleEndianByteArrayParser, convertToByteArray(bytes));
 
@@ -179,7 +197,26 @@ describe('readDicomElementImplicit', () => {
     expect(invoker).to.throw();
   });
 
-  it('bytes resembling an end-of-sequence tag are not treated like an SQ item when using a callback', () => {
+  it('bytes resembling an end-of-sequence tag look like an implicit SQ item when using a callback that returns undefined', () => {
+    // Arrange
+    // (7fe0,0010)                                           11
+    const bytes = [0xe0, 0x7f, 0x10, 0x00, 0x0B, 0x00, 0x00, 0x00,
+      // Looks like a sequence delimiter tag, but isn't since it's within pixel data
+      0xfe, 0xff, 0xdd, 0xe0, 0x0A, 0x00, 0x00, 0x00,
+      0x12, 0x43, 0x98,
+    ];
+    const callback = (tag) => {
+      return undefined;
+    };
+    const byteStream = new ByteStream(littleEndianByteArrayParser, convertToByteArray(bytes));
+    const invoker = () => readDicomElementImplicit(byteStream, undefined, callback);
+
+    // Act/Assert
+    // item tag (FFFE,E000) not found at offset 8
+    expect(invoker).to.throw();
+  });
+
+  it('bytes resembling an end-of-sequence tag are not treated like an SQ item when using a callback (callback overrides peeking)', () => {
     // Arrange
     // (7fe0,0010)                              11
     const bytes = [0xe0, 0x7f, 0x10, 0x00, 0x0B, 0x00, 0x00, 0x00,
@@ -188,7 +225,7 @@ describe('readDicomElementImplicit', () => {
       0x12, 0x43, 0x98,
     ];
     const callback = (tag) => {
-      return undefined; // nothing should be interpreted as an SQ
+      return (tag === 'x7fe00010') ? 'OW' : undefined;
     };
     const byteStream = new ByteStream(littleEndianByteArrayParser, convertToByteArray(bytes));
 

diff --git a/test/readSequenceItemsImplicit_test.js b/test/readSequenceItemsImplicit_test.js
@@ -28,7 +28,7 @@ describe('readSequenceItemsImplicit', () => {
                  0xfe, 0xff, 0xdd, 0xe0, 0x00, 0x00, 0x00, 0x00,
     ];
     const callback = (tag) => {
-      return undefined; // nothing should be interpreted as an SQ
+      return (tag === 'x7fe00010') ? 'OW' : undefined;
     };
     const byteStream = new ByteStream(littleEndianByteArrayParser, convertToByteArray(bytes));
     const element = { length: 0xFFFFFFFF };
@@ -95,7 +95,7 @@ describe('readSequenceItemsImplicit', () => {
                  0xfe, 0xff, 0xdd, 0xe0, 0x00, 0x00, 0x00, 0x00,
     ];
     const callback = (tag) => {
-      return undefined; // nothing should be interpreted as an SQ
+      return (tag === 'x7fe00010') ? 'OW' : undefined;
     };
     const byteStream = new ByteStream(littleEndianByteArrayParser, convertToByteArray(bytes));
     const element = { length: 0xFFFFFFFF };
@@ -128,7 +128,7 @@ describe('readSequenceItemsImplicit', () => {
                  0xfe, 0xff, 0x00, 0xe0, 0x0A, 0x00, 0x00, 0x00,
     ];
     const callback = (tag) => {
-      return undefined; // nothing should be interpreted as an SQ
+      return (tag === 'x7fe00010') ? 'OW' : undefined;
     };
     const byteStream = new ByteStream(littleEndianByteArrayParser, convertToByteArray(bytes));
     const element = {dataOffset: 0, length: 24};