Parse undefined length OB/OW elements (#350)

This addresses #349.

OB and OW fields with undefined length now parse their contents as a sequence, concatenating all the bytes in the items within.

Author: patrickwhite256

read.go

@@ -29,9 +29,11 @@ var (
 	// ErrorUnsupportedBitsAllocated indicates that the BitsAllocated in the
 	// NativeFrame PixelData is unsupported. In this situation, the rest of the
 	// dataset returned is still valid.
-	ErrorUnsupportedBitsAllocated = errors.New("unsupported BitsAllocated")
-	errorUnableToParseFloat       = errors.New("unable to parse float type")
-	ErrorExpectedEvenLength       = errors.New("field length is not even, in violation of DICOM spec")
+	ErrorUnsupportedBitsAllocated     = errors.New("unsupported BitsAllocated")
+	errorUnableToParseFloat           = errors.New("unable to parse float type")
+	ErrorExpectedEvenLength           = errors.New("field length is not even, in violation of DICOM spec")
+	ErrorExpectedDefinedLength        = errors.New("unable to read field of undefined length")
+	ErrorExpectedSequenceDelimitation = errors.New("expected to find sequence delimitation item (fffe,e0dd)")
 )
 
 // reader is responsible for mid-level dicom parsing capabilities, like
@@ -122,6 +124,9 @@ func (r *reader) readValue(t tag.Tag, vr string, vl uint32, isImplicit bool, d *
 	// TODO: if we keep consistent function signature, consider a static map of VR to func?
 	switch vrkind {
 	case tag.VRBytes:
+		if vl == tag.VLUndefinedLength {
+			return r.readUndefinedLengthByteValue()
+		}
 		return r.readBytes(t, vr, vl)
 	case tag.VRString:
 		return r.readString(t, vr, vl)
@@ -154,6 +159,34 @@ func (r *reader) readValue(t tag.Tag, vr string, vl uint32, isImplicit bool, d *
 	}
 }
 
+// readUndefinedLengthByteValue reads a value of type OB or OW of undefined length.
+// see https://github.com/suyashkumar/dicom/issues/349 for details.
+// pixel data should not use this and instead use readPixelData.
+func (r *reader) readUndefinedLengthByteValue() (Value, error) {
+	var allData []byte
+	foundDelimeter := false
+
+	for !r.rawReader.IsLimitExhausted() {
+		data, terminated, err := r.readRawItem(false)
+		if err != nil {
+			return nil, fmt.Errorf("error reading undefined byte value: %w", err)
+		}
+
+		if terminated {
+			foundDelimeter = true
+			break
+		}
+
+		allData = append(allData, data...)
+	}
+
+	if !foundDelimeter {
+		return nil, ErrorExpectedSequenceDelimitation
+	}
+
+	return &bytesValue{value: allData}, nil
+}
+
 // readHeader reads the DICOM magic header and group two metadata elements.
 // This should only be called once per DICOM at the start of parsing.
 func (r *reader) readHeader() ([]*Element, error) {
@@ -641,6 +674,10 @@ func (r *reader) readSequenceItem(t tag.Tag, vr string, vl uint32, d *Dataset) (
 }
 
 func (r *reader) readBytes(t tag.Tag, vr string, vl uint32) (Value, error) {
+	if vl == tag.VLUndefinedLength {
+		return nil, ErrorExpectedDefinedLength
+	}
+
 	// TODO: add special handling of PixelData
 	if vr == vrraw.OtherByte || vr == vrraw.Unknown {
 		data := make([]byte, vl)
@@ -672,6 +709,10 @@ func (r *reader) readBytes(t tag.Tag, vr string, vl uint32) (Value, error) {
 }
 
 func (r *reader) readString(t tag.Tag, vr string, vl uint32) (Value, error) {
+	if vl == tag.VLUndefinedLength {
+		return nil, ErrorExpectedDefinedLength
+	}
+
 	str, err := r.rawReader.ReadString(vl)
 	if err != nil {
 		return nil, fmt.Errorf("error reading string element (%v) value: %w", t, err)
@@ -821,12 +862,11 @@ func (r *reader) readElement(d *Dataset, fc chan<- *frame.Frame) (*Element, erro
 	}
 
 	return &Element{Tag: *t, ValueRepresentation: tag.GetVRKind(*t, vr), RawValueRepresentation: vr, ValueLength: vl, Value: val}, nil
-
 }
 
 // Read an Item object as raw bytes, useful when parsing encapsulated PixelData.
 // This returns the read raw item, an indication if this is the end of the set
-// of items, and a possible errorawReader.
+// of items, and a possible error.
 func (r *reader) readRawItem(shouldSkip bool) ([]byte, bool, error) {
 	t, err := r.readTag()
 	if err != nil {

read_test.go

@@ -206,6 +206,7 @@ func TestReadOWBytes(t *testing.T) {
 			}
 
 			r := &reader{rawReader: dicomio.NewReader(bufio.NewReader(&data), binary.LittleEndian, int64(data.Len()))}
+
 			got, err := r.readBytes(tag.Tag{}, tc.VR, uint32(data.Len()))
 			if !errors.Is(err, tc.expectedErr) {
 				t.Fatalf("readBytes(r, tg, %s, %d) got unexpected error: got: %v, want: %v", tc.VR, data.Len(), err, tc.expectedErr)
@@ -217,6 +218,66 @@ func TestReadOWBytes(t *testing.T) {
 	}
 }
 
+func TestReadUndefinedLengthByteValue(t *testing.T) {
+	cases := []struct {
+		name        string
+		bytes       []byte
+		want        Value
+		expectedErr error
+	}{
+		{
+			name: "happy path",
+			bytes: []byte{
+				0xfe, 0xff, 0x00, 0xe0, // item tag (fffe, e000)
+				0x04, 0x00, 0x00, 0x00, // length 4
+				0x01, 0x02, 0x03, 0x04, // data (4 bytes)
+				0xfe, 0xff, 0xdd, 0xe0, // sequence delimitation item tag (fffe, e0dd)
+				0x00, 0x00, 0x00, 0x00, // length 0 (required)
+			},
+			want:        &bytesValue{value: []byte{0x01, 0x02, 0x03, 0x04}},
+			expectedErr: nil,
+		},
+		{
+			name: "not formatted as sequence",
+			bytes: []byte{
+				0x01, 0x02, 0x03, 0x04, // data (4 bytes)
+			},
+			want:        &bytesValue{value: []byte{}},
+			expectedErr: io.EOF,
+		},
+		{
+			name: "missing delimitation item",
+			bytes: []byte{
+				0xfe, 0xff, 0x00, 0xe0, // item tag (fffe, e000)
+				0x04, 0x00, 0x00, 0x00, // length 4
+				0x01, 0x02, 0x03, 0x04, // data (4 bytes)
+			},
+			want:        &bytesValue{value: []byte{}},
+			expectedErr: ErrorExpectedSequenceDelimitation,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			data := bytes.Buffer{}
+			if err := binary.Write(&data, binary.LittleEndian, tc.bytes); err != nil {
+				t.Errorf("TestReadOWBytes: Unable to setup test buffer")
+			}
+
+			r := &reader{rawReader: dicomio.NewReader(bufio.NewReader(&data), binary.LittleEndian, int64(data.Len()))}
+
+			got, err := r.readUndefinedLengthByteValue()
+			if tc.expectedErr != nil {
+				if !errors.Is(err, tc.expectedErr) {
+					t.Fatalf("readUndefinedLengthByteValue got unexpected error: got: %v, want: %v", err, tc.expectedErr)
+				}
+			} else if diff := cmp.Diff(got, tc.want, cmp.AllowUnexported(bytesValue{})); diff != "" {
+				t.Errorf("readUndefinedLengthByteValue unexpected diff: %s", diff)
+			}
+		})
+	}
+}
+
 func TestReadNativeFrames(t *testing.T) {
 	cases := []struct {
 		Name              string