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.

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

feat(http, validate): validate modal titles #2142

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
client::Client,
error::Error,
error::{Error, ErrorType},
request::{attachment::AttachmentManager, Request, TryIntoRequest},
response::{marker::EmptyBody, Response, ResponseFuture},
routing::Route,
Expand Down Expand Up @@ -61,6 +61,19 @@ impl IntoFuture for CreateResponse<'_> {

impl TryIntoRequest for CreateResponse<'_> {
fn try_into_request(self) -> Result<Request, Error> {
// Check if the title for a modal response is valid.
if let Some(modal_title) = self
.response
.data
.as_ref()
.and_then(|data| data.title.as_ref())
{
twilight_validate::component::modal_title_length(modal_title).map_err(|e| Error {
kind: ErrorType::BuildingRequest,
source: Some(Box::new(e)),
})?;
Comment on lines +71 to +74
Copy link
Member Author

@suneettipirneni suneettipirneni Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if this is the way I should handle this. If not, please let me know the desired solution.

Copy link
Member

@zeylahellyer zeylahellyer Feb 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't do validation in IntoFuture at the moment... current intuition and prior art is that this should go in CreateResponse::new (which is a breaking change). But the downside there, too, is that it's a breaking change. Both of these are problems we're aiming to solve for 0.16, so I don't have a good answer here at the moment. cc @7596ff

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this request already accepts invalid data, so this isn't even a complete fix. my reflex is to just punt to 0.16 because this comes under the whole "validate at the end" paradigm. we can at least leave the changes in the validate crate to save this pr, but i don't want to do anything to the request at the moment

}

let mut request = Request::builder(&Route::InteractionCallback {
interaction_id: self.interaction_id.get(),
interaction_token: self.interaction_token,
Expand Down Expand Up @@ -99,7 +112,9 @@ mod tests {
use std::error::Error;
use twilight_http_ratelimiting::Path;
use twilight_model::{
http::interaction::{InteractionResponse, InteractionResponseType},
http::interaction::{
InteractionResponse, InteractionResponseData, InteractionResponseType,
},
id::Id,
};

Expand Down Expand Up @@ -129,4 +144,49 @@ mod tests {

Ok(())
}

#[test]
fn invalid_modal() -> Result<(), Box<dyn Error>> {
let application_id = Id::new(1);
let interaction_id = Id::new(2);
let token = "foo".to_owned().into_boxed_str();

let client = Client::new(String::new());

let response = InteractionResponse {
kind: InteractionResponseType::Modal,
data: Some(InteractionResponseData {
title: Some("a".repeat(100)),
custom_id: Some(String::from("12345")),
components: Some(Vec::new()),
..Default::default()
}),
};

let req = client
.interaction(application_id)
.create_response(interaction_id, &token, &response)
.try_into_request();

assert!(req.is_err());

let response = InteractionResponse {
kind: InteractionResponseType::Modal,
data: Some(InteractionResponseData {
title: Some("a".repeat(45)),
custom_id: Some(String::from("12345")),
components: Some(Vec::new()),
..Default::default()
}),
};

let req = client
.interaction(application_id)
.create_response(interaction_id, &token, &response)
.try_into_request();

assert!(req.is_ok());

Ok(())
}
}
52 changes: 52 additions & 0 deletions twilight-validate/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,17 @@ pub const COMPONENT_CUSTOM_ID_LENGTH: usize = 100;
/// [1]: https://discord.com/developers/docs/interactions/message-components#component-object-component-structure
pub const COMPONENT_BUTTON_LABEL_LENGTH: usize = 80;

/// Maximum length of the title of a [`Modal`] interaction response in codepoints.
///
/// An example of a modal with a title is the [`Modal`][`Modal::title`].
///
/// This is defined in Discord's documentation, per
/// [Discord Docs/Modal][1].
///
/// [1]: https://discord.com/developers/docs/interactions/receiving-and-responding#interaction-response-object-modal
/// [`Modal`]: twilight_model::http::interaction::InteractionResponseData
pub const MODAL_MAX_TITLE_LENGTH: usize = 45;

/// Maximum number of [`SelectMenuOption`]s that can be chosen in a
/// [`SelectMenu`].
///
Expand Down Expand Up @@ -243,6 +254,13 @@ impl Display for ComponentValidationError {

f.write_str("' component was provided, but can not be a root component")
}
ComponentValidationErrorType::ModalTitleLength { chars } => {
f.write_str("a modal's title is ")?;
Display::fmt(&chars, f)?;
f.write_str(" characters long, but the max is ")?;

Display::fmt(&MODAL_MAX_TITLE_LENGTH, f)
}
ComponentValidationErrorType::SelectMaximumValuesCount { count } => {
f.write_str("maximum number of values that can be chosen is ")?;
Display::fmt(count, f)?;
Expand Down Expand Up @@ -386,6 +404,12 @@ pub enum ComponentValidationErrorType {
/// Type of provided component.
kind: ComponentType,
},
/// Value of a select menu option is larger than the
/// [the maximum][`MODAL_MAX_TITLE_LENGTH`].
ModalTitleLength {
/// Number of codepoints that were provided.
chars: usize,
},
/// Maximum number of items that can be chosen is smaller than
/// [the minimum][`SELECT_MAXIMUM_VALUES_REQUIREMENT`] or larger than
/// [the maximum][`SELECT_MAXIMUM_VALUES_LIMIT`].
Expand Down Expand Up @@ -1046,6 +1070,26 @@ fn component_text_input_value(value: impl AsRef<str>) -> Result<(), ComponentVal
}
}

/// Ensure a [`Modal::title`] length is valid.
///
/// # Errors
///
/// Returns an error of type [`ModalTitleLength`] if the length is invalid.
///
/// [`Modal::title`]: twilight_model::http::interaction::InteractionResponseData
/// [`ModalTitleLength`]: ComponentValidationErrorType::ModalTitleLength
pub fn modal_title_length(title: impl AsRef<str>) -> Result<(), ComponentValidationError> {
let chars = title.as_ref().chars().count();

if chars > MODAL_MAX_TITLE_LENGTH {
return Err(ComponentValidationError {
kind: ComponentValidationErrorType::ModalTitleLength { chars },
});
}

Ok(())
}

#[allow(clippy::non_ascii_literal)]
#[cfg(test)]
mod tests {
Expand Down Expand Up @@ -1316,4 +1360,12 @@ mod tests {

assert!(component_text_input_min(4001).is_err());
}

#[test]
fn modal_title_length_value() {
assert!(modal_title_length("a".repeat(10)).is_ok());

assert!(modal_title_length("a".repeat(46)).is_err());
assert!(modal_title_length("a".repeat(50)).is_err());
}
}