fix: Handle tags with sizes that exceed the file size (fixes #156)

* Invalid tags are returned with the Error::partial_tag field if possible
* Also refactor the StorageFile writer to be easier to debug

Author: polyfloyd

src/storage/plain.rs

@@ -147,24 +147,25 @@ impl<F: StorageFile> io::Write for PlainWriter<'_, F> {
                 let old_file_end = self.storage.file.seek(io::SeekFrom::End(0))?;
                 let old_region_end = self.storage.region.end;
                 let new_region_end = self.storage.region.start + buf_len;
-                let new_file_end = old_file_end - (old_region_end - new_region_end);
+                let new_file_end =
+                    self.storage.region.start + buf_len + (old_file_end - old_region_end);
 
                 let mut rwbuf = [0; COPY_BUF_SIZE];
                 let rwbuf_len = rwbuf.len();
                 for i in 0.. {
                     let from = old_region_end + i * rwbuf.len() as u64;
                     let to = new_region_end + i * rwbuf.len() as u64;
-                    assert!(from >= to);
+                    assert!(from > to);
 
-                    let part = (to + rwbuf_len as u64).saturating_sub(new_file_end);
-                    let rwbuf_part = &mut rwbuf[part as usize..];
+                    if from >= old_file_end {
+                        break;
+                    }
+                    let part = cmp::min(rwbuf_len as i64, (old_file_end - from) as i64);
+                    let rwbuf_part = &mut rwbuf[..part as usize];
                     self.storage.file.seek(io::SeekFrom::Start(from))?;
                     self.storage.file.read_exact(rwbuf_part)?;
                     self.storage.file.seek(io::SeekFrom::Start(to))?;
                     self.storage.file.write_all(rwbuf_part)?;
-                    if rwbuf_part.len() < rwbuf_len {
-                        break;
-                    }
                 }
 
                 self.storage.file.set_len(new_file_end)?;

src/stream/tag.rs

@@ -481,16 +481,26 @@ impl Default for Encoder {
 
 pub fn locate_id3v2(reader: impl io::Read + io::Seek) -> crate::Result<Range<u64>> {
     let mut reader = io::BufReader::new(reader);
+    let start = reader.stream_position()?;
 
-    let header = Header::decode(&mut reader)?;
+    let file_size = reader.seek(io::SeekFrom::End(0))?;
+    reader.seek(io::SeekFrom::Start(start))?;
 
+    let header = Header::decode(&mut reader)?;
     let tag_size = header.tag_size();
+
+    if start + tag_size >= file_size {
+        // Seen in the wild: tags that are encoded to be larger than the files actuall are.
+        reader.seek(io::SeekFrom::End(0))?;
+        return Ok(start..file_size);
+    }
+
     reader.seek(io::SeekFrom::Start(tag_size))?;
     let num_padding = reader
         .bytes()
         .take_while(|rs| rs.as_ref().map(|b| *b == 0x00).unwrap_or(false))
         .count();
-    Ok(0..tag_size + num_padding as u64)
+    Ok(start..tag_size + num_padding as u64)
 }
 
 #[cfg(test)]

src/tag.rs

@@ -759,6 +759,24 @@ mod tests {
         let _tag = Tag::read_from_path("testdata/github-issue-147.id3").unwrap();
     }
 
+    #[test]
+    fn github_issue_156a() {
+        let tmp = tempfile::NamedTempFile::new().unwrap();
+        std::fs::copy("testdata/github-issue-156a.id3", &tmp).unwrap();
+
+        let tag = crate::partial_tag_ok(Tag::read_from_path(&tmp)).unwrap();
+        tag.write_to_path(&tmp, Version::Id3v23).unwrap();
+    }
+
+    #[test]
+    fn github_issue_156b() {
+        let tmp = tempfile::NamedTempFile::new().unwrap();
+        std::fs::copy("testdata/github-issue-156b.id3", &tmp).unwrap();
+
+        let tag = Tag::read_from_path(&tmp).unwrap();
+        tag.write_to_path(&tmp, Version::Id3v23).unwrap();
+    }
+
     #[test]
     fn aiff_read_and_write() {
         // Copy