From c6f1bad941a1551140ba284d233dde9fc65c491b Mon Sep 17 00:00:00 2001 From: Igor von Nyssen Date: Mon, 19 Jan 2026 12:07:58 -0800 Subject: [PATCH 1/6] return InvalidValue if usize index gets a negative value --- src/server/params.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/server/params.rs b/src/server/params.rs index 380e695..df2662a 100644 --- a/src/server/params.rs +++ b/src/server/params.rs @@ -31,8 +31,20 @@ where &mut self, name: &'static str, ) -> super::Result> { - self.0 - .swap_remove(name.as_ref()) + let value_opt = self.0.swap_remove(name.as_ref()); + + // Specialization: indices should return InvalidValue for negatives. + if let Some(ref value) = value_opt + && (TypeId::of::() == TypeId::of::()) + && value.trim_start().starts_with('-') + { + return Err(crate::ASCOMError::invalid_value(format!( + "Parameter {name:?} must be non-negative, got: {value}" + )) + .into()); + } + + value_opt .map(|mut value| { // Specialization: optimized path to avoid cloning the string. if TypeId::of::() == TypeId::of::() { From 952f5272f65dc312ce2dbf8cd859603826199103 Mon Sep 17 00:00:00 2001 From: Igor von Nyssen Date: Mon, 19 Jan 2026 19:57:16 -0800 Subject: [PATCH 2/6] Custom deserialier now treats invalid integers as OK, with ASCOM Error --- src/server/error.rs | 10 +- src/server/params.rs | 282 ++++++++++++++++++++++++++++++++++++----- src/server/response.rs | 7 + 3 files changed, 268 insertions(+), 31 deletions(-) diff --git a/src/server/error.rs b/src/server/error.rs index 24282ce..526eb1a 100644 --- a/src/server/error.rs +++ b/src/server/error.rs @@ -24,6 +24,12 @@ pub(crate) enum Error { #[source] err: serde_plain::Error, }, + #[error("Parameter {name:?} value {value} is out of range for {target_type}")] + ParameterOutOfRange { + name: &'static str, + value: i32, + target_type: &'static str, + }, #[error(transparent)] Ascom(#[from] ASCOMError), } @@ -32,7 +38,9 @@ impl IntoResponse for Error { fn into_response(self) -> Response { let code = match self { Self::UnknownDeviceNumber { .. } | Self::UnknownAction { .. } => StatusCode::NOT_FOUND, - Self::MissingParameter { .. } | Self::BadParameter { .. } => StatusCode::BAD_REQUEST, + Self::MissingParameter { .. } + | Self::BadParameter { .. } + | Self::ParameterOutOfRange { .. } => StatusCode::BAD_REQUEST, Self::Ascom(_) => StatusCode::INTERNAL_SERVER_ERROR, }; (code, format!("{self:#}")).into_response() diff --git a/src/server/params.rs b/src/server/params.rs index df2662a..cce90d0 100644 --- a/src/server/params.rs +++ b/src/server/params.rs @@ -6,11 +6,244 @@ use axum::response::{IntoResponse, Response}; use http::{Method, StatusCode}; use indexmap::IndexMap; use serde::Deserialize; -use serde::de::{DeserializeOwned, IntoDeserializer}; -use std::any::TypeId; -use std::fmt::Debug; +use serde::de::{DeserializeOwned, Deserializer, Error as _, Visitor}; +use std::fmt::{self, Debug}; use std::hash::Hash; +/// Error type for ALPACA parameter parsing that distinguishes parse errors from range errors. +#[derive(Debug)] +enum AlpacaParseError { + /// Invalid format (not a valid integer/bool/etc) -> `BadParameter` (HTTP 400) + BadFormat(String), + /// Valid i32 but out of range for target type -> `INVALID_VALUE` (ASCOM error) + OutOfRange { value: i32, target_type: &'static str }, +} + +impl fmt::Display for AlpacaParseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::BadFormat(msg) => write!(f, "{msg}"), + Self::OutOfRange { value, target_type } => { + write!(f, "value {value} is out of range for {target_type}") + } + } + } +} + +impl std::error::Error for AlpacaParseError {} + +impl serde::de::Error for AlpacaParseError { + fn custom(msg: T) -> Self { + Self::BadFormat(msg.to_string()) + } +} + +/// Custom serde Deserializer for ALPACA parameters. +/// +/// Per ASCOM spec, integers are transmitted as i32, so we parse as i32 first +/// then convert to the target type. This allows distinguishing parse errors +/// (`BadParameter`) from range errors (`INVALID_VALUE`). +struct AlpacaDeserializer { + value: String, +} + +impl AlpacaDeserializer { + /// Parse an integer per ALPACA spec: parse as i32, then convert to target type. + fn parse_integer>(&self) -> Result { + let i32_value: i32 = self.value.trim().parse().map_err(|_parse_err| { + AlpacaParseError::BadFormat(format!("invalid integer: {}", self.value)) + })?; + + T::try_from(i32_value).map_err(|_range_err| AlpacaParseError::OutOfRange { + value: i32_value, + target_type: std::any::type_name::(), + }) + } +} + +impl<'de> Deserializer<'de> for AlpacaDeserializer { + type Error = AlpacaParseError; + + fn deserialize_any>(self, visitor: V) -> Result { + visitor.visit_string(self.value) + } + + fn deserialize_bool>(self, visitor: V) -> Result { + match self.value.trim().to_ascii_lowercase().as_str() { + "true" => visitor.visit_bool(true), + "false" => visitor.visit_bool(false), + _ => Err(AlpacaParseError::BadFormat(format!( + "invalid boolean: {}", + self.value + ))), + } + } + + fn deserialize_i8>(self, visitor: V) -> Result { + visitor.visit_i8(self.parse_integer()?) + } + + fn deserialize_i16>(self, visitor: V) -> Result { + visitor.visit_i16(self.parse_integer()?) + } + + fn deserialize_i32>(self, visitor: V) -> Result { + visitor.visit_i32(self.parse_integer()?) + } + + fn deserialize_i64>(self, visitor: V) -> Result { + visitor.visit_i64(self.parse_integer()?) + } + + fn deserialize_u8>(self, visitor: V) -> Result { + visitor.visit_u8(self.parse_integer()?) + } + + fn deserialize_u16>(self, visitor: V) -> Result { + visitor.visit_u16(self.parse_integer()?) + } + + fn deserialize_u32>(self, visitor: V) -> Result { + visitor.visit_u32(self.parse_integer()?) + } + + fn deserialize_u64>(self, visitor: V) -> Result { + visitor.visit_u64(self.parse_integer()?) + } + + fn deserialize_f32>(self, visitor: V) -> Result { + let value: f32 = self.value.trim().parse().map_err(|_parse_err| { + AlpacaParseError::BadFormat(format!("invalid float: {}", self.value)) + })?; + visitor.visit_f32(value) + } + + fn deserialize_f64>(self, visitor: V) -> Result { + let value: f64 = self.value.trim().parse().map_err(|_parse_err| { + AlpacaParseError::BadFormat(format!("invalid float: {}", self.value)) + })?; + visitor.visit_f64(value) + } + + fn deserialize_char>(self, visitor: V) -> Result { + let mut chars = self.value.chars(); + match (chars.next(), chars.next()) { + (Some(c), None) => visitor.visit_char(c), + _ => Err(AlpacaParseError::BadFormat(format!( + "expected single character: {}", + self.value + ))), + } + } + + fn deserialize_bytes>(self, visitor: V) -> Result { + visitor.visit_bytes(self.value.as_bytes()) + } + + fn deserialize_byte_buf>(self, visitor: V) -> Result { + visitor.visit_byte_buf(self.value.into_bytes()) + } + + fn deserialize_str>(self, visitor: V) -> Result { + visitor.visit_string(self.value) + } + + fn deserialize_string>(self, visitor: V) -> Result { + visitor.visit_string(self.value) + } + + fn deserialize_newtype_struct>( + self, + _name: &'static str, + visitor: V, + ) -> Result { + visitor.visit_newtype_struct(self) + } + + fn deserialize_enum>( + self, + _name: &'static str, + _variants: &'static [&'static str], + visitor: V, + ) -> Result { + visitor.visit_enum(serde::de::value::StringDeserializer::new(self.value)) + } + + fn deserialize_identifier>(self, visitor: V) -> Result { + visitor.visit_string(self.value) + } + + fn deserialize_ignored_any>(self, visitor: V) -> Result { + visitor.visit_string(self.value) + } + + fn deserialize_option>(self, _visitor: V) -> Result { + Err(AlpacaParseError::BadFormat( + "Option types are not supported as ALPACA parameters".to_owned(), + )) + } + + fn deserialize_unit>(self, _visitor: V) -> Result { + Err(AlpacaParseError::BadFormat( + "unit type is not supported as ALPACA parameter".to_owned(), + )) + } + + fn deserialize_unit_struct>( + self, + _name: &'static str, + _visitor: V, + ) -> Result { + Err(AlpacaParseError::BadFormat( + "unit struct is not supported as ALPACA parameter".to_owned(), + )) + } + + fn deserialize_seq>(self, _visitor: V) -> Result { + Err(AlpacaParseError::BadFormat( + "sequences are not supported as ALPACA parameters".to_owned(), + )) + } + + fn deserialize_tuple>( + self, + _len: usize, + _visitor: V, + ) -> Result { + Err(AlpacaParseError::BadFormat( + "tuples are not supported as ALPACA parameters".to_owned(), + )) + } + + fn deserialize_tuple_struct>( + self, + _name: &'static str, + _len: usize, + _visitor: V, + ) -> Result { + Err(AlpacaParseError::BadFormat( + "tuple structs are not supported as ALPACA parameters".to_owned(), + )) + } + + fn deserialize_map>(self, _visitor: V) -> Result { + Err(AlpacaParseError::BadFormat( + "maps are not supported as ALPACA parameters".to_owned(), + )) + } + + fn deserialize_struct>( + self, + _name: &'static str, + _fields: &'static [&'static str], + _visitor: V, + ) -> Result { + Err(AlpacaParseError::BadFormat( + "structs are not supported as ALPACA parameters".to_owned(), + )) + } +} + #[derive(Deserialize, derive_more::Debug)] #[debug("{_0:?}")] #[serde(transparent)] @@ -27,40 +260,29 @@ impl OpaqueParams where str: AsRef, { - pub(crate) fn maybe_extract( + pub(crate) fn maybe_extract( &mut self, name: &'static str, ) -> super::Result> { - let value_opt = self.0.swap_remove(name.as_ref()); - - // Specialization: indices should return InvalidValue for negatives. - if let Some(ref value) = value_opt - && (TypeId::of::() == TypeId::of::()) - && value.trim_start().starts_with('-') - { - return Err(crate::ASCOMError::invalid_value(format!( - "Parameter {name:?} must be non-negative, got: {value}" - )) - .into()); - } - - value_opt - .map(|mut value| { - // Specialization: optimized path to avoid cloning the string. - if TypeId::of::() == TypeId::of::() { - return T::deserialize(value.into_deserializer()); - } - // Specialization: booleans in ASCOM must be case-insensitive. - if TypeId::of::() == TypeId::of::() { - value.make_ascii_lowercase(); - } - serde_plain::from_str(&value) + self.0 + .swap_remove(name.as_ref()) + .map(|value| { + T::deserialize(AlpacaDeserializer { value }).map_err(|err| match err { + AlpacaParseError::OutOfRange { value, target_type } => Error::ParameterOutOfRange { + name, + value, + target_type, + }, + AlpacaParseError::BadFormat(msg) => Error::BadParameter { + name, + err: serde_plain::Error::custom(msg), + }, + }) }) .transpose() - .map_err(|err| Error::BadParameter { name, err }) } - pub(crate) fn extract( + pub(crate) fn extract( &mut self, name: &'static str, ) -> super::Result { diff --git a/src/server/response.rs b/src/server/response.rs index 227848c..2d2eed1 100644 --- a/src/server/response.rs +++ b/src/server/response.rs @@ -52,6 +52,13 @@ where match self.response { Ok(response) => Ok(Ok(response)), Err(Error::Ascom(err)) => Ok(Err(err)), + Err(Error::ParameterOutOfRange { + name, + value, + target_type, + }) => Ok(Err(ASCOMError::invalid_value(format!( + "Parameter {name:?} value {value} is out of range for {target_type}" + )))), Err(err @ (Error::MissingParameter { .. } | Error::BadParameter { .. })) => { Err((StatusCode::BAD_REQUEST, err.to_string())) } From 327b980af479e0946ee9ea6e8c0185854e70e5e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Apr 2026 23:32:15 +0000 Subject: [PATCH 3/6] Widen integer parsing from i32 to i64 to support uint32 parameters Co-Authored-By: Claude Opus 4.6 --- src/server/error.rs | 2 +- src/server/params.rs | 115 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 106 insertions(+), 11 deletions(-) diff --git a/src/server/error.rs b/src/server/error.rs index 526eb1a..0cf1f05 100644 --- a/src/server/error.rs +++ b/src/server/error.rs @@ -27,7 +27,7 @@ pub(crate) enum Error { #[error("Parameter {name:?} value {value} is out of range for {target_type}")] ParameterOutOfRange { name: &'static str, - value: i32, + value: i64, target_type: &'static str, }, #[error(transparent)] diff --git a/src/server/params.rs b/src/server/params.rs index cce90d0..59bc3a0 100644 --- a/src/server/params.rs +++ b/src/server/params.rs @@ -15,8 +15,8 @@ use std::hash::Hash; enum AlpacaParseError { /// Invalid format (not a valid integer/bool/etc) -> `BadParameter` (HTTP 400) BadFormat(String), - /// Valid i32 but out of range for target type -> `INVALID_VALUE` (ASCOM error) - OutOfRange { value: i32, target_type: &'static str }, + /// Valid integer but out of range for target type -> `INVALID_VALUE` (ASCOM error) + OutOfRange { value: i64, target_type: &'static str }, } impl fmt::Display for AlpacaParseError { @@ -40,22 +40,23 @@ impl serde::de::Error for AlpacaParseError { /// Custom serde Deserializer for ALPACA parameters. /// -/// Per ASCOM spec, integers are transmitted as i32, so we parse as i32 first -/// then convert to the target type. This allows distinguishing parse errors -/// (`BadParameter`) from range errors (`INVALID_VALUE`). +/// Integers are parsed as i64 first, then converted to the target type. +/// This allows distinguishing parse errors (`BadParameter`) from range errors +/// (`INVALID_VALUE`), and supports both i32 device parameters and uint32 +/// transaction/identity parameters (`ClientID`, `ClientTransactionID`). struct AlpacaDeserializer { value: String, } impl AlpacaDeserializer { - /// Parse an integer per ALPACA spec: parse as i32, then convert to target type. - fn parse_integer>(&self) -> Result { - let i32_value: i32 = self.value.trim().parse().map_err(|_parse_err| { + /// Parse an integer: parse as i64 first, then convert to the target type. + fn parse_integer>(&self) -> Result { + let i64_value: i64 = self.value.trim().parse().map_err(|_parse_err| { AlpacaParseError::BadFormat(format!("invalid integer: {}", self.value)) })?; - T::try_from(i32_value).map_err(|_range_err| AlpacaParseError::OutOfRange { - value: i32_value, + T::try_from(i64_value).map_err(|_range_err| AlpacaParseError::OutOfRange { + value: i64_value, target_type: std::any::type_name::(), }) } @@ -327,3 +328,97 @@ impl FromRequest for ActionParams { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(value: &str) -> super::super::Result { + let deser = AlpacaDeserializer { + value: value.to_owned(), + }; + T::deserialize(deser).map_err(|err| match err { + AlpacaParseError::OutOfRange { value, target_type } => Error::ParameterOutOfRange { + name: "test", + value, + target_type, + }, + AlpacaParseError::BadFormat(msg) => Error::BadParameter { + name: "test", + err: serde_plain::Error::custom(msg), + }, + }) + } + + #[test] + fn u32_above_i32_max() { + let val: u32 = parse("3000000000").expect("should parse u32 above i32::MAX"); + assert_eq!(val, 3_000_000_000_u32); + } + + #[test] + fn u32_max_value() { + let val: u32 = parse("4294967295").expect("should parse u32::MAX"); + assert_eq!(val, u32::MAX); + } + + #[test] + fn i32_positive() { + let val: i32 = parse("42").expect("should parse positive i32"); + assert_eq!(val, 42_i32); + } + + #[test] + fn i32_negative() { + let val: i32 = parse("-1").expect("should parse negative i32"); + assert_eq!(val, -1_i32); + } + + #[test] + fn i32_max_boundary() { + let val: i32 = parse("2147483647").expect("should parse i32::MAX"); + assert_eq!(val, i32::MAX); + } + + #[test] + fn i32_out_of_range() { + let err = parse::("3000000000").expect_err("should fail for i32 out of range"); + assert!( + matches!(err, Error::ParameterOutOfRange { .. }), + "expected OutOfRange, got: {err:?}" + ); + } + + #[test] + fn u32_negative_out_of_range() { + let err = parse::("-1").expect_err("should fail for negative u32"); + assert!( + matches!(err, Error::ParameterOutOfRange { .. }), + "expected OutOfRange, got: {err:?}" + ); + } + + #[test] + fn u8_out_of_range() { + let err = parse::("256").expect_err("should fail for u8 out of range"); + assert!( + matches!(err, Error::ParameterOutOfRange { .. }), + "expected OutOfRange, got: {err:?}" + ); + } + + #[test] + fn bad_format_non_numeric() { + let err = parse::("abc").expect_err("should fail for non-numeric input"); + assert!( + matches!(err, Error::BadParameter { .. }), + "expected BadParameter, got: {err:?}" + ); + } + + #[test] + fn whitespace_trimming() { + let val: u32 = parse(" 42 ").expect("should parse with surrounding whitespace"); + assert_eq!(val, 42_u32); + } +} From 28bba71e37fa5876e2eb28f8b1584fd8cbb2e82e Mon Sep 17 00:00:00 2001 From: Igor von Nyssen Date: Mon, 20 Apr 2026 17:42:52 -0700 Subject: [PATCH 4/6] Rename ALPACA to Alpaca in params.rs Alpaca is a proper noun, not an acronym. Co-Authored-By: Claude Opus 4.7 --- src/server/params.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/server/params.rs b/src/server/params.rs index 59bc3a0..868f00b 100644 --- a/src/server/params.rs +++ b/src/server/params.rs @@ -10,7 +10,7 @@ use serde::de::{DeserializeOwned, Deserializer, Error as _, Visitor}; use std::fmt::{self, Debug}; use std::hash::Hash; -/// Error type for ALPACA parameter parsing that distinguishes parse errors from range errors. +/// Error type for Alpaca parameter parsing that distinguishes parse errors from range errors. #[derive(Debug)] enum AlpacaParseError { /// Invalid format (not a valid integer/bool/etc) -> `BadParameter` (HTTP 400) @@ -38,7 +38,7 @@ impl serde::de::Error for AlpacaParseError { } } -/// Custom serde Deserializer for ALPACA parameters. +/// Custom serde Deserializer for Alpaca parameters. /// /// Integers are parsed as i64 first, then converted to the target type. /// This allows distinguishing parse errors (`BadParameter`) from range errors @@ -180,13 +180,13 @@ impl<'de> Deserializer<'de> for AlpacaDeserializer { fn deserialize_option>(self, _visitor: V) -> Result { Err(AlpacaParseError::BadFormat( - "Option types are not supported as ALPACA parameters".to_owned(), + "Option types are not supported as Alpaca parameters".to_owned(), )) } fn deserialize_unit>(self, _visitor: V) -> Result { Err(AlpacaParseError::BadFormat( - "unit type is not supported as ALPACA parameter".to_owned(), + "unit type is not supported as Alpaca parameter".to_owned(), )) } @@ -196,13 +196,13 @@ impl<'de> Deserializer<'de> for AlpacaDeserializer { _visitor: V, ) -> Result { Err(AlpacaParseError::BadFormat( - "unit struct is not supported as ALPACA parameter".to_owned(), + "unit struct is not supported as Alpaca parameter".to_owned(), )) } fn deserialize_seq>(self, _visitor: V) -> Result { Err(AlpacaParseError::BadFormat( - "sequences are not supported as ALPACA parameters".to_owned(), + "sequences are not supported as Alpaca parameters".to_owned(), )) } @@ -212,7 +212,7 @@ impl<'de> Deserializer<'de> for AlpacaDeserializer { _visitor: V, ) -> Result { Err(AlpacaParseError::BadFormat( - "tuples are not supported as ALPACA parameters".to_owned(), + "tuples are not supported as Alpaca parameters".to_owned(), )) } @@ -223,13 +223,13 @@ impl<'de> Deserializer<'de> for AlpacaDeserializer { _visitor: V, ) -> Result { Err(AlpacaParseError::BadFormat( - "tuple structs are not supported as ALPACA parameters".to_owned(), + "tuple structs are not supported as Alpaca parameters".to_owned(), )) } fn deserialize_map>(self, _visitor: V) -> Result { Err(AlpacaParseError::BadFormat( - "maps are not supported as ALPACA parameters".to_owned(), + "maps are not supported as Alpaca parameters".to_owned(), )) } @@ -240,7 +240,7 @@ impl<'de> Deserializer<'de> for AlpacaDeserializer { _visitor: V, ) -> Result { Err(AlpacaParseError::BadFormat( - "structs are not supported as ALPACA parameters".to_owned(), + "structs are not supported as Alpaca parameters".to_owned(), )) } } From 73a3bb80e98c189fa2e846c16fb4f35f2458876b Mon Sep 17 00:00:00 2001 From: Igor von Nyssen Date: Mon, 20 Apr 2026 17:47:00 -0700 Subject: [PATCH 5/6] Merge ParameterOutOfRange into BadParameter Make AlpacaParseError pub(crate) and carry it directly on Error::BadParameter. The ResponseWithTransaction> IntoResponse impl now destructures the inner AlpacaParseError::OutOfRange to produce ASCOM INVALID_VALUE, while plain BadFormat errors continue to return HTTP 400. Co-Authored-By: Claude Opus 4.7 --- src/server/error.rs | 13 ++----- src/server/params.rs | 84 ++++++++++++++++++++---------------------- src/server/response.rs | 6 +-- 3 files changed, 45 insertions(+), 58 deletions(-) diff --git a/src/server/error.rs b/src/server/error.rs index 0cf1f05..9a68daf 100644 --- a/src/server/error.rs +++ b/src/server/error.rs @@ -1,3 +1,4 @@ +use super::params::AlpacaParseError; use crate::ASCOMError; use crate::api::DeviceType; use axum::http::StatusCode; @@ -22,13 +23,7 @@ pub(crate) enum Error { BadParameter { name: &'static str, #[source] - err: serde_plain::Error, - }, - #[error("Parameter {name:?} value {value} is out of range for {target_type}")] - ParameterOutOfRange { - name: &'static str, - value: i64, - target_type: &'static str, + err: AlpacaParseError, }, #[error(transparent)] Ascom(#[from] ASCOMError), @@ -38,9 +33,7 @@ impl IntoResponse for Error { fn into_response(self) -> Response { let code = match self { Self::UnknownDeviceNumber { .. } | Self::UnknownAction { .. } => StatusCode::NOT_FOUND, - Self::MissingParameter { .. } - | Self::BadParameter { .. } - | Self::ParameterOutOfRange { .. } => StatusCode::BAD_REQUEST, + Self::MissingParameter { .. } | Self::BadParameter { .. } => StatusCode::BAD_REQUEST, Self::Ascom(_) => StatusCode::INTERNAL_SERVER_ERROR, }; (code, format!("{self:#}")).into_response() diff --git a/src/server/params.rs b/src/server/params.rs index 868f00b..b58dc49 100644 --- a/src/server/params.rs +++ b/src/server/params.rs @@ -6,32 +6,21 @@ use axum::response::{IntoResponse, Response}; use http::{Method, StatusCode}; use indexmap::IndexMap; use serde::Deserialize; -use serde::de::{DeserializeOwned, Deserializer, Error as _, Visitor}; +use serde::de::{DeserializeOwned, Deserializer, Visitor}; use std::fmt::{self, Debug}; use std::hash::Hash; /// Error type for Alpaca parameter parsing that distinguishes parse errors from range errors. -#[derive(Debug)] -enum AlpacaParseError { - /// Invalid format (not a valid integer/bool/etc) -> `BadParameter` (HTTP 400) +#[derive(Debug, thiserror::Error)] +pub(crate) enum AlpacaParseError { + /// Invalid format (not a valid integer/bool/etc) -> HTTP 400 + #[error("{0}")] BadFormat(String), - /// Valid integer but out of range for target type -> `INVALID_VALUE` (ASCOM error) + /// Valid integer but out of range for target type -> ASCOM `INVALID_VALUE` + #[error("value {value} is out of range for {target_type}")] OutOfRange { value: i64, target_type: &'static str }, } -impl fmt::Display for AlpacaParseError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::BadFormat(msg) => write!(f, "{msg}"), - Self::OutOfRange { value, target_type } => { - write!(f, "value {value} is out of range for {target_type}") - } - } - } -} - -impl std::error::Error for AlpacaParseError {} - impl serde::de::Error for AlpacaParseError { fn custom(msg: T) -> Self { Self::BadFormat(msg.to_string()) @@ -268,17 +257,8 @@ where self.0 .swap_remove(name.as_ref()) .map(|value| { - T::deserialize(AlpacaDeserializer { value }).map_err(|err| match err { - AlpacaParseError::OutOfRange { value, target_type } => Error::ParameterOutOfRange { - name, - value, - target_type, - }, - AlpacaParseError::BadFormat(msg) => Error::BadParameter { - name, - err: serde_plain::Error::custom(msg), - }, - }) + T::deserialize(AlpacaDeserializer { value }) + .map_err(|err| Error::BadParameter { name, err }) }) .transpose() } @@ -337,17 +317,7 @@ mod tests { let deser = AlpacaDeserializer { value: value.to_owned(), }; - T::deserialize(deser).map_err(|err| match err { - AlpacaParseError::OutOfRange { value, target_type } => Error::ParameterOutOfRange { - name: "test", - value, - target_type, - }, - AlpacaParseError::BadFormat(msg) => Error::BadParameter { - name: "test", - err: serde_plain::Error::custom(msg), - }, - }) + T::deserialize(deser).map_err(|err| Error::BadParameter { name: "test", err }) } #[test] @@ -384,7 +354,13 @@ mod tests { fn i32_out_of_range() { let err = parse::("3000000000").expect_err("should fail for i32 out of range"); assert!( - matches!(err, Error::ParameterOutOfRange { .. }), + matches!( + err, + Error::BadParameter { + err: AlpacaParseError::OutOfRange { .. }, + .. + } + ), "expected OutOfRange, got: {err:?}" ); } @@ -393,7 +369,13 @@ mod tests { fn u32_negative_out_of_range() { let err = parse::("-1").expect_err("should fail for negative u32"); assert!( - matches!(err, Error::ParameterOutOfRange { .. }), + matches!( + err, + Error::BadParameter { + err: AlpacaParseError::OutOfRange { .. }, + .. + } + ), "expected OutOfRange, got: {err:?}" ); } @@ -402,7 +384,13 @@ mod tests { fn u8_out_of_range() { let err = parse::("256").expect_err("should fail for u8 out of range"); assert!( - matches!(err, Error::ParameterOutOfRange { .. }), + matches!( + err, + Error::BadParameter { + err: AlpacaParseError::OutOfRange { .. }, + .. + } + ), "expected OutOfRange, got: {err:?}" ); } @@ -411,8 +399,14 @@ mod tests { fn bad_format_non_numeric() { let err = parse::("abc").expect_err("should fail for non-numeric input"); assert!( - matches!(err, Error::BadParameter { .. }), - "expected BadParameter, got: {err:?}" + matches!( + err, + Error::BadParameter { + err: AlpacaParseError::BadFormat(_), + .. + } + ), + "expected BadFormat, got: {err:?}" ); } diff --git a/src/server/response.rs b/src/server/response.rs index 2d2eed1..e4439ae 100644 --- a/src/server/response.rs +++ b/src/server/response.rs @@ -1,3 +1,4 @@ +use super::params::AlpacaParseError; use super::{Error, ResponseWithTransaction}; use crate::response::ValueResponse; use crate::{ASCOMError, ASCOMErrorCode, ASCOMResult}; @@ -52,10 +53,9 @@ where match self.response { Ok(response) => Ok(Ok(response)), Err(Error::Ascom(err)) => Ok(Err(err)), - Err(Error::ParameterOutOfRange { + Err(Error::BadParameter { name, - value, - target_type, + err: AlpacaParseError::OutOfRange { value, target_type }, }) => Ok(Err(ASCOMError::invalid_value(format!( "Parameter {name:?} value {value} is out of range for {target_type}" )))), From 234c4a2fee3af6add5274aa1adc23da56c18f817 Mon Sep 17 00:00:00 2001 From: Igor von Nyssen Date: Mon, 20 Apr 2026 17:47:34 -0700 Subject: [PATCH 6/6] Drop serde_plain dependency The custom AlpacaDeserializer now handles all Alpaca parameter parsing, so the serde_plain helper crate is no longer needed. Co-Authored-By: Claude Opus 4.7 --- Cargo.lock | 10 ---------- Cargo.toml | 2 -- 2 files changed, 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d8ad69b..44260b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -280,7 +280,6 @@ dependencies = [ "serde", "serde-ndim", "serde_json", - "serde_plain", "serde_repr", "serdebug", "socket2", @@ -4053,15 +4052,6 @@ dependencies = [ "serde_core", ] -[[package]] -name = "serde_plain" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ce1fc6db65a611022b23a0dec6975d63fb80a302cb3388835ff02c097258d50" -dependencies = [ - "serde", -] - [[package]] name = "serde_repr" version = "0.1.20" diff --git a/Cargo.toml b/Cargo.toml index 9245701..919298e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,7 +66,6 @@ reqwest = { version = "0.13.1", optional = true, default-features = false, featu serde = { version = "1.0.228", features = ["derive", "rc"] } serde-ndim = { version = "2.2.0", optional = true, features = ["ndarray"] } serde_json = { version = "1.0.148" } -serde_plain = { version = "1.0.2", optional = true } serde_repr = "0.1.20" socket2 = "0.6.1" thiserror = "2.0.17" @@ -147,7 +146,6 @@ server = [ "dep:axum", "dep:http", "dep:indexmap", - "dep:serde_plain", ] [package.metadata.docs.rs]