print better errors when deserializing json (#218)

- print stripped json whenever there is a type mismatch when
deserializing
- Revert "print stripped json whenever there is a type mismatch when
deserializing"
- print better errors when deserializing json
This commit is contained in:
Carson McManus 2023-06-24 12:06:12 -04:00 committed by GitHub
parent f080e35971
commit 2dc6376533
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 71 additions and 26 deletions

10
Cargo.lock generated
View file

@ -1840,6 +1840,15 @@ dependencies = [
"serde", "serde",
] ]
[[package]]
name = "serde_path_to_error"
version = "0.1.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f7f05c1d5476066defcdfacce1f52fc3cae3af1d3089727100c02ae92e5abbe0"
dependencies = [
"serde",
]
[[package]] [[package]]
name = "serde_urlencoded" name = "serde_urlencoded"
version = "0.7.1" version = "0.7.1"
@ -2089,6 +2098,7 @@ dependencies = [
"secrecy", "secrecy",
"serde", "serde",
"serde_json", "serde_json",
"serde_path_to_error",
"standback", "standback",
"stderrlog", "stderrlog",
"steamguard", "steamguard",

View file

@ -60,6 +60,7 @@ qrcode = { version = "0.12.0", optional = true }
gethostname = "0.4.3" gethostname = "0.4.3"
secrecy = { version = "0.8", features = ["serde"] } secrecy = { version = "0.8", features = ["serde"] }
zeroize = "^1.4.3" zeroize = "^1.4.3"
serde_path_to_error = "0.1.11"
[dev-dependencies] [dev-dependencies]
tempdir = "0.3" tempdir = "0.3"

View file

@ -53,7 +53,8 @@ impl AccountManager {
let mut reader = BufReader::new(file); let mut reader = BufReader::new(file);
let mut buffer = String::new(); let mut buffer = String::new();
reader.read_to_string(&mut buffer)?; reader.read_to_string(&mut buffer)?;
let manifest: Manifest = match serde_json::from_str(&buffer) { let mut deser = serde_json::Deserializer::from_str(&buffer);
let manifest: Manifest = match serde_path_to_error::deserialize(&mut deser) {
Ok(m) => m, Ok(m) => m,
Err(orig_err) => match serde_json::from_str::<SdaManifest>(&buffer) { Err(orig_err) => match serde_json::from_str::<SdaManifest>(&buffer) {
Ok(_) => return Err(ManifestLoadError::MigrationNeeded)?, Ok(_) => return Err(ManifestLoadError::MigrationNeeded)?,
@ -159,7 +160,8 @@ impl AccountManager {
let file = File::open(path)?; let file = File::open(path)?;
let reader = BufReader::new(file); let reader = BufReader::new(file);
let account: SteamGuardAccount = serde_json::from_reader(reader)?; let mut deser = serde_json::Deserializer::from_reader(reader);
let account: SteamGuardAccount = serde_path_to_error::deserialize(&mut deser)?;
ensure!( ensure!(
!self.account_exists(&account.account_name), !self.account_exists(&account.account_name),
"Account already exists in manifest, please remove it first." "Account already exists in manifest, please remove it first."
@ -361,12 +363,16 @@ impl EntryLoader<SteamGuardAccount> for ManifestEntry {
return Err(ManifestAccountLoadError::IncorrectPasskey); return Err(ManifestAccountLoadError::IncorrectPasskey);
} }
let s = std::str::from_utf8(&plaintext).unwrap(); let s = std::str::from_utf8(&plaintext).unwrap();
serde_json::from_str(s)? let mut deser = serde_json::Deserializer::from_str(s);
serde_path_to_error::deserialize(&mut deser)?
} }
(None, Some(_)) => { (None, Some(_)) => {
return Err(ManifestAccountLoadError::MissingPasskey); return Err(ManifestAccountLoadError::MissingPasskey);
} }
(_, None) => serde_json::from_reader(reader)?, (_, None) => {
let mut deser = serde_json::Deserializer::from_reader(reader);
serde_path_to_error::deserialize(&mut deser)?
}
}; };
Ok(account) Ok(account)
} }
@ -378,8 +384,8 @@ pub enum ManifestLoadError {
Missing(#[from] std::io::Error), Missing(#[from] std::io::Error),
#[error("Manifest needs to be migrated to the latest format.")] #[error("Manifest needs to be migrated to the latest format.")]
MigrationNeeded, MigrationNeeded,
#[error("Failed to deserialize the manifest.")] #[error("Failed to deserialize the manifest. {self:?}")]
DeserializationFailed(#[from] serde_json::Error), DeserializationFailed(#[from] serde_path_to_error::Error<serde_json::Error>),
#[error(transparent)] #[error(transparent)]
Unknown(#[from] anyhow::Error), Unknown(#[from] anyhow::Error),
} }
@ -394,8 +400,8 @@ pub enum ManifestAccountLoadError {
IncorrectPasskey, IncorrectPasskey,
#[error("Failed to decrypt account. {self:?}")] #[error("Failed to decrypt account. {self:?}")]
DecryptionFailed(#[from] crate::encryption::EntryEncryptionError), DecryptionFailed(#[from] crate::encryption::EntryEncryptionError),
#[error("Failed to deserialize the account.")] #[error("Failed to deserialize the account. {self:?}")]
DeserializationFailed(#[from] serde_json::Error), DeserializationFailed(#[from] serde_path_to_error::Error<serde_json::Error>),
#[error(transparent)] #[error(transparent)]
Unknown(#[from] anyhow::Error), Unknown(#[from] anyhow::Error),
} }

View file

@ -88,12 +88,16 @@ impl EntryLoader<SdaAccount> for SdaManifestEntry {
return Err(ManifestAccountLoadError::IncorrectPasskey); return Err(ManifestAccountLoadError::IncorrectPasskey);
} }
let s = std::str::from_utf8(&plaintext).unwrap(); let s = std::str::from_utf8(&plaintext).unwrap();
serde_json::from_str(s)? let mut deser = serde_json::Deserializer::from_str(s);
serde_path_to_error::deserialize(&mut deser)?
} }
(None, Some(_)) => { (None, Some(_)) => {
return Err(ManifestAccountLoadError::MissingPasskey); return Err(ManifestAccountLoadError::MissingPasskey);
} }
(_, None) => serde_json::from_reader(reader)?, (_, None) => {
let mut deser = serde_json::Deserializer::from_reader(reader);
serde_path_to_error::deserialize(&mut deser)?
}
}; };
Ok(account) Ok(account)
} }

