chiark / gitweb /
~ianmdlvl / otter.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 59cf669)
item names: Launder properly
authorIan Jackson <ijackson@chiark.greenend.org.uk>
Mon, 29 Mar 2021 21:25:54 +0000 (22:25 +0100)
committerIan Jackson <ijackson@chiark.greenend.org.uk>
Mon, 29 Mar 2021 21:25:54 +0000 (22:25 +0100)
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
daemon/cmdlistener.rs patch | blob | history
src/bin/otter.rs patch | blob | history
src/bin/otterlib.rs patch | blob | history
src/commands.rs patch | blob | history
src/shapelib.rs patch | blob | history

diff --git a/daemon/cmdlistener.rs b/daemon/cmdlistener.rs
index bc6c5603ecd7acd0f7a589a227dde26570bd660b..5ef042a04ac26869df4083bddfb8d7944c77cf2e 100644 (file)
--- a/daemon/cmdlistener.rs
+++ b/daemon/cmdlistener.rs
@@ -453,6 +453,9 @@ fn execute_game_insn<'cs, 'igr, 'ig: 'igr>(
           } else {
             "occulted-item".to_string()
           };
+          let itemname = itemname.try_into().map_err(
+            |e| internal_error_bydebug(&e)
+          )?;
           then {
             Some(MgmtGamePieceInfo {
               piece,
diff --git a/src/bin/otter.rs b/src/bin/otter.rs
index a15fcf88ce43875b88cf97aa93c4c76513ad3c9c..9d0e750181b269cd5f11399505fa2df22313f0da 100644 (file)
--- a/src/bin/otter.rs
+++ b/src/bin/otter.rs
@@ -1056,7 +1056,7 @@ mod library_add {
     let args = parse_args::<Args,_>(args, &subargs, &ok_id, None);
     let mut chan = access_game(&ma, &args.table_name)?;
     let (pieces, _pcaliases) = chan.list_pieces()?;
-    let markers = pieces.iter().filter(|p| p.itemname == MAGIC)
+    let markers = pieces.iter().filter(|p| p.itemname.as_str() == MAGIC)
       .collect::<Vec<_>>();
 
     let already = if args.incremental { Some(
@@ -1242,7 +1242,7 @@ mod library_add {
       };
       let spec = shapelib::ItemSpec {
         lib: args.tlg.pat.lib.clone(),
-        item: it.itemname.clone(),
+        item: it.itemname.as_str().to_owned(),
       };
       let spec = PiecesSpec {
         pos: Some(pos),
diff --git a/src/bin/otterlib.rs b/src/bin/otterlib.rs
index 340739cb389ed571ff1455a84e5ba68bc5b43b30..3c2f674bd3721c665925efe5a2f79b1c57b6bd37 100644 (file)
--- a/src/bin/otterlib.rs
+++ b/src/bin/otterlib.rs
@@ -90,7 +90,7 @@ fn preview(items: Vec<ItemForOutput>) {
   }
 
   let mut pieces: Vec<Prep> = items.into_iter().map(|it| {
-    let spec = ItemSpec { lib: it.0, item: it.1.itemname };
+    let spec = ItemSpec { lib: it.0, item: it.1.itemname.into() };
     (||{
       let (p, _occultable) = spec.clone().find_load(&pcaliases)
         .context("load")?;
diff --git a/src/commands.rs b/src/commands.rs
index d62f8a540b96bf0f570415861935eb5d2acb4fab..8af875d952d250df834f522231b012dbc8292d51 100644 (file)
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -151,7 +151,7 @@ pub struct MgmtPlayerInfo {
 #[derive(Debug,Clone,Serialize,Deserialize)]
 pub struct MgmtGamePieceInfo {
   pub piece: PieceId,
-  pub itemname: String,
+  pub itemname: shapelib::GoodItemName,
   #[serde(flatten)]
   pub visible: Option<MgmtGamePieceVisibleInfo>,
 }
diff --git a/src/shapelib.rs b/src/shapelib.rs
index 45e80710ebe97d9f43b112ef78f3297afbaaa3aa..1908bdbc665d47692572096483de7be705c69a21 100644 (file)
--- a/src/shapelib.rs
+++ b/src/shapelib.rs
@@ -33,7 +33,7 @@ pub trait OutlineDefn: Debug + Sync + Send + 'static {
 pub struct Contents {
   libname: String,
   dirname: String,
-  items: HashMap<String /* item (name) */, ItemData>,
+  items: HashMap<GoodItemName, ItemData>,
 }
 
 #[derive(Debug,Clone)]
@@ -67,6 +67,44 @@ struct OccData_Internal {
   svgd: parking_lot::Mutex<Option<Arc<Html>>>,
 }
 
+mod item_name {
+  use super::*;
+
+  #[derive(Clone,Debug,Eq,PartialEq,Ord,PartialOrd,Hash)]
+  #[derive(Serialize,Deserialize)]
+  #[serde(transparent)]
+  pub struct GoodItemName(String);
+  impl Borrow<String> for GoodItemName {
+    fn borrow(&self) -> &String { &self.0 }
+  }
+  impl GoodItemName {
+    pub fn as_str(&self) -> &str { &self.0 }
+  }
+  impl From<GoodItemName> for String {
+    fn from(i: GoodItemName) -> String { i.0 }
+  }
+  impl Display for GoodItemName {
+    #[throws(fmt::Error)]
+    fn fmt(&self, f: &mut fmt::Formatter) { f.write_str(&self.0)? }
+  }
+
+  impl TryFrom<String> for GoodItemName {
+    type Error = LibraryLoadError;
+    #[throws(LibraryLoadError)]
+    fn try_from(s: String) -> GoodItemName {
+      if s.as_bytes().iter().all(|&c:&u8| (
+        c.is_ascii_alphanumeric() ||
+        b"-_. ()".contains(&c)
+      )) {
+        GoodItemName(s)
+      } else {
+        throw!(LLE::BadItemName(s))
+      }
+    }
+  }
+}
+pub use item_name::*;
+
 #[derive(Error,Debug)]
 pub enum LibraryLoadError {
   #[error(transparent)]
@@ -109,6 +147,8 @@ pub enum LibraryLoadError {
   MultipleMultipleFaceDefinitions,
   #[error("{0}")]
   UnsupportedColourSpec(#[from] UnsupportedColourSpec),
+  #[error("bad item name (invalid characters) in {0:?}")]
+  BadItemName(String),
 }
 
 impl LibraryLoadError {
@@ -191,14 +231,14 @@ impl OccultedPieceTrait for ItemOccultable {
 
 #[derive(Debug,Clone,Serialize,Deserialize,Eq,PartialEq,Ord,PartialOrd)]
 pub struct ItemEnquiryData {
-  pub itemname: String,
+  pub itemname: GoodItemName,
   pub f0desc: Html,
   pub f0bbox: Rect,
 }
 
 impl ItemEnquiryData {
   pub fn line_for_list(&self) -> String {
-    format!("{:20}  {}", self.itemname, self.f0desc.0)
+    format!("{:20}  {}", self.itemname.as_str(), self.f0desc.0)
   }
 }
 
@@ -450,15 +490,15 @@ impl Contents {
       pat: pat.to_string(), msg: pe.msg.to_string() })?;
     let mut out = vec![];
     for (k,v) in &self.items {
-      if !pat.matches(&k) { continue }
-      let (loaded, _) = match self.load1(v, &k, &default()) {
+      if !pat.matches(k.as_str()) { continue }
+      let (loaded, _) = match self.load1(v, k.as_str(), &default()) {
         Err(SpecError::LibraryItemNotFound(_)) => continue,
         e@ Err(_) => e?,
         Ok(r) => r,
       };
       let f0bbox = loaded.bbox_approx()?;
       let ier = ItemEnquiryData {
-        itemname: k.clone(),
+        itemname: k.to_owned(),
         f0bbox,
         f0desc: loaded.describe_face(default())?,
       };
@@ -634,6 +674,7 @@ fn load_catalogue(libname: &str, dirname: &str, toml_path: &str) -> Contents {
 
       let item_name = format!("{}{}{}", gdefn.item_prefix,
                               fe.item_spec, gdefn.item_suffix);
+      let item_name: GoodItemName = item_name.try_into()?;
 
       let occ = match &group.d.occulted {
         None => OccData::None,
@@ -643,7 +684,7 @@ fn load_catalogue(libname: &str, dirname: &str, toml_path: &str) -> Contents {
           }
           let colour: Colour = colour.try_into()?;
           OccData::Internal(Arc::new(OccData_Internal {
-            item_name: Arc::new(subst(&item_name, "_c", &colour.0)?),
+            item_name: Arc::new(subst(item_name.as_str(), "_c", &colour.0)?),
             desc: Html(subst(&fe.desc.0, "_colour", "")?),
             outline: outline.clone(),
             xform: FaceTransform::from_group(&group.d)?,
@@ -658,7 +699,7 @@ fn load_catalogue(libname: &str, dirname: &str, toml_path: &str) -> Contents {
         },
       };
 
-      let mut add1 = |item_name: &str, desc: Html| {
+      let mut add1 = |item_name: &GoodItemName, desc: Html| {
         let desc = if let Some(desc_template) = &group.d.desc_template {
           Html(subst(desc_template, "_desc", &desc.0)?)
         } else {
@@ -671,14 +712,14 @@ fn load_catalogue(libname: &str, dirname: &str, toml_path: &str) -> Contents {
           d: Arc::new(ItemDetails { desc }),
         };
         type H<'e,X,Y> = hash_map::Entry<'e,X,Y>;
-        match l.items.entry(item_name.to_owned()) {
+        match l.items.entry(item_name.clone()) {
           H::Occupied(oe) => throw!(LLE::DuplicateItem {
-            item: item_name.to_owned(),
+            item: item_name.as_str().to_owned(),
             group1: oe.get().group.groupname.clone(),
             group2: groupname.clone(),
           }),
           H::Vacant(ve) => {
-            debug!("loaded shape {} {}", libname, item_name);
+            debug!("loaded shape {} {}", libname, item_name.as_str());
             ve.insert(idata);
           }
         };
@@ -689,9 +730,9 @@ fn load_catalogue(libname: &str, dirname: &str, toml_path: &str) -> Contents {
         add1(&item_name, fe.desc.clone())?;
       } else {
         for (colour, recolourdata) in &group.d.colours {
-          let t_item_name = subst(&item_name, "_c", &recolourdata.abbrev)?;
+          let t_item_name = subst(item_name.as_str(), "_c", &recolourdata.abbrev)?;
           let t_desc = Html(subst(&fe.desc.0, "_colour", colour)?);
-          add1(&t_item_name, t_desc)?;
+          add1(&t_item_name.try_into()?, t_desc)?;
         }
 
       }
Unnamed repository; edit this file 'description' to name the repository.