search

samtools / htsjdk

A Java API for high-throughput sequencing data (HTS) formats.
http://samtools.github.io/htsjdk/
283 stars 242 forks source link

CramHeader ID does not clear its buffer on set or construction #1307

Closed jmthibault79 closed 2 years ago

jmthibault79 commented 5 years ago

Subject of the issue

CramHeader construction with ID (and setId()) do not simply set the ID value. Instead they copy up to 20 bytes of the desired value into a 20-byte buffer. As a result, extra bytes remain in the buffer. If there was a previous longer ID set, the stale bytes past the new limit remain. As a result, setID() and getId() do not round-trip.

Is there a reason for this behavior?

cmnbroad commented 5 years ago

I doubt there is any reason for it - sounds like a bug. The contents of the 20 byte space reserved in the header is not defined by the spec, and is left up to the implementation. htsjdk uses the first 20 bytes of the file name (which has it's own problems). We should fix it.

cmnbroad commented 5 years ago

@jmthibault79 Is this still an issue, or was this fixed in one of your PRs ?

jmthibault79 commented 5 years ago

Addressed in #1306 which needs review

cmnbroad commented 5 years ago

Fixed in refactor branch.

cmnbroad commented 2 years ago

Fixed in CramHeader in #1440.