View file

@ -1,7 +1,7 @@
use std::{fs::File, io::Read, path::Path}; use std::{fs::File, io::Read, path::Path};
use log::debug; use log::debug;
use serde::de::Error; use serde::{de::Error, Deserialize};
use steamguard::SteamGuardAccount; use steamguard::SteamGuardAccount;
use thiserror::Error; use thiserror::Error;
@ -82,7 +82,7 @@ pub(crate) enum MigrationError {
#[error("Passkey is required to decrypt manifest")] #[error("Passkey is required to decrypt manifest")]
MissingPasskey, MissingPasskey,
#[error("Failed to deserialize manifest: {0}")] #[error("Failed to deserialize manifest: {0}")]
ManifestDeserializeFailed(serde_json::Error), ManifestDeserializeFailed(serde_path_to_error::Error<serde_json::Error>),
#[error("IO error when upgrading manifest: {0}")] #[error("IO error when upgrading manifest: {0}")]
IoError(#[from] std::io::Error), IoError(#[from] std::io::Error),
#[error("An unexpected error occurred during manifest migration: {0}")] #[error("An unexpected error occurred during manifest migration: {0}")]
@ -180,20 +180,44 @@ impl From<MigratingManifest> for Manifest {
} }
} }
fn deserialize_manifest(text: String) -> Result<MigratingManifest, serde_json::Error> { #[derive(Deserialize)]
let json: serde_json::Value = serde_json::from_str(&text)?; struct JustVersion {
debug!("deserializing manifest: version {}", json["version"]); version: Option<u32>,
if json["version"] == 1 { }
let manifest: ManifestV1 = serde_json::from_str(&text)?;
fn deserialize_manifest(
text: String,
) -> Result<MigratingManifest, serde_path_to_error::Error<serde_json::Error>> {
let mut deser = serde_json::Deserializer::from_str(&text);
let version: JustVersion = serde_path_to_error::deserialize(&mut deser)?;
debug!("deserializing manifest: version {:?}", version.version);
let mut deser = serde_json::Deserializer::from_str(&text);
match version.version {
Some(1) => {
let manifest: ManifestV1 = serde_path_to_error::deserialize(&mut deser)?;
Ok(MigratingManifest::ManifestV1(manifest)) Ok(MigratingManifest::ManifestV1(manifest))
} else if json["version"] == serde_json::Value::Null { }
let manifest: SdaManifest = serde_json::from_str(&text)?; None => {
let manifest: SdaManifest = serde_path_to_error::deserialize(&mut deser)?;
Ok(MigratingManifest::Sda(manifest)) Ok(MigratingManifest::Sda(manifest))
} else { }
Err(serde_json::Error::custom(format!( _ => {
"Unknown manifest version: {}", // HACK: there's no way to construct the Path type, so we create it by forcing a deserialize error
json["version"]
))) #[derive(Debug)]
struct Dummy;
impl<'de> Deserialize<'de> for Dummy {
fn deserialize<D: serde::Deserializer<'de>>(_: D) -> Result<Self, D::Error> {
Err(D::Error::custom("Unknown manifest version".to_string()))
}
}
let mut deser = serde_json::Deserializer::from_str("");
let err = serde_path_to_error::deserialize::<_, Dummy>(&mut deser).unwrap_err();
Err(err)
}
} }
} }