Skip to content

Commit

Permalink
add explicit error for if the header could not be read entirely, upda…
Browse files Browse the repository at this point in the history
…te changelog, add some test
  • Loading branch information
bmatthieu3 committed Jan 29, 2025
1 parent 87c6ca6 commit db73486
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 31 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
Features:

- BREAKING API change. Provide an iterator over the HDU list
- #16: Parsing of comments, history, continued, card as enum.
- BREAKING API change. `get` method on the Header object now takes a `&str` instead of a `[u8; 8]` slice.

Bugfixes:

Expand Down
25 changes: 12 additions & 13 deletions src/card.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ impl TryFrom<&CardBuf> for Card {
fn try_from(buf: &CardBuf) -> Result<Self, Self::Error> {
let kw = std::str::from_utf8(buf[..8].trim_ascii())?;
match kw {
"" => parse_empty_keyword_card(buf),
"COMMENT" => Ok(Card::Comment(parse_comment_text(&buf[8..])?)),
"HISTORY" => Ok(Card::History(parse_comment_text(&buf[8..])?)),
"" => Ok(parse_empty_keyword_card(buf)),
"COMMENT" => Ok(Card::Comment(parse_comment_text(&buf[8..]))),
"HISTORY" => Ok(Card::History(parse_comment_text(&buf[8..]))),
"CONTINUE" => parse_continuation(buf),
"XTENSION" => parse_extension(buf),
"END" => Ok(Card::End),
Expand Down Expand Up @@ -354,7 +354,7 @@ fn parse_continuation(buf: &[u8; 80]) -> Result<Card, Error> {
Ok(Card::Continuation { string, comment })
}

fn parse_comment_text(buf: &[u8]) -> Result<String, Error> {
fn parse_comment_text(buf: &[u8]) -> String {
let mut comment = String::new();
buf.iter()
.map(|b| match b {
Expand All @@ -365,18 +365,18 @@ fn parse_comment_text(buf: &[u8]) -> Result<String, Error> {
})
.for_each(|ch| comment.push(ch))
;
Ok(comment.trim_ascii_end().to_owned())
comment.trim_ascii_end().to_owned()
}

/// Returns a [Card::Comment] if the card contains text, else [Card::Space].
///
/// FITSv4, section 4.4.2.4. Commentary keywords, last two paragraphs.
fn parse_empty_keyword_card(buf: &[u8; 80]) -> Result<Card, Error> {
let c = parse_comment_text(&buf[8..])?;
fn parse_empty_keyword_card(buf: &[u8; 80]) -> Card {
let c = parse_comment_text(&buf[8..]);
if c.is_empty() {
Ok(Card::Space)
Card::Space
} else {
Ok(Card::Comment(c.to_owned()))
Card::Comment(c.to_owned())
}
}

Expand Down Expand Up @@ -766,17 +766,16 @@ mod tests {
}

#[test]
fn empty_keyword_card() -> Result<(),Error> {
fn empty_keyword_card() {

let r = b" empty header comment with an illegal \t tab and \n newline ";
assert_eq!(
parse_empty_keyword_card(r)?,
parse_empty_keyword_card(r),
Card::Comment("empty header comment with an illegal � tab and � newline".to_owned()),
);

let r = b" ";
assert_eq!(parse_empty_keyword_card(r)?, Card::Space);
Ok(())
assert_eq!(parse_empty_keyword_card(r), Card::Space);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/fits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ where
}
}

#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub struct HDU<X>
where
X: Xtension,
Expand Down
151 changes: 137 additions & 14 deletions src/hdu/header/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,31 +167,22 @@ where

/// Get the value of a card, returns `None` if the card is not
/// found or is not a value card.
pub fn get(&self, key: &Keyword) -> Option<&Value> {
let kw = String::from_utf8_lossy(key);
self.values.get(kw.trim())
}

/// Get the value for a value card, returns `None` if the keyword is not
/// found or if the card is not a value card.
pub fn value(&self, keyword: &str) -> Option<&Value> {
self.values.get(keyword)
pub fn get(&self, key: &str) -> Option<&Value> {
self.values.get(key)
}

/// Get the value a specific card and try to parse the value
/// Returns an error if the asking type does not match the true inner type of
/// the value got
/// # Params
/// * `key` - The key of a card
pub fn get_parsed<T>(&self, key: &[u8; 8]) -> Option<Result<T, Error>>
pub fn get_parsed<T>(&self, key: &str) -> Option<Result<T, Error>>
where
T: CardValue,
{
self.get(key).map(|card| {
let value = card.clone();
self.get(key).map(|value| {
<T as CardValue>::parse(value.clone()).map_err(|_| {
let card = String::from_utf8_lossy(key);
Error::FailTypeCardParsing(card.to_string(), std::any::type_name::<T>().to_string())
Error::FailTypeCardParsing(key.to_string(), std::any::type_name::<T>().to_string())
})
})
}
Expand Down Expand Up @@ -394,6 +385,138 @@ mod tests {
Ok(())
}

#[test]
fn end_card_not_found() {
let data = mock_fits_data([
b"SIMPLE = T / Standard FITS Format ",
b"BITPIX = 8 / Character data ",
b"NAXIS = 0 / No Image --- just extension(s) ",
b"EXTEND = T / There are standard extensions ",
]);
let reader = Cursor::new(data);
let mut fits = Fits::from_reader(reader);
let hdu = fits
.next()
.expect("Should contain a primary HDU");

assert_eq!(Err(Error::StaticError("END card not found")), hdu);
// As the primary hdu parsing failed (EOF reached), next call to fits should result in None
assert_eq!(fits.next(), None);
}

#[test]
fn blank_interpreted_as_comments() -> Result<(), Error> {
let data = mock_fits_data([
b"SIMPLE = T / Standard FITS Format ",
b"BITPIX = 8 / Character data ",
b"NAXIS = 0 / No Image --- just extension(s) ",
b"EXTEND = T / There are standard extensions ",
b"ORIGIN = 'xml2fits_v1.95' / Converted from XML-Astrores to FITS ",
b" e-mail: [email protected] ",
b"LONGSTRN= 'OGIP 1.0' / Long string convention (&/CONTINUE) may be used",
b"DATE = '2018-04-12' / Written on 2018-04-12:13:25:09 (GMT) ",
b" by: [email protected] ",
b" ********************************************************** ",
b" EXCERPT from catalogues stored in VizieR (CDS) ",
b" with the following conditions: ",
b" ********************************************************** ",
b" ",
b" VizieR Astronomical Server vizier.u-strasbg.fr ",
b" Date: 2018-04-12T13:25:09 [V1.99+ (14-Oct-2013)] ",
b" In case of problem, please report to: [email protected] ",
b" ",
b"INFO = 'votable-version=1.99+ (14-Oct-2013)' / # ",
b"INFO = '-ref=VIZ5acf5dfe7d66' / # ",
b"INFO = '-out.max=50' / # ",
b"END "
]);
let reader = Cursor::new(data);
let mut fits = Fits::from_reader(reader);
let hdu = fits
.next()
.expect("Should contain a primary HDU")
.unwrap()
;
assert!(matches!(hdu, HDU::Primary(_)));
if let HDU::Primary(hdu) = hdu {
let mut cards = hdu.get_header().cards();
assert_eq!(cards.next(), Some(&Card::Value {
name: "SIMPLE".to_owned(),
value: Value::Logical {
value: true,
comment: Some(" Standard FITS Format".to_owned())
}
}));
assert_eq!(dbg!(cards.next()), Some(&Card::Value {
name: "BITPIX".to_owned(),
value: Value::Integer {
value: 8,
comment: Some(" Character data".to_owned())
}
}));
assert_eq!(cards.next(), Some(&Card::Value {
name: "NAXIS".to_owned(),
value: Value::Integer {
value: 0,
comment: Some(" No Image --- just extension(s)".to_owned())
}
}));
assert_eq!(cards.next(), Some(&Card::Value {
name: "EXTEND".to_owned(),
value: Value::Logical {
value: true,
comment: Some(" There are standard extensions".to_owned())
}
}));
assert_eq!(cards.next(), Some(&Card::Value {
name: "ORIGIN".to_owned(),
value: Value::String {
value: "xml2fits_v1.95".to_owned(),
comment: Some(" Converted from XML-Astrores to FITS".to_owned())
}
}));
assert_eq!(cards.next(), Some(&Card::Comment(" e-mail: [email protected]".to_string())));
assert_eq!(cards.next(), Some(&Card::Value {
name: "LONGSTRN".to_owned(),
value: Value::String {
value: "OGIP 1.0".to_owned(),
comment: Some(" Long string convention (&/CONTINUE) may be used".to_owned())
}
}));

assert_eq!(cards.next(), Some(&Card::Value {
name: "DATE".to_owned(),
value: Value::String {
value: "2018-04-12".to_owned(),
comment: Some(" Written on 2018-04-12:13:25:09 (GMT)".to_owned())
}
}));
/*b" by: [email protected] ",
b" ********************************************************** ",
b" EXCERPT from catalogues stored in VizieR (CDS) ",
b" with the following conditions: ",
b" ********************************************************** ",
b" ",
b" VizieR Astronomical Server vizier.u-strasbg.fr ",
b" Date: 2018-04-12T13:25:09 [V1.99+ (14-Oct-2013)] ",
b" In case of problem, please report to: [email protected] ",
b" ",*/
assert_eq!(cards.next(), Some(&Card::Comment(" by: [email protected]".to_string())));
assert_eq!(cards.next(), Some(&Card::Comment("**********************************************************".to_string())));
assert_eq!(cards.next(), Some(&Card::Comment(" EXCERPT from catalogues stored in VizieR (CDS)".to_string())));
assert_eq!(cards.next(), Some(&Card::Comment(" with the following conditions:".to_string())));
assert_eq!(cards.next(), Some(&Card::Comment("**********************************************************".to_string())));

assert_eq!(cards.next(), Some(&Card::Space));

assert_eq!(cards.next(), Some(&Card::Comment("VizieR Astronomical Server vizier.u-strasbg.fr".to_string())));
assert_eq!(cards.next(), Some(&Card::Comment("Date: 2018-04-12T13:25:09 [V1.99+ (14-Oct-2013)]".to_string())));
assert_eq!(cards.next(), Some(&Card::Comment("In case of problem, please report to: [email protected]".to_string())));
}

Ok(())
}

/// panics if N > 36
fn mock_fits_data<const N: usize>(cards: [&CardBuf; N]) -> [u8; 2880] {
let mut data = [b' '; 2880];
Expand Down
11 changes: 8 additions & 3 deletions src/hdu/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::async_fits;
use crate::fits;
use crate::hdu::primary::consume_next_card;

#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum HDU {
Primary(fits::HDU<Image>),
XImage(fits::HDU<Image>),
Expand All @@ -47,7 +47,10 @@ where

/* Consume cards until `END` is reached */
loop {
consume_next_card(reader, &mut card_80_bytes_buf, num_bytes_read)?;
consume_next_card(reader, &mut card_80_bytes_buf, num_bytes_read)
// Precise the error that we did not encounter the END stopping card
.map_err(|_| Error::StaticError("Fail reading the header without encountering the END card"))?;

if let Ok(card) = Card::try_from(&card_80_bytes_buf) {
cards.push(card);
if Some(&Card::End) == cards.last() {
Expand All @@ -73,7 +76,9 @@ where

/* Consume cards until `END` is reached */
loop {
consume_next_card_async(reader, &mut card_80_bytes_buf, num_bytes_read).await?;
consume_next_card_async(reader, &mut card_80_bytes_buf, num_bytes_read).await
// Precise the error that we did not encounter the END stopping card
.map_err(|_| Error::StaticError("Fail reading the header without encountering the END card"))?;
if let Ok(card) = Card::try_from(&card_80_bytes_buf) {
cards.push(card);
if Some(&Card::End) == cards.last() {
Expand Down

0 comments on commit db73486

Please sign in to comment.