add check that the subtype is valid for the partition type

Author: icewind1991

espflash/Cargo.toml

@@ -26,6 +26,7 @@ slip-codec =  "0.2.4"
 thiserror = "1.0.20"
 xmas-elf = "0.8.0"
 serde = { version = "1.0", features = ["derive"] }
+serde_plain = "1"
 toml = "0.5"
 directories-next = "2.0.0"
 color-eyre = "0.5"

espflash/src/error.rs

@@ -1,5 +1,6 @@
 use crate::flasher::Command;
 use crate::image_format::ImageFormatId;
+use crate::partition_table::{SubType, Type};
 use crate::Chip;
 use miette::{Diagnostic, SourceOffset, SourceSpan};
 use slip_codec::Error as SlipError;
@@ -276,6 +277,9 @@ pub enum PartitionTableError {
     #[error(transparent)]
     #[diagnostic(transparent)]
     Duplicate(#[from] DuplicatePartitionsError),
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    InvalidSubType(#[from] InvalidSubTypeError),
 }
 
 #[derive(Debug, Error, Diagnostic)]
@@ -384,6 +388,32 @@ impl DuplicatePartitionsError {
     }
 }
 
+#[derive(Debug, Error, Diagnostic)]
+#[error("Invalid subtype for type")]
+#[diagnostic(
+    code(espflash::partition_table::invalid_type),
+    help("'{}' supports the following subtypes: {}", self.ty, self.ty.subtype_hint())
+)]
+pub struct InvalidSubTypeError {
+    #[source_code]
+    source_code: String,
+    #[label("'{}' is not a valid subtype for '{}'", self.sub_type, self.ty)]
+    span: SourceSpan,
+    ty: Type,
+    sub_type: SubType,
+}
+
+impl InvalidSubTypeError {
+    pub fn new(source: &str, line: usize, ty: Type, sub_type: SubType) -> Self {
+        InvalidSubTypeError {
+            source_code: source.into(),
+            span: line_to_span(&source, line),
+            ty,
+            sub_type,
+        }
+    }
+}
+
 #[derive(Debug, Error)]
 #[error("{0}")]
 pub struct ElfError(&'static str);

espflash/src/partition_table.rs

@@ -1,17 +1,19 @@
-use md5::{Context, Digest};
-use regex::Regex;
-use serde::{Deserialize, Deserializer};
-
 use crate::error::{
-    CSVError, DuplicatePartitionsError, OverlappingPartitionsError, PartitionTableError,
+    CSVError, DuplicatePartitionsError, InvalidSubTypeError, OverlappingPartitionsError,
+    PartitionTableError,
 };
+use md5::{Context, Digest};
+use regex::Regex;
+use serde::{Deserialize, Deserializer, Serialize};
 use std::cmp::{max, min};
+use std::fmt::Write as _;
+use std::fmt::{Display, Formatter};
 use std::io::Write;
 
 const MAX_PARTITION_LENGTH: usize = 0xC00;
 const PARTITION_TABLE_SIZE: usize = 0x1000;
 
-#[derive(Copy, Clone, Debug, Deserialize)]
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq)]
 #[repr(u8)]
 #[allow(dead_code)]
 #[serde(rename_all = "lowercase")]
@@ -20,7 +22,46 @@ pub enum Type {
     Data = 0x01,
 }
 
-#[derive(Copy, Clone, Debug, Deserialize, PartialEq)]
+impl Type {
+    pub fn subtype_hint(&self) -> String {
+        match self {
+            Type::App => "'factory', 'ota_0' trough 'ota_15' and 'test'".into(),
+            Type::Data => {
+                let types = [
+                    DataType::Ota,
+                    DataType::Phy,
+                    DataType::Nvs,
+                    DataType::CoreDump,
+                    DataType::NvsKeys,
+                    DataType::EFuse,
+                    DataType::EspHttpd,
+                    DataType::Fat,
+                    DataType::Spiffs,
+                ];
+
+                let mut out = format!("'{}'", serde_plain::to_string(&types[0]).unwrap());
+                for ty in &types[1..types.len() - 2] {
+                    let ser = serde_plain::to_string(&ty).unwrap();
+                    write!(&mut out, ", '{}'", ser).unwrap();
+                }
+
+                let ser = serde_plain::to_string(&types[types.len() - 1]).unwrap();
+                write!(&mut out, " and '{}'", ser).unwrap();
+
+                out
+            }
+        }
+    }
+}
+
+impl Display for Type {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let ser = serde_plain::to_string(self).unwrap();
+        write!(f, "{}", ser)
+    }
+}
+
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq)]
 #[repr(u8)]
 #[allow(dead_code)]
 #[serde(rename_all = "lowercase")]
@@ -61,7 +102,7 @@ pub enum AppType {
     Test = 0x20,
 }
 
-#[derive(Copy, Clone, Debug, Deserialize, PartialEq)]
+#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq)]
 #[repr(u8)]
 #[allow(dead_code)]
 #[serde(rename_all = "lowercase")]
@@ -78,14 +119,25 @@ pub enum DataType {
     Spiffs = 0x82,
 }
 
-#[derive(Debug, Deserialize, PartialEq)]
+#[derive(Debug, Deserialize, PartialEq, Copy, Clone)]
 #[allow(dead_code)]
 #[serde(untagged)]
 pub enum SubType {
     App(AppType),
     Data(DataType),
 }
 
+impl Display for SubType {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let ser = match self {
+            SubType::App(sub) => serde_plain::to_string(sub),
+            SubType::Data(sub) => serde_plain::to_string(sub),
+        }
+        .unwrap();
+        write!(f, "{}", ser)
+    }
+}
+
 impl SubType {
     fn as_u8(&self) -> u8 {
         match self {
@@ -194,6 +246,24 @@ impl PartitionTable {
     }
 
     fn validate(&self, source: &str) -> Result<(), PartitionTableError> {
+        for partition in &self.partitions {
+            if let Some(line) = &partition.line {
+                let expected_type = match partition.sub_type {
+                    SubType::App(_) => Type::App,
+                    SubType::Data(_) => Type::Data,
+                };
+                if expected_type != partition.ty {
+                    return Err(InvalidSubTypeError::new(
+                        source,
+                        *line,
+                        partition.ty,
+                        partition.sub_type,
+                    )
+                    .into());
+                }
+            }
+        }
+
         for partition1 in &self.partitions {
             for partition2 in &self.partitions {
                 if let (Some(line1), Some(line2)) = (&partition1.line, &partition2.line) {