Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(model): Don't deserialize option values as Id's if option type is String #2190

Merged
merged 13 commits into from
Apr 23, 2023
Merged

fix(model): Don't deserialize option values as Id's if option type is String #2190

Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
459bba5
fix(model): Don't deserialize option values as `Id`'s if option type …
suneettipirneni Apr 3, 2023
0e41eed
chore: remove redundant generic parameter
suneettipirneni Apr 3, 2023
955ba6b
refactor: use a more resilient solution
suneettipirneni Apr 3, 2023
096b477
chore: typo
suneettipirneni Apr 3, 2023
f6db253
chore: remove unused logic
suneettipirneni Apr 3, 2023
1bccb64
refactor: still accept Id's
suneettipirneni Apr 3, 2023
ae32f23
refactor: use `str::parse`
suneettipirneni Apr 4, 2023
46cce4c
add additional test for leading zeroes in string
suneettipirneni Apr 4, 2023
85a1a20
remove string match arms
suneettipirneni Apr 6, 2023
d85d1c9
Revert "remove string match arms"
suneettipirneni Apr 6, 2023
896cbb5
refactor: remove Id variant
suneettipirneni Apr 15, 2023
860a080
use if-let-else instead of match
suneettipirneni Apr 15, 2023
b28510b
make requested changes
suneettipirneni Apr 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 85 additions & 47 deletions twilight-model/src/application/interaction/application_command/option.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use serde::{
ser::SerializeStruct,
Deserialize, Deserializer, Serialize, Serializer,
};
use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
use std::{
fmt::{Debug, Display, Formatter, Result as FmtResult},
str::FromStr,
};

/// Data received when a user fills in a command option.
///
Expand Down Expand Up @@ -83,17 +86,20 @@ impl<'de> Deserialize<'de> for CommandDataOption {
Focused,
}

// Id before string such that IDs will always be interpreted
// as such, this does mean that string inputs that looks like
// IDs will have to be caught if it is a string.
// An `Id` variant is purposely placed below `String` here.
// We first deserialize into a `String` and then
// later parse it into an `Id` variant. This is done
// to prevent situations where an option's string value
// is parsed as an `Id` variant, which omits features leading zeros.
// The `Id` variant is only used when the type is an `Id`.
#[derive(Debug, Deserialize)]
#[serde(untagged)]
enum ValueEnvelope {
Boolean(bool),
Integer(i64),
Number(f64),
Id(Id<GenericMarker>),
String(String),
Id(Id<GenericMarker>),
suneettipirneni marked this conversation as resolved.
Show resolved Hide resolved
}

