Skip to content

Reject a BAM record whose read-name length is 0 when it is decoded - #1952

Open
tfenne wants to merge 1 commit into
masterfrom
tf_bam_zero_read_name_length
Open

tfenne wants to merge 1 commit into
masterfrom
tf_bam_zero_read_name_length

Conversation

@tfenne

@tfenne tfenne commented Oct 30, 2026

Copy link
Copy Markdown
Member

A BAM record with a read-name length of 0 decoded without complaint and failed later; it is now rejected at decode.

  • l_read_name counts the NUL that ends the name, so it is at least 1. With 0, getReadName() threw StringIndexOutOfBoundsException, and the CIGAR, bases and qualities, found by offset from the name, decoded from the wrong bytes.
  • BAMRecordCodec.decode now throws SAMFormatException, as it does for a record length below the fixed block size, and as htslib's bam_read1 rejects l_qname < 1.

Found while reading broadinstitute/picard#2063.

Fixes #1948

l_read_name counts the NUL that ends the read name, so it is at least
1. decode accepted 0, and the record failed later and far from the
cause: getReadName() threw StringIndexOutOfBoundsException, and the
CIGAR, bases and qualities, found by offset from the name, decoded from
the wrong bytes. decode now throws SAMFormatException, as it does for a
record length below the fixed block size, and as htslib rejects the
record in bam_read1.
@coderabbitai

coderabbitai Bot commented Oct 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 17 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9bba5966-34f5-4e9e-b7f4-db50435f25a0
Commits

Reviewing files that changed from the base of the PR and between 3b92d7b and 524a1be.

Files selected for processing: 3
  • CHANGELOG.md
  • src/main/java/htsjdk/samtools/BAMRecordCodec.java
  • src/test/java/htsjdk/samtools/BAMRecordCodecTest.java
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BAMRecordCodec accepts a read-name length of 0 and fails later with StringIndexOutOfBoundsException

1 participant