dicom.Write() fails on nested dataset where 'ItemDelimitationItem' has a blank VR when dicom.SkipVRVerification() is used.

[bpeake-illuscio]

Hello!

I'm having an issue writing a dicom that I've read in.

Here is some example code, where I make no changes at all to the file:

package main

import (
	"bytes"
	"fmt"
	"github.com/illuscio-dev/edesia-go/service/internal/edesiatest"
	"github.com/suyashkumar/dicom"
)


func main() {
	source := bytes.NewBuffer(edesiatest.DicomFileBasicMR1Data)
	dataset, err := dicom.Parse(source, int64(len(edesiatest.DicomFileBasicMR1Data)), nil)
	if err != nil {
		panic(err)
	}

	writeBuffer := new(bytes.Buffer)
	err = dicom.Write(writeBuffer, dataset, dicom.SkipValueTypeVerification(), dicom.SkipVRVerification())
	fmt.Println("ERROR:", err)
}

I get the following error:

ERROR: ERROR dicomio.writeVRVL: Value Representation must be of length 2, e.g. 'UN'. For tag=(fffe,e00d)[ItemDelimitationItem], it was RawValueRepresentation=

The weird thing is that when I do this:

func main() {
	source := bytes.NewBuffer(edesiatest.DicomFileBasicMR1Data)
	dataset, err := dicom.Parse(source, int64(len(edesiatest.DicomFileBasicMR1Data)), nil)
	if err != nil {
		panic(err)
	}

	_, err = dataset.FindElementByTagNested(tag.ItemDelimitationItem)
	if err != nil {
		panic(err)
	}
}

It doesn't get find that item. From the Dicom Spec it looks like this is a special element that signals the end of a nested dataset. I am guessing your code is inserting this, but failing it's own VR checks since the DICOM spec does not seem to define a VR for this element type.

According to my debugger the error is coming from write.go, line 309:

if len(vr) != 2 && vl != tag.VLUndefinedLength && t != tag.SequenceDelimitationItem {
	return fmt.Errorf("ERROR dicomio.writeVRVL: Value Representation must be of length 2, e.g. 'UN'. For tag=%v, it was RawValueRepresentation=%v",
		tag.DebugString(t), vr)
}

I think we need to add && t != tag.ItemDelimitationItem to this bit of logic.

Digging a bit deeper, it seems that this specifically triggers when dicom.SkipVRVerification() is used because this option ALSO causes the writer to skip over setting a default VR if none is set, here (write.go, line 185):

if !opts.skipVRVerification {
	var err error
	vr, err = verifyVROrDefault(elem.Tag, elem.RawValueRepresentation)
	if err != nil {
		return err
	}
}

So if skipVRVerification, ItemDelimitationItem never gets a default VR and is not caught when checking VR length.

I'll make a PR shortly for this.