add validation that partitions don't overlap

Author: icewind1991

espflash/src/error.rs

@@ -1,7 +1,6 @@
 use crate::flasher::Command;
 use crate::image_format::ImageFormatId;
 use crate::Chip;
-use csv::Position;
 use miette::{Diagnostic, SourceOffset, SourceSpan};
 use slip_codec::Error as SlipError;
 use std::fmt::{Display, Formatter};
@@ -266,15 +265,25 @@ impl<T> ResultExt for Result<T, Error> {
     }
 }
 
+#[derive(Debug, Error, Diagnostic)]
+pub enum PartitionTableError {
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    CSV(#[from] CSVError),
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    Overlapping(#[from] OverlappingPartitionsError),
+}
+
 #[derive(Debug, Error, Diagnostic)]
 #[error("Malformed partition table")]
 #[diagnostic(
-    code(espflash::mallformed_partition_table),
+    code(espflash::partition_table::mallformed),
     help("See the espressif documentation for information on the partition table format:
 
 https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/partition-tables.html#creating-custom-tables")
 )]
-pub struct PartitionTableError {
+pub struct CSVError {
     #[source_code]
     source: String,
     #[label("{}", self.hint)]
@@ -284,12 +293,12 @@ pub struct PartitionTableError {
     error: csv::Error,
 }
 
-impl PartitionTableError {
+impl CSVError {
     pub fn new(error: csv::Error, source: String) -> Self {
-        let err_pos = match error.kind() {
-            csv::ErrorKind::Deserialize { pos: Some(pos), .. } => pos.clone(),
-            csv::ErrorKind::UnequalLengths { pos: Some(pos), .. } => pos.clone(),
-            _ => Position::new(),
+        let err_line = match error.kind() {
+            csv::ErrorKind::Deserialize { pos: Some(pos), .. } => pos.line(),
+            csv::ErrorKind::UnequalLengths { pos: Some(pos), .. } => pos.line(),
+            _ => 0,
         };
         let hint = match error.kind() {
             csv::ErrorKind::Deserialize { err, .. } => err.to_string(),
@@ -302,16 +311,9 @@ impl PartitionTableError {
             _ => String::new(),
         };
 
-        // since csv doesn't give us the position in the line the error occurs, we highlight the entire line
-        let line_length = source
-            .lines()
-            .nth(err_pos.line() as usize - 1)
-            .unwrap()
-            .len()
-            .into();
-        let err_span = SourceSpan::new(pos_to_offset(err_pos), line_length);
+        let err_span = line_to_span(&source, err_line as usize);
 
-        PartitionTableError {
+        CSVError {
             source,
             err_span,
             hint,
@@ -320,8 +322,34 @@ impl PartitionTableError {
     }
 }
 
-fn pos_to_offset(pos: Position) -> SourceOffset {
-    (pos.byte() as usize).into()
+/// since csv doesn't give us the position in the line the error occurs, we highlight the entire line
+///
+/// line starts at 1
+fn line_to_span(source: &str, line: usize) -> SourceSpan {
+    let line_length = source.lines().nth(line - 1).unwrap().len().into();
+    SourceSpan::new(SourceOffset::from_location(source, line, 2), line_length)
+}
+
+#[derive(Debug, Error, Diagnostic)]
+#[error("Overlapping partitions")]
+#[diagnostic(code(espflash::partition_table::overlapping))]
+pub struct OverlappingPartitionsError {
+    #[source_code]
+    source_code: String,
+    #[label("This partition")]
+    partition1_span: SourceSpan,
+    #[label("Overlaps with this partition")]
+    partition2_span: SourceSpan,
+}
+
+impl OverlappingPartitionsError {
+    pub fn new(source: &str, partition1_line: usize, partition2_line: usize) -> Self {
+        OverlappingPartitionsError {
+            source_code: source.into(),
+            partition1_span: line_to_span(&source, partition1_line),
+            partition2_span: line_to_span(&source, partition2_line),
+        }
+    }
 }
 
 #[derive(Debug, Error)]

espflash/src/partition_table.rs

@@ -2,12 +2,12 @@ use md5::{Context, Digest};
 use regex::Regex;
 use serde::{Deserialize, Deserializer};
 
-use crate::error::PartitionTableError;
+use crate::error::{CSVError, OverlappingPartitionsError, PartitionTableError};
+use std::cmp::{max, min};
 use std::io::Write;
 
 const MAX_PARTITION_LENGTH: usize = 0xC00;
 const PARTITION_TABLE_SIZE: usize = 0x1000;
-const MAX_PARTITION_TABLE_ENTRIES: usize = 95;
 
 #[derive(Copy, Clone, Debug, Deserialize)]
 #[repr(u8)]
@@ -147,14 +147,20 @@ impl PartitionTable {
             .trim(csv::Trim::All)
             .from_reader(data.trim().as_bytes());
 
-        let mut partitions = Vec::with_capacity(MAX_PARTITION_TABLE_ENTRIES);
-        for partition in reader.deserialize() {
-            let partition: Partition =
-                partition.map_err(|e| PartitionTableError::new(e, data.clone()))?;
+        let mut partitions = Vec::with_capacity(data.lines().count());
+        for record in reader.records() {
+            let record = record.map_err(|e| CSVError::new(e, data.clone()))?;
+            let position = record.position();
+            let mut partition: Partition = record
+                .deserialize(None)
+                .map_err(|e| CSVError::new(e, data.clone()))?;
+            partition.line = position.map(|pos| pos.line() as usize);
             partitions.push(partition);
         }
 
-        Ok(Self { partitions })
+        let table = Self { partitions };
+        table.validate(&data)?;
+        Ok(table)
     }
 
     pub fn to_bytes(&self) -> Vec<u8> {
@@ -184,6 +190,20 @@ impl PartitionTable {
 
         Ok(())
     }
+
+    fn validate(&self, source: &str) -> Result<(), PartitionTableError> {
+        for partition1 in &self.partitions {
+            for partition2 in &self.partitions {
+                if let (Some(line1), Some(line2)) = (&partition1.line, &partition2.line) {
+                    if line1 != line2 && partition1.overlaps(partition2) {
+                        return Err(OverlappingPartitionsError::new(source, *line1, *line2).into());
+                    }
+                }
+            }
+        }
+
+        Ok(())
+    }
 }
 
 const PARTITION_SIZE: usize = 32;
@@ -199,6 +219,8 @@ struct Partition {
     #[serde(deserialize_with = "deserialize_partition_offset_or_size")]
     size: u32,
     flags: Option<u32>,
+    #[serde(skip)]
+    line: Option<usize>,
 }
 
 impl Partition {
@@ -219,6 +241,7 @@ impl Partition {
             offset,
             size,
             flags,
+            line: None,
         }
     }
 
@@ -242,6 +265,10 @@ impl Partition {
 
         Ok(())
     }
+
+    fn overlaps(&self, other: &Partition) -> bool {
+        max(self.offset, other.offset) < min(self.offset + self.size, other.offset + other.size)
+    }
 }
 
 fn deserialize_partition_name<'de, D>(deserializer: D) -> Result<String, D::Error>