diff --git a/noodles-bam/CHANGELOG.md b/noodles-bam/CHANGELOG.md index 73b755053..c7603de3a 100644 --- a/noodles-bam/CHANGELOG.md +++ b/noodles-bam/CHANGELOG.md @@ -7,6 +7,12 @@ * bam/io/writer/builder: Add build from writer (`Builder::build_from_writer`). +### Changed + + * bam/record/codec/decoder: Remove panic when reading bin on EOF. + + This now returns a `DecodeError` if the input reaches EOF. + ## 0.66.0 - 2024-08-04 ### Added diff --git a/noodles-bam/src/record/codec/decoder.rs b/noodles-bam/src/record/codec/decoder.rs index 2366df8a5..94fa9d209 100644 --- a/noodles-bam/src/record/codec/decoder.rs +++ b/noodles-bam/src/record/codec/decoder.rs @@ -1,5 +1,6 @@ //! BAM record decoder. +mod bin; pub(crate) mod cigar; pub mod data; mod flags; @@ -16,14 +17,14 @@ pub(crate) use self::{ reference_sequence_id::get_reference_sequence_id, sequence::get_sequence, }; -use std::{error, fmt, mem}; +use std::{error, fmt}; use bytes::Buf; use noodles_sam::{self as sam, alignment::RecordBuf}; use self::{ - flags::get_flags, mapping_quality::get_mapping_quality, name::get_name, position::get_position, - template_length::get_template_length, + bin::discard_bin, flags::get_flags, mapping_quality::get_mapping_quality, name::get_name, + position::get_position, template_length::get_template_length, }; /// An error when a raw BAM record fails to parse. @@ -35,6 +36,8 @@ pub enum DecodeError { InvalidAlignmentStart(position::DecodeError), /// The mapping quality is invalid. InvalidMappingQuality(mapping_quality::DecodeError), + /// The bin is invalid. + InvalidBin(bin::DecodeError), /// The flags are invalid. InvalidFlags(flags::DecodeError), /// The mate reference sequence ID is invalid. @@ -61,6 +64,7 @@ impl error::Error for DecodeError { Self::InvalidReferenceSequenceId(e) => Some(e), Self::InvalidAlignmentStart(e) => Some(e), Self::InvalidMappingQuality(e) => Some(e), + Self::InvalidBin(e) => Some(e), Self::InvalidFlags(e) => Some(e), Self::InvalidMateReferenceSequenceId(e) => Some(e), Self::InvalidMateAlignmentStart(e) => Some(e), @@ -80,6 +84,7 @@ impl fmt::Display for DecodeError { Self::InvalidReferenceSequenceId(_) => write!(f, "invalid reference sequence ID"), Self::InvalidAlignmentStart(_) => write!(f, "invalid alignment start"), Self::InvalidMappingQuality(_) => write!(f, "invalid mapping quality"), + Self::InvalidBin(_) => write!(f, "invalid bin"), Self::InvalidFlags(_) => write!(f, "invalid flags"), Self::InvalidMateReferenceSequenceId(_) => { write!(f, "invalid mate reference sequence ID") @@ -116,8 +121,7 @@ where *record.mapping_quality_mut() = get_mapping_quality(src).map_err(DecodeError::InvalidMappingQuality)?; - // Discard bin. - src.advance(mem::size_of::()); + discard_bin(src).map_err(DecodeError::InvalidBin)?; let n_cigar_op = cigar::get_op_count(src).map_err(DecodeError::InvalidCigar)?; diff --git a/noodles-bam/src/record/codec/decoder/bin.rs b/noodles-bam/src/record/codec/decoder/bin.rs new file mode 100644 index 000000000..5c0988c73 --- /dev/null +++ b/noodles-bam/src/record/codec/decoder/bin.rs @@ -0,0 +1,50 @@ +use std::{error, fmt, mem}; + +use bytes::Buf; + +/// An error when raw BAM record bin fail to parse. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum DecodeError { + /// Unexpected EOF. + UnexpectedEof, +} + +impl error::Error for DecodeError {} + +impl fmt::Display for DecodeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::UnexpectedEof => write!(f, "unexpected EOF"), + } + } +} + +pub(super) fn discard_bin(src: &mut B) -> Result<(), DecodeError> +where + B: Buf, +{ + if src.remaining() < mem::size_of::() { + return Err(DecodeError::UnexpectedEof); + } + + src.advance(mem::size_of::()); + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_discard_bin() { + let mut src = &[0x00, 0x00][..]; + assert!(discard_bin(&mut src).is_ok()); + + let mut src = &[][..]; + assert_eq!(discard_bin(&mut src), Err(DecodeError::UnexpectedEof)); + + let mut src = &[0x00][..]; + assert_eq!(discard_bin(&mut src), Err(DecodeError::UnexpectedEof)); + } +}