From 7e68883a27c59b770022484626730e5b89b9587e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 7 Oct 2021 09:23:35 -0400 Subject: [PATCH] Change tor-persist to use json instead of toml. The limitations with toml seemed to be reaching a head, and I wasn't able to refactor the guardmgr code enough to actually have its state be serializable as toml. Json's limitations are much narrower. --- Cargo.lock | 2 +- crates/tor-persist/Cargo.toml | 2 +- crates/tor-persist/src/fs.rs | 17 ++++++++--------- crates/tor-persist/src/lib.rs | 16 +++++++++------- crates/tor-persist/src/testing.rs | 7 ++++--- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c2bc1554..6f66e1d02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2684,9 +2684,9 @@ dependencies = [ "fslock", "sanitize-filename", "serde", + "serde_json", "tempfile", "thiserror", - "toml", ] [[package]] diff --git a/crates/tor-persist/Cargo.toml b/crates/tor-persist/Cargo.toml index 29785f8c8..05a19f546 100644 --- a/crates/tor-persist/Cargo.toml +++ b/crates/tor-persist/Cargo.toml @@ -15,7 +15,7 @@ testing = [] [dependencies] serde = { version = "1.0.124" } -toml = "0.5.8" +serde_json = "1.0.59" sanitize-filename = "0.3.0" thiserror = "1.0.24" diff --git a/crates/tor-persist/src/fs.rs b/crates/tor-persist/src/fs.rs index 31bfbdcf3..9c591dfa1 100644 --- a/crates/tor-persist/src/fs.rs +++ b/crates/tor-persist/src/fs.rs @@ -1,4 +1,4 @@ -//! Filesystem + Toml implementation of StateMgr. +//! Filesystem + JSON implementation of StateMgr. use crate::{Error, Result, StateMgr}; use serde::{de::DeserializeOwned, Serialize}; @@ -8,7 +8,7 @@ use std::sync::{Arc, Mutex}; #[cfg(target_family = "unix")] use std::os::unix::fs::DirBuilderExt; -/// Implementation of StateMgr that stores state as Toml files on disk. +/// Implementation of StateMgr that stores state as JSON files on disk. /// /// # Locking /// @@ -21,10 +21,9 @@ use std::os::unix::fs::DirBuilderExt; /// /// # Limitations /// -/// 1) This manager only accepts objects that can be serialized as Toml -/// documents. Some types (like strings or lists) serialize to Toml -/// types that cannot appear at the head of a document. You'll be -/// able to store them, but reloading them later on will fail. +/// 1) This manager only accepts objects that can be serialized as +/// JSON documents. Some types (like maps with non-string keys) can't +/// be serialized as JSON. /// /// 2) This manager normalizes keys to an fs-safe format before saving /// data with them. This keeps you from accidentally creating or @@ -81,7 +80,7 @@ impl FsStateMgr { fn filename(&self, key: &str) -> PathBuf { self.inner .statepath - .join(sanitize_filename::sanitize(key) + ".toml") + .join(sanitize_filename::sanitize(key) + ".json") } } @@ -123,7 +122,7 @@ impl StateMgr for FsStateMgr { } }; - Ok(Some(toml::from_str(&string)?)) + Ok(Some(serde_json::from_str(&string)?)) } fn store(&self, key: &str, val: &S) -> Result<()> @@ -136,7 +135,7 @@ impl StateMgr for FsStateMgr { let fname = self.filename(key); - let output = toml::ser::to_string(val)?; + let output = serde_json::to_string_pretty(val)?; let fname_tmp = fname.with_extension("tmp"); std::fs::write(&fname_tmp, (&output).as_bytes())?; diff --git a/crates/tor-persist/src/lib.rs b/crates/tor-persist/src/lib.rs index 6572754ce..7f270f78d 100644 --- a/crates/tor-persist/src/lib.rs +++ b/crates/tor-persist/src/lib.rs @@ -113,13 +113,9 @@ pub enum Error { #[error("Storage not locked")] NoLock, - /// Unable to serialize data as TOML. - #[error("Toml serialization error")] - TomlWriteError(#[from] toml::ser::Error), - - /// Unable to deserialize data from TOML - #[error("Toml deserialization error")] - TomlReadError(#[from] toml::de::Error), + /// Problem when serializing or deserializing JSON data. + #[error("JSON serialization error")] + JsonError(#[source] Arc), } impl From for Error { @@ -127,3 +123,9 @@ impl From for Error { Error::IoError(Arc::new(e)) } } + +impl From for Error { + fn from(e: serde_json::Error) -> Error { + Error::JsonError(Arc::new(e)) + } +} diff --git a/crates/tor-persist/src/testing.rs b/crates/tor-persist/src/testing.rs index 05afcbbb6..6c01f88c1 100644 --- a/crates/tor-persist/src/testing.rs +++ b/crates/tor-persist/src/testing.rs @@ -24,7 +24,7 @@ struct TestingStateMgrInner { /// True if we are pretending that someone else is holding the /// simulated write lock on the state. lock_blocked: bool, - /// Map from key to toml-encoded values. + /// Map from key to JSON-encoded values. /// /// We serialize our values here for conveinence (so that we don't /// have to use `Any`) and to try to detect any @@ -66,7 +66,7 @@ impl StateMgr for TestingStateMgr { { let inner = self.inner.lock().expect("Lock poisoned."); match inner.entries.get(key) { - Some(value) => Ok(Some(toml::from_str(value)?)), + Some(value) => Ok(Some(serde_json::from_str(value)?)), None => Ok(None), } } @@ -80,7 +80,8 @@ impl StateMgr for TestingStateMgr { return Err(Error::NoLock); } - let val = toml::ser::to_string(val)?; + let val = serde_json::to_string_pretty(val)?; + inner.entries.insert(key.to_string(), val); Ok(()) }