Skip to content

v1.2.0

Choose a tag to compare

@amrshadid amrshadid released this 25 Jul 22:53
· 159 commits to main since this release
16547ef

This release makes go-dicom interoperable with conforming DICOM implementations. Two defects meant data written or sent by the library could not be read by any other DICOM software — neither was caught by the existing tests, because each side of the library was self-consistent in its own wrongness.

Upgrade notes

Files written by v1.1.1 and earlier have malformed meta headers. They will now read back with an empty MediaStorageSOPClassUID / MediaStorageSOPInstanceUID and a wrong TransferSyntaxUID, because the reader now looks at the correct tags. Files written by v1.2.0 are correct. Any archive produced by an earlier version should be rewritten.

Warnings moved from stdout to stderr. dataset.Add, the file reader, and the file writer's validation path used fmt.Printf, which corrupted piped output with no way to silence it. These now go through config.Logger. If you scrape stdout for Warning: lines, update accordingly — use config.SetLogger to redirect.

No breaking API changes.

Interoperability

Written files were not valid DICOM

WriteFileMetaInfo used the wrong group-0002 tags throughout: the SOP Class UID went to (0002,0010) — which is Transfer Syntax UID — the SOP Instance UID to (0002,0012)Implementation Class UID — and so on. Reading a written file back reported the SOP Class UID as the transfer syntax and left both SOP UIDs empty.

BEFORE                                          AFTER
SOPClassUID    = ""                             SOPClassUID    = "1.2.840.10008.5.1.4.1.1.2"
SOPInstanceUID = ""                             SOPInstanceUID = "1.2.3.4.5.6.7.8.9.100"
TransferSyntax = "1.2.840.10008.5.1.4.1.1.2"    TransferSyntax = "1.2.840.10008.1.2.1"

Tags now follow PS3.6. The Type 1 (0002,0001) File Meta Information Version is always written. The reader was also looking for AE titles at (0002,0100..0102), a range belonging to Private Information attributes.

Data sets ignored the negotiated transfer syntax

Every DIMSE data set was encoded as Implicit VR Little Endian regardless of what was negotiated. Since Explicit VR Little Endian is proposed first by default, essentially every real peer received an unparsable data set. Adds a dataset codec covering Implicit VR LE, Explicit VR LE, Explicit VR BE, and Deflated Explicit VR LE, threaded through every DIMSE path.

Odd-length values corrupted the byte stream

DICOM requires even-length values (PS3.5 §7.1.1); one odd value misaligns everything after it. Padding is now applied centrally using the VR's designated character, rather than left to callers.

Security

Four vectors reachable from an unauthenticated peer:

  • SCP process crash — a presentation context with zero transfer syntax sub-items caused an index-out-of-range panic in NegotiatePresentationContexts, terminating the whole server.
  • ~4 GiB allocation from a 6-byte headerDecodePDU sized its buffer from the peer-controlled 32-bit PDU length before reading. Now capped at MaxPDULengthLimit (128 MiB).
  • Oversized PDV allocationdecodeDataTF allocated from the PDV length, which is independent of the enclosing PDU length.
  • Multi-gigabyte allocation from a file — the reader allocated directly from the declared Value Length; an element claiming 0xFFFFFFFF allocated that much. Lengths above 16 MiB are now verified against the bytes remaining.

Also hardened: DecodeCommandDataset used Read rather than io.ReadFull (silent zero-padded values on short reads), and the role-selection and user-identity decoders trusted peer-supplied lengths without bounds checks.

Features

Nested sequence (SQ) parsing

The reader stopped at the first Item or Sequence Delimitation Item, so everything from the first sequence onward was silently dropped — any Structured Report, multi-frame functional group, or referenced-image sequence was read only up to that point.

Now parses recursively: defined- and undefined-length sequences and items, sequences under implicit VR (VR recovered from the dictionary), empty sequences, and encapsulated fragmented pixel data. Nesting is bounded by MaxSequenceDepth (64).

DICOMFile.GetDataset()

Materialises the parsed tree as a Dataset with nested sequences as child Datasets. The README documented this method but it did not exist — the documented example did not compile.

Extended negotiation, actually negotiated

UserInformationItem neither emitted nor parsed the extended sub-items, so async operations, SCP/SCU role selection, and user identity were never negotiated despite being listed as supported. Now wired through SCUConfig.ExtendedNegotiation, with Association.PeerUserInformation() and RoleSelectionFor() exposing the outcome.

Other fixes

  • SCPConfig.MaxAssociations was documented but never read — the server accepted unbounded concurrent associations. Now enforced, with an A-ASSOCIATE-RJ (local-limit-exceeded) rather than a dropped socket.
  • QueryRetrieveHandler.OnGet had no HandleCGet method, so setting it silently did nothing.
  • SCU.NEventReport reported the wrong MessageIDRespondedTo and burned an extra message ID.

Known limitations

Now stated plainly in the README rather than implied to work:

  • C-MOVE / C-GET as an SCP send no C-STORE sub-operations. Both work fully as an SCU. This is the largest remaining gap.
  • Asynchronous operations are negotiated but not enforced — the SCU is serial.
  • No transfer syntax transcoding.
  • filewriter does not yet serialise SQ elements.
  • show / info / convert use a separate flat parser that does not descend into sequences.

Verification

Every commit builds and passes independently, so the history is bisectable. gofmt, go vet, and golangci-lint clean; 29/29 packages passing under -race on Linux, macOS, and Windows.

End-to-end verified against real TCP with the built CLI: storescp receiving from echoscu and storescu, with the received file re-read and every value compared.

New coverage: per-vector security regression tests, transfer-syntax round trips, sequence parsing, file meta tag assignments, and a full write → C-STORE → receive → read → compare integration test.

Full changelog: v1.1.1...v1.2.0