Improved errors

3 files changed, 148 insertions, 76 deletions

diff --git a/src/binary.rs b/src/binary.rs
index d7a51fc..0640d4f 100644
--- a/src/binary.rs
+++ b/src/binary.rs
@@ -3,9 +3,8 @@ use encoding::all::UTF_16BE;
 use encoding::types::{DecoderTrap, Encoding};
 use itertools::Interleave;
 use std::io::{Read, Seek, SeekFrom};
-use std::io::Result as IoResult;
 
-use super::PlistEvent;
+use super::{ParserError, ParserResult, PlistEvent};
 
 struct StackItem {
 	object_refs: Vec<u64>,
@@ -28,23 +27,16 @@ pub struct StreamingParser<R> {
 }
 
 impl<R: Read+Seek> StreamingParser<R> {
-	pub fn open(reader: R) -> Result<StreamingParser<R>, ()> {
-		let mut parser = StreamingParser {
+	pub fn new(reader: R) -> StreamingParser<R> {
+		StreamingParser {
 			stack: Vec::new(),
 			object_offsets: Vec::new(),
 			reader: reader,
 			ref_size: 0
-		};
-		
-		match parser.read_trailer() {
-			Ok(_) => (),
-			Err(_) => return Err(())
 		}
-		
-		Ok(parser)
 	}
 
-	fn read_trailer(&mut self) -> IoResult<()> {
+	fn read_trailer(&mut self) -> ParserResult<()> {
 		try!(self.reader.seek(SeekFrom::Start(0)));
 		let mut magic = [0; 8];
 		try!(self.reader.read(&mut magic));
@@ -72,28 +64,27 @@ impl<R: Read+Seek> StreamingParser<R> {
 		Ok(())
 	}
 
-	fn read_ints(&mut self, len: u64, size: u8) -> IoResult<Vec<u64>> {
+	fn read_ints(&mut self, len: u64, size: u8) -> ParserResult<Vec<u64>> {
 		let mut ints = Vec::with_capacity(len as usize);
 		// TODO: Is the match hoisted out of the loop?
-		// Is this even right wrt 1,2,4,8 etc. Should it be 0,1,2...
 		for _ in 0..len {
 			match size {
 				1 => ints.push(try!(self.reader.read_u8()) as u64),
 				2 => ints.push(try!(self.reader.read_u16::<BigEndian>()) as u64),
 				4 => ints.push(try!(self.reader.read_u32::<BigEndian>()) as u64),
 				8 => ints.push(try!(self.reader.read_u64::<BigEndian>()) as u64),
-				_ => panic!("wtf")
+				_ => return Err(ParserError::InvalidData)
 			}
 		}
 		Ok(ints)
 	}
 
-	fn read_refs(&mut self, len: u64) -> IoResult<Vec<u64>> {
+	fn read_refs(&mut self, len: u64) -> ParserResult<Vec<u64>> {
 		let ref_size = self.ref_size;
 		self.read_ints(len, ref_size)
 	}
 
-	fn read_object_len(&mut self, len: u8) -> IoResult<u64> {
+	fn read_object_len(&mut self, len: u8) -> ParserResult<u64> {
 		if (len & 0xf) == 0xf {
 			let len_power_of_two = try!(self.reader.read_u8()) & 0x3;
 			Ok(match len_power_of_two {
@@ -101,28 +92,40 @@ impl<R: Read+Seek> StreamingParser<R> {
 				1 => try!(self.reader.read_u16::<BigEndian>()) as u64,
 				2 => try!(self.reader.read_u32::<BigEndian>()) as u64,
 				3 => try!(self.reader.read_u64::<BigEndian>()),
-				_ => panic!("wrong len {}", len_power_of_two)
+				_ => return Err(ParserError::InvalidData)
 			})
 		} else {
 			Ok(len as u64)
 		}
 	}
 
-	fn read_data(&mut self, len: u64) -> IoResult<Vec<u8>> {
+	fn read_data(&mut self, len: u64) -> ParserResult<Vec<u8>> {
 		let mut data = vec![0; len as usize];
-		let read_len = try!(self.reader.read(&mut data));
-		if (read_len as u64) != len { panic!("not enough read") }
+		let mut total_read = 0usize;
+
+		while (total_read as u64) < len {
+			let read = try!(self.reader.read(&mut data[total_read..]));
+			if read == 0 {
+				return Err(ParserError::UnexpectedEof);
+			}
+			total_read += read;
+		}
+
 		Ok(data)
 	}
 
-	fn seek_to_object(&mut self, object_ref: u64) -> IoResult<u64> {
-		// TODO: Better ways to deal with this cast?
-		// I geuss not store the table locally if it's huge
+	fn seek_to_object(&mut self, object_ref: u64) -> ParserResult<u64> {
 		let offset = *&self.object_offsets[object_ref as usize];
-		self.reader.seek(SeekFrom::Start(offset))
+		let pos = try!(self.reader.seek(SeekFrom::Start(offset)));
+		Ok(pos)
 	}
 
-	fn read_next(&mut self) -> IoResult<Option<PlistEvent>> {
+	fn read_next(&mut self) -> ParserResult<Option<PlistEvent>> {
+		// Initialise here rather than in new
+		if self.ref_size == 0 {
+			try!(self.read_trailer());
+		}
+
 		let object_ref = match self.stack.last_mut() {
 			Some(stack_item) => stack_item.object_refs.pop(),
 			// Reached the end of the plist
@@ -149,21 +152,21 @@ impl<R: Read+Seek> StreamingParser<R> {
 		let size = token & 0x0f;
 
 		let result = match (ty, size) {
-			(0x0, 0x00) => panic!("null"),
+			(0x0, 0x00) => return Err(ParserError::UnsupportedType), // null
 			(0x0, 0x08) => Some(PlistEvent::BooleanValue(false)),
 			(0x0, 0x09) => Some(PlistEvent::BooleanValue(true)),
-			(0x0, 0x0f) => panic!("fill"),
+			(0x0, 0x0f) => return Err(ParserError::UnsupportedType), // fill
 			(0x1, 0) => Some(PlistEvent::IntegerValue(try!(self.reader.read_u8()) as i64)),
 			(0x1, 1) => Some(PlistEvent::IntegerValue(try!(self.reader.read_u16::<BigEndian>()) as i64)),
 			(0x1, 2) => Some(PlistEvent::IntegerValue(try!(self.reader.read_u32::<BigEndian>()) as i64)),
 			(0x1, 3) => Some(PlistEvent::IntegerValue(try!(self.reader.read_i64::<BigEndian>()))),
-			(0x1, 4) => panic!("128 bit int"),
-			(0x1, _) => panic!("variable length int"),
+			(0x1, 4) => return Err(ParserError::UnsupportedType), // 128 bit int
+			(0x1, _) => return Err(ParserError::UnsupportedType), // variable length int
 			(0x2, 2) => Some(PlistEvent::RealValue(try!(self.reader.read_f32::<BigEndian>()) as f64)),
 			(0x2, 3) => Some(PlistEvent::RealValue(try!(self.reader.read_f64::<BigEndian>()))),
-			(0x2, _) => panic!("odd length float"),
-			(0x3, 3) => panic!("date"),
-			(0x4, n) => { // data
+			(0x2, _) => return Err(ParserError::UnsupportedType), // odd length float
+			(0x3, 3) => return Err(ParserError::UnsupportedType), // date
+			(0x4, n) => { // Data
 				let len = try!(self.read_object_len(n));
 				Some(PlistEvent::DataValue(try!(self.read_data(len))))
 			},
@@ -208,26 +211,24 @@ impl<R: Read+Seek> StreamingParser<R> {
 
 				Some(PlistEvent::StartDictionary)
 			},
-			(_, _) => panic!("unsupported type")
+			(_, _) => return Err(ParserError::InvalidData)
 		};
 
 		Ok(result)
 	}
 }
 
-
 impl<R: Read+Seek> Iterator for StreamingParser<R> {
 	type Item = PlistEvent;
 
 	fn next(&mut self) -> Option<PlistEvent> {
 		match self.read_next() {
 			Ok(result) => result,
-			Err(_) => Some(PlistEvent::Error(()))
+			Err(err) => Some(PlistEvent::Error(err))
 		}
 	}
 }
 
-
 #[cfg(test)]
 mod tests {
 	use std::fs::File;
@@ -241,7 +242,7 @@ mod tests {
 		use super::super::PlistEvent::*;
 
 		let reader = File::open(&Path::new("./tests/data/binary.plist")).unwrap();
-		let streaming_parser = StreamingParser::open(reader).unwrap();
+		let streaming_parser = StreamingParser::new(reader);
 		let events: Vec<PlistEvent> = streaming_parser.collect();
 
 		let comparison = &[
diff --git a/src/lib.rs b/src/lib.rs
index 1613e53..2d0bc79 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -7,8 +7,10 @@ extern crate xml as xml_rs;
 pub mod binary;
 pub mod xml;
 
+use byteorder::Error as ByteorderError;
 use std::collections::HashMap;
-use std::io::{Read, Seek};
+use std::io::{Read, Seek, SeekFrom};
+use std::io::Error as IoError;
 
 #[derive(Clone, Debug, PartialEq)]
 pub enum Plist {
@@ -22,7 +24,7 @@ pub enum Plist {
 	String(String)
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Debug, PartialEq)]
 pub enum PlistEvent {
 	StartArray,
 	EndArray,
@@ -37,7 +39,39 @@ pub enum PlistEvent {
 	RealValue(f64),
 	StringValue(String),
 
-	Error(())
+	Error(ParserError)
+}
+
+type ParserResult<T> = Result<T, ParserError>;
+
+#[derive(Debug)]
+pub enum ParserError {
+	InvalidData,
+	UnexpectedEof,
+	UnsupportedType,
+	Io(IoError)
+}
+
+// No two errors are the same - this is a bit annoying though
+impl PartialEq for ParserError {
+	fn eq(&self, other: &ParserError) -> bool {
+		false
+	}
+}
+
+impl From<IoError> for ParserError {
+	fn from(io_error: IoError) -> ParserError {
+		ParserError::Io(io_error)
+	}
+}
+
+impl From<ByteorderError> for ParserError {
+	fn from(err: ByteorderError) -> ParserError {
+		match err {
+			ByteorderError::UnexpectedEOF => ParserError::UnexpectedEof,
+			ByteorderError::Io(err) => ParserError::Io(err)
+		}
+	}
 }
 
 pub enum StreamingParser<R: Read+Seek> {
@@ -46,8 +80,23 @@ pub enum StreamingParser<R: Read+Seek> {
 }
 
 impl<R: Read+Seek> StreamingParser<R> {
-	pub fn new() -> StreamingParser<R> {
-		panic!()
+	pub fn new(mut reader: R) -> StreamingParser<R> {
+		match StreamingParser::is_binary(&mut reader) {
+			Ok(true) => StreamingParser::Binary(binary::StreamingParser::new(reader)),
+			Ok(false) | Err(_) => StreamingParser::Xml(xml::StreamingParser::new(reader))
+		}
+	}
+
+	fn is_binary(reader: &mut R) -> Result<bool, IoError> {
+		try!(reader.seek(SeekFrom::Start(0)));
+		let mut magic = [0; 8];
+		try!(reader.read(&mut magic));
+
+		Ok(if &magic == b"bplist00" {
+			true
+		} else {
+			false
+		})
 	}
 }
 
@@ -62,41 +111,57 @@ impl<R: Read+Seek> Iterator for StreamingParser<R> {
 	}
 }
 
-pub struct Parser<T> {
-	reader: T,
+pub type BuilderResult<T> = Result<T, BuilderError>;
+
+#[derive(Debug, PartialEq)]
+pub enum BuilderError {
+	InvalidEvent,
+	UnsupportedDictionaryKey,
+	ParserError(ParserError)
+}
+
+impl From<ParserError> for BuilderError {
+	fn from(err: ParserError) -> BuilderError {
+		BuilderError::ParserError(err)
+	}
+}
+
+pub struct Builder<T> {
+	stream: T,
 	token: Option<PlistEvent>,
 }
 
-impl<R: Read + Seek> Parser<StreamingParser<R>> {
-	pub fn new(reader: R) -> Parser<StreamingParser<R>> {
-		Parser::from_event_stream(StreamingParser::Xml(xml::StreamingParser::new(reader)))
+impl<R: Read + Seek> Builder<StreamingParser<R>> {
+	pub fn new(reader: R) -> Builder<StreamingParser<R>> {
+		Builder::from_event_stream(StreamingParser::new(reader))
 	}
 }
 
-impl<T:Iterator<Item=PlistEvent>> Parser<T> {
-	pub fn from_event_stream(stream: T) -> Parser<T> {
-		Parser {
-			reader: stream,
+impl<T:Iterator<Item=PlistEvent>> Builder<T> {
+	pub fn from_event_stream(stream: T) -> Builder<T> {
+		Builder {
+			stream: stream,
 			token: None
 		}
 	}
 
-	pub fn parse(mut self) -> Result<Plist, ()> {
+	pub fn build(mut self) -> BuilderResult<Plist> {
 		self.bump();
 		let plist = try!(self.build_value());
 		self.bump();
 		match self.token {
 			None => (),
-			_ => return Err(())
+			// The stream should have finished
+			_ => return Err(BuilderError::InvalidEvent)
 		};
 		Ok(plist)
 	}
 
 	fn bump(&mut self) {
-		self.token = self.reader.next();
+		self.token = self.stream.next();
 	}
 
-	fn build_value(&mut self) -> Result<Plist, ()> {
+	fn build_value(&mut self) -> BuilderResult<Plist> {
 		match self.token.take() {
 			Some(PlistEvent::StartArray) => Ok(Plist::Array(try!(self.build_array()))),
 			Some(PlistEvent::StartDictionary) => Ok(Plist::Dictionary(try!(self.build_dict()))),
@@ -108,14 +173,15 @@ impl<T:Iterator<Item=PlistEvent>> Parser<T> {
 			Some(PlistEvent::RealValue(f)) => Ok(Plist::Real(f)),
 			Some(PlistEvent::StringValue(s)) => Ok(Plist::String(s)),
 
-			Some(PlistEvent::EndArray) => Err(()),
-			Some(PlistEvent::EndDictionary) => Err(()),
-			Some(PlistEvent::Error(_)) => Err(()),
-			None => Err(())
+			Some(PlistEvent::EndArray) => Err(BuilderError::InvalidEvent),
+			Some(PlistEvent::EndDictionary) => Err(BuilderError::InvalidEvent),
+			Some(PlistEvent::Error(_)) => Err(BuilderError::InvalidEvent),
+			// The stream should not have ended here
+			None => Err(BuilderError::InvalidEvent)
 		}
 	}
 
-	fn build_array(&mut self) -> Result<Vec<Plist>, ()> {	
+	fn build_array(&mut self) -> Result<Vec<Plist>, BuilderError> {	
 		let mut values = Vec::new();
 
 		loop {
@@ -128,7 +194,7 @@ impl<T:Iterator<Item=PlistEvent>> Parser<T> {
 		}
 	}
 
-	fn build_dict(&mut self) -> Result<HashMap<String, Plist>, ()> {
+	fn build_dict(&mut self) -> Result<HashMap<String, Plist>, BuilderError> {
 		let mut values = HashMap::new();
 
 		loop {
@@ -142,7 +208,7 @@ impl<T:Iterator<Item=PlistEvent>> Parser<T> {
 				},
 				_ => {
 					// Only string keys are supported in plists
-					return Err(())
+					return Err(BuilderError::UnsupportedDictionaryKey)
 				}
 			}
 		}
@@ -156,12 +222,12 @@ mod tests {
 	use super::*;
 
 	#[test]
-	fn parser() {
+	fn builder() {
 		use super::PlistEvent::*;
 
 		// Input
 
-		let events = &[
+		let events = vec![
 			StartDictionary,
 			StringValue("Author".to_owned()),
 			StringValue("William Shakespeare".to_owned()),
@@ -177,8 +243,8 @@ mod tests {
 			EndDictionary
 		];
 
-		let parser = Parser::from_event_stream(events.into_iter().cloned());
-		let plist = parser.parse();
+		let builder = Builder::from_event_stream(events.into_iter());
+		let plist = builder.build();
 
 		// Expected output
 
diff --git a/src/xml.rs b/src/xml.rs
index cb2a339..a15db70 100644
--- a/src/xml.rs
+++ b/src/xml.rs
@@ -4,7 +4,7 @@ use std::str::FromStr;
 use xml_rs::reader::{EventReader, ParserConfig};
 use xml_rs::reader::events::XmlEvent;
 
-use super::PlistEvent;
+use super::{ParserError, PlistEvent};
 
 pub struct StreamingParser<R: Read> {
 	xml_reader: EventReader<R>,
@@ -30,7 +30,7 @@ impl<R: Read> StreamingParser<R> {
 	fn read_content<F>(&mut self, f: F) -> PlistEvent where F:FnOnce(String) -> PlistEvent {
 		match self.xml_reader.next() {
 			XmlEvent::Characters(s) => f(s),
-			_ => PlistEvent::Error(())
+			_ => PlistEvent::Error(ParserError::InvalidData)
 		}
 	}
 }
@@ -55,32 +55,32 @@ impl<R: Read> Iterator for StreamingParser<R> {
 						"data" => return Some(self.read_content(|s| {
 							match FromBase64::from_base64(&s[..]) {
 								Ok(b) => PlistEvent::DataValue(b),
-								Err(_) => PlistEvent::Error(())
+								Err(_) => PlistEvent::Error(ParserError::InvalidData)
 							}
 						})),
 						"date" => return Some(self.read_content(|s| PlistEvent::DateValue(s))),
 						"integer" => return Some(self.read_content(|s| {
 							match FromStr::from_str(&s)	{
 								Ok(i) => PlistEvent::IntegerValue(i),
-								Err(_) => PlistEvent::Error(())
+								Err(_) => PlistEvent::Error(ParserError::InvalidData)
 							}
 						})),
 						"real" => return Some(self.read_content(|s| {
 							match FromStr::from_str(&s)	{
 								Ok(f) => PlistEvent::RealValue(f),
-								Err(_) => PlistEvent::Error(())
+								Err(_) => PlistEvent::Error(ParserError::InvalidData)
 							}
 						})),
 						"string" => return Some(self.read_content(|s| PlistEvent::StringValue(s))),
-						_ => return Some(PlistEvent::Error(()))
+						_ => return Some(PlistEvent::Error(ParserError::InvalidData))
 					}
 				},
 				XmlEvent::EndElement { name, .. } => {
 					// Check the corrent element is being closed
 					match self.element_stack.pop() {
 						Some(ref open_name) if &name.local_name == open_name => (),
-						Some(ref open_name) => return Some(PlistEvent::Error(())),
-						None => return Some(PlistEvent::Error(()))
+						Some(ref open_name) => return Some(PlistEvent::Error(ParserError::InvalidData)),
+						None => return Some(PlistEvent::Error(ParserError::InvalidData))
 					}
 
 					match &name.local_name[..] {
@@ -89,7 +89,12 @@ impl<R: Read> Iterator for StreamingParser<R> {
 						_ => ()
 					}
 				},
-				XmlEvent::EndDocument => return None,
+				XmlEvent::EndDocument => {
+					match self.element_stack.is_empty() {
+						true => return None,
+						false => return Some(PlistEvent::Error(ParserError::UnexpectedEof))
+					}
+				}
 				_ => ()
 			}
 		}