feat(parser): support transfer syntax inference when parsing small DICOM fragments (<100 bytes)

[mlibanori]

This PR introduces the SkipTransferSyntaxDetection option in the DICOM parser. The primary motivation for this change is to enable compatibility between this project and go-netdicom, which requires parsing DICOM network messages without enforcing transfer syntax detection.

[suyashkumar]

Thanks for the contribution! Out of curiosity, is there some reason why the auto-detection doesn't work? https://github.com/suyashkumar/dicom/blob/0fbaef53037ef4c533a9e6004dfc5b7b224ecc39/parse.go#L179-L180

Or is the idea to make this behavior more explicit?

[mlibanori]

Thanks for the review!

The main reason for this change is that certain DICOM commands, such as C-ECHO, can be smaller than 100 bytes. When this happens, the auto-detection logic fails with the error:

https://github.com/suyashkumar/dicom/blob/0fbaef53037ef4c533a9e6004dfc5b7b224ecc39/parse.go#L184

Another possible fix would be to reduce p.reader.rawReader.Peek(100) to p.reader.rawReader.Peek(50), which would allow reading smaller PDU messages. https://github.com/suyashkumar/dicom/blob/0fbaef53037ef4c533a9e6004dfc5b7b224ecc39/parse.go#L181

However, since I'm not entirely sure why the current value is set to 100, I opted to add an option to bypass the transfer syntax detection restriction rather than directly changing the default behavior.

[suyashkumar]

Makes sense! First off, I think adding an option if needed is totally reasonable. Out of curiosity, I'd like to see if we can make it work without an option (if possible and not too complex).

There's no real reason for the 100 byte explicit limit, other than the thought that at least one element would fit in there and that most dicoms would be at least 100bytes. Both assumptions may not hold true, as you point out in this case!

We can probably make this work for dicoms < 100 bytes:

• Attempt to read 100 bytes, and if that results in io.EOF:
• Read as much as possible until io.EOF exhaustion, and continue forward with the existing auto-detection. (We can use io.ReadAll for this).
• We should also correctly handle non io.EOF errors here: https://github.com/suyashkumar/dicom/blob/0fbaef53037ef4c533a9e6004dfc5b7b224ecc39/parse.go#L182

It may not read as nicely, but we could also try to use io.ReadFull for 100 bytes, and simply intentionally ignore a io.ErrUnexpectedEOF to allow smaller reads. We could also wrap this in "readAtMost(n int)" helper which might make it more readable.

What do you think? If you're willing to help with this change, please go ahead! We'll likely want some tests to cover this case as well.

(Also, I'm curious if your payloads pass the autodetection test)!

[mlibanori]

Thank you for the review! I really appreciate the suggestion. Based on your feedback, I implemented the PeekAtMost(n) function, which ignores EOF errors and returns the maximum available buffer size.

https://github.com/suyashkumar/dicom/blob/0c00440ecf6de608a6fc60b8b64aa179cf918ce2/pkg/dicomio/reader.go#L238-L244

I also developed a test using a buffer captured from a real DICOM communication, and the parsing was successfully completed. https://github.com/suyashkumar/dicom/blob/0c00440ecf6de608a6fc60b8b64aa179cf918ce2/parse_test.go#L78-L106

If you have any concerns or think there’s a better approach, I’m happy to iterate further

PS: I noticed that this project does not include the Command Set tags (group 0x0000). Is there a specific reason for this, or have they simply not been implemented yet?

If you’re open to it, I’d be happy to submit a new pull request contributing to the generation of these tags as well.

[mlibanori]

@suyashkumar, do you have any other revisions?

[mlibanori]

all done! lets go?