impl ValueEnvelope {
Expand All @@ -102,8 +108,8 @@ impl<'de> Deserialize<'de> for CommandDataOption {
Self::Boolean(b) => Unexpected::Bool(*b),
Self::Integer(i) => Unexpected::Signed(*i),
Self::Number(f) => Unexpected::Float(*f),
Self::Id(_) => Unexpected::Other("ID"),
Self::String(s) => Unexpected::Str(s),
Self::Id(_) => Unexpected::Other("ID"),
}
}
}
Expand All @@ -114,8 +120,8 @@ impl<'de> Deserialize<'de> for CommandDataOption {
Self::Boolean(b) => Display::fmt(b, f),
Self::Integer(i) => Display::fmt(i, f),
Self::Number(n) => Display::fmt(n, f),
Self::Id(i) => Display::fmt(i, f),
Self::String(s) => Display::fmt(s, f),
Self::Id(i) => Display::fmt(i, f),
}
}
}
Expand Down Expand Up @@ -202,13 +208,19 @@ impl<'de> Deserialize<'de> for CommandDataOption {
CommandOptionType::Attachment => {
let val = value_opt.ok_or_else(|| DeError::missing_field("value"))?;

if let ValueEnvelope::Id(id) = val {
CommandOptionValue::Attachment(id.cast())
} else {
return Err(DeError::invalid_type(
val.as_unexpected(),
&"attachment id",
));
match &val {
ValueEnvelope::String(id) => CommandOptionValue::Attachment(
Id::from_str(id.as_str()).map_err(|_e| {
DeError::invalid_type(val.as_unexpected(), &"attachment id")
})?,
),
suneettipirneni marked this conversation as resolved.
Show resolved Hide resolved
ValueEnvelope::Id(id) => CommandOptionValue::Attachment(id.cast()),
_ => {
return Err(DeError::invalid_type(
val.as_unexpected(),
&"attachment id",
))
}
}
}
CommandOptionType::Boolean => {
Expand All @@ -223,13 +235,19 @@ impl<'de> Deserialize<'de> for CommandDataOption {
CommandOptionType::Channel => {
let val = value_opt.ok_or_else(|| DeError::missing_field("value"))?;

if let ValueEnvelope::Id(id) = val {
CommandOptionValue::Channel(id.cast())
} else {
return Err(DeError::invalid_type(
val.as_unexpected(),
&"channel id",
));
match &val {
ValueEnvelope::String(id) => CommandOptionValue::Channel(
Id::from_str(id.as_str()).map_err(|_e| {
DeError::invalid_type(val.as_unexpected(), &"channel id")
})?,
),
ValueEnvelope::Id(id) => CommandOptionValue::Channel(id.cast()),
_ => {
return Err(DeError::invalid_type(
val.as_unexpected(),
&"channel id",
))
}
}
}
CommandOptionType::Integer => {
Expand All @@ -244,13 +262,22 @@ impl<'de> Deserialize<'de> for CommandDataOption {
CommandOptionType::Mentionable => {
let val = value_opt.ok_or_else(|| DeError::missing_field("value"))?;

if let ValueEnvelope::Id(id) = val {
CommandOptionValue::Mentionable(id)
} else {
return Err(DeError::invalid_type(
val.as_unexpected(),
&"mentionable id",
));
match &val {
ValueEnvelope::String(id) => CommandOptionValue::Mentionable(
Id::from_str(id.as_str()).map_err(|_e| {
DeError::invalid_type(
val.as_unexpected(),
&"mentionable id",
)
})?,
),
ValueEnvelope::Id(id) => CommandOptionValue::Mentionable(id.cast()),
_ => {
return Err(DeError::invalid_type(
val.as_unexpected(),
&"mentionable id",
))
}
}
}
CommandOptionType::Number => {
Expand Down Expand Up @@ -278,26 +305,28 @@ impl<'de> Deserialize<'de> for CommandDataOption {
CommandOptionType::Role => {
let val = value_opt.ok_or_else(|| DeError::missing_field("value"))?;

if let ValueEnvelope::Id(id) = val {
CommandOptionValue::Role(id.cast())
} else {
return Err(DeError::invalid_type(val.as_unexpected(), &"role id"));
match &val {
ValueEnvelope::String(id) => {
CommandOptionValue::Role(Id::from_str(id.as_str()).map_err(
|_e| DeError::invalid_type(val.as_unexpected(), &"role id"),
)?)
}
ValueEnvelope::Id(id) => CommandOptionValue::Role(id.cast()),
_ => {
return Err(DeError::invalid_type(
val.as_unexpected(),
&"role id",
))
}
}
}
CommandOptionType::String => {
let val = value_opt.ok_or_else(|| DeError::missing_field("value"))?;

match val {
ValueEnvelope::String(s) => CommandOptionValue::String(s),
ValueEnvelope::Id(id) => {
CommandOptionValue::String(id.get().to_string())
}
other => {
return Err(DeError::invalid_type(
other.as_unexpected(),
&"string",
));
}
if let ValueEnvelope::String(s) = val {
CommandOptionValue::String(s)
} else {
return Err(DeError::invalid_type(val.as_unexpected(), &"string"));
}
}
CommandOptionType::SubCommand => CommandOptionValue::SubCommand(options),
Expand All @@ -307,10 +336,19 @@ impl<'de> Deserialize<'de> for CommandDataOption {
CommandOptionType::User => {
let val = value_opt.ok_or_else(|| DeError::missing_field("value"))?;

if let ValueEnvelope::Id(id) = val {
CommandOptionValue::User(id.cast())
} else {
return Err(DeError::invalid_type(val.as_unexpected(), &"user id"));
match &val {
ValueEnvelope::String(id) => {
CommandOptionValue::User(Id::from_str(id.as_str()).map_err(
|_e| DeError::invalid_type(val.as_unexpected(), &"user id"),
)?)
}
ValueEnvelope::Id(id) => CommandOptionValue::User(id.cast()),
_ => {
return Err(DeError::invalid_type(
val.as_unexpected(),
&"user id",
))
}
}
}
}
Expand Down