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

Replace outdated secrecy library with own impl #2997

Merged
merged 2 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ bitflags = "2.4.2"
serde_json = "1.0.108"
async-trait = "0.1.74"
tracing = { version = "0.1.40", features = ["log"] }
serde = { version = "1.0.192", features = ["derive"] }
serde = { version = "1.0.192", features = ["derive", "rc"] }
url = { version = "2.4.1", features = ["serde"] }
tokio = { version = "1.34.0", features = ["macros", "rt", "sync", "time", "io-util"] }
futures = { version = "0.3.29", default-features = false, features = ["std"] }
dep_time = { version = "0.3.36", package = "time", features = ["formatting", "parsing", "serde-well-known"] }
base64 = { version = "0.22.0" }
secrecy = { version = "0.8.0", features = ["serde"] }
zeroize = { version = "1.7" } # Not used in serenity, but bumps the minimal version from secrecy
arrayvec = { version = "0.7.4", features = ["serde"] }
serde_cow = { version = "0.1.0" }
Expand Down
11 changes: 5 additions & 6 deletions src/gateway/sharding/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ use std::sync::Arc;
use std::time::{Duration as StdDuration, Instant};

use aformat::{aformat, CapStr};
use secrecy::{ExposeSecret as _, Secret};
use tokio_tungstenite::tungstenite::error::Error as TungsteniteError;
use tokio_tungstenite::tungstenite::protocol::frame::CloseFrame;
use tracing::{debug, error, info, trace, warn};
Expand All @@ -57,7 +56,6 @@ pub use self::shard_queuer::{ShardQueue, ShardQueuer, ShardQueuerMessage};
pub use self::shard_runner::{ShardRunner, ShardRunnerMessage, ShardRunnerOptions};
use super::{ActivityData, ChunkGuildFilter, GatewayError, PresenceData, WsClient};
use crate::constants::{self, close_codes};
use crate::http::Token;
use crate::internal::prelude::*;
use crate::model::event::{Event, GatewayEvent};
use crate::model::gateway::{GatewayIntents, ShardInfo};
Expand Down Expand Up @@ -111,7 +109,7 @@ pub struct Shard {
// This acts as a timeout to determine if the shard has - for some reason - not started within
// a decent amount of time.
pub started: Instant,
token: Secret<Token>,
token: SecretString,
ws_url: Arc<str>,
resume_ws_url: Option<FixedString>,
pub intents: GatewayIntents,
Expand All @@ -133,13 +131,14 @@ impl Shard {
/// use serenity::gateway::Shard;
/// use serenity::model::gateway::{GatewayIntents, ShardInfo};
/// use serenity::model::id::ShardId;
/// use serenity::secret_string::SecretString;
/// use tokio::sync::Mutex;
/// #
/// # use serenity::http::Http;
/// #
/// # async fn run() -> Result<(), Box<dyn std::error::Error>> {
/// # let http: Arc<Http> = unimplemented!();
/// let token = Arc::from(std::env::var("DISCORD_BOT_TOKEN")?);
/// let token = SecretString::new(Arc::from(std::env::var("DISCORD_BOT_TOKEN")?));
/// let shard_info = ShardInfo {
/// id: ShardId(0),
/// total: NonZeroU16::MIN,
Expand All @@ -161,7 +160,7 @@ impl Shard {
/// TLS error.
pub async fn new(
ws_url: Arc<str>,
token: Arc<str>,
token: SecretString,
shard_info: ShardInfo,
intents: GatewayIntents,
presence: Option<PresenceData>,
Expand All @@ -188,7 +187,7 @@ impl Shard {
seq,
stage,
started: Instant::now(),
token: Token::new(token),
token,
session_id,
shard_info,
ws_url,
Expand Down
2 changes: 1 addition & 1 deletion src/gateway/sharding/shard_queuer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ impl ShardQueuer {
};
let mut shard = Shard::new(
Arc::clone(&self.ws_url),
Arc::clone(self.http.token()),
self.http.token(),
shard_info,
self.intents,
self.presence.clone(),
Expand Down
37 changes: 4 additions & 33 deletions src/http/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use reqwest::header::{HeaderMap as Headers, HeaderValue};
#[cfg(feature = "utils")]
use reqwest::Url;
use reqwest::{Client, ClientBuilder, Response as ReqwestResponse, StatusCode};
use secrecy::{ExposeSecret as _, Secret};
use serde::de::DeserializeOwned;
use serde::ser::SerializeSeq as _;
use serde_json::{from_value, to_string, to_vec};
Expand All @@ -37,34 +36,6 @@ use crate::constants;
use crate::internal::prelude::*;
use crate::model::prelude::*;

#[derive(Clone)]
pub(crate) struct Token(Arc<str>);

impl Token {
pub fn new(inner: Arc<str>) -> Secret<Self> {
Secret::new(Self(inner))
}
}

impl std::ops::Deref for Token {
type Target = str;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl secrecy::Zeroize for Token {
fn zeroize(&mut self) {
if let Some(string) = Arc::get_mut(&mut self.0) {
string.zeroize();
}
}
}

impl secrecy::CloneableSecret for Token {}
impl secrecy::DebugSecret for Token {}

// NOTE: This cannot be passed in from outside, due to `Cell` being !Send.
struct SerializeIter<I>(Cell<Option<I>>);

Expand Down Expand Up @@ -229,7 +200,7 @@ impl HttpBuilder {
client,
ratelimiter,
proxy: self.proxy,
token: Token::new(self.token),
token: SecretString::new(self.token),
application_id,
default_allowed_mentions: self.default_allowed_mentions,
}
Expand Down Expand Up @@ -268,7 +239,7 @@ pub struct Http {
pub(crate) client: Client,
pub ratelimiter: Option<Ratelimiter>,
pub proxy: Option<FixedString<u16>>,
token: Secret<Token>,
token: SecretString,
application_id: AtomicU64,
pub default_allowed_mentions: Option<CreateAllowedMentions<'static>>,
}
Expand Down Expand Up @@ -297,8 +268,8 @@ impl Http {
}

#[cfg(feature = "gateway")]
pub(crate) fn token(&self) -> &Arc<str> {
&self.token.expose_secret().0
pub(crate) fn token(&self) -> SecretString {
self.token.clone()
}

/// Adds a [`User`] to a [`Guild`] with a valid OAuth2 access token.
Expand Down
7 changes: 3 additions & 4 deletions src/http/ratelimiting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,12 @@ use std::time::SystemTime;
use dashmap::DashMap;
use reqwest::header::HeaderMap;
use reqwest::{Client, Response, StatusCode};
use secrecy::{ExposeSecret as _, Secret};
use tokio::sync::Mutex;
use tokio::time::{sleep, Duration};
use tracing::debug;

pub use super::routing::RatelimitingBucket;
use super::{HttpError, LightMethod, Request, Token};
use super::{HttpError, LightMethod, Request};
use crate::internal::prelude::*;

/// Passed to the [`Ratelimiter::set_ratelimit_callback`] callback. If using Client, that callback
Expand Down Expand Up @@ -86,7 +85,7 @@ pub struct Ratelimiter {
client: Client,
global: Mutex<()>,
routes: DashMap<RatelimitingBucket, Ratelimit>,
token: Secret<Token>,
token: SecretString,
absolute_ratelimits: bool,
ratelimit_callback: parking_lot::RwLock<Box<dyn Fn(RatelimitInfo) + Send + Sync>>,
}
Expand All @@ -112,7 +111,7 @@ impl Ratelimiter {
pub fn new(client: Client, token: Arc<str>) -> Self {
Self {
client,
token: Token::new(token),
token: SecretString::new(token),
global: Mutex::default(),
routes: DashMap::new(),
absolute_ratelimits: false,
Expand Down
1 change: 1 addition & 0 deletions src/internal/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ pub use super::utils::join_to_string;
#[cfg(feature = "http")]
pub use crate::error::Error;
pub use crate::error::Result;
pub use crate::secret_string::SecretString;

pub type JsonMap = serde_json::Map<String, Value>;
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ pub mod gateway;
pub mod http;
#[cfg(feature = "interactions_endpoint")]
pub mod interactions_endpoint;
pub mod secret_string;
#[cfg(feature = "utils")]
pub mod utils;

Expand Down
19 changes: 0 additions & 19 deletions src/model/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,25 +203,6 @@ pub mod single_recipient {
}
}

pub mod secret {
use secrecy::{ExposeSecret, Secret, Zeroize};
use serde::{Deserialize, Deserializer, Serialize, Serializer};

pub fn deserialize<'de, S: Deserialize<'de> + Zeroize, D: Deserializer<'de>>(
deserializer: D,
) -> Result<Option<Secret<S>>, D::Error> {
Option::<S>::deserialize(deserializer).map(|s| s.map(Secret::new))
}

#[allow(clippy::ref_option)]
pub fn serialize<S: Serialize + Zeroize, Sr: Serializer>(
secret: &Option<Secret<S>>,
serializer: Sr,
) -> Result<Sr::Ok, Sr::Error> {
secret.as_ref().map(ExposeSecret::expose_secret).serialize(serializer)
}
}

pub fn discord_colours_opt<'de, D>(deserializer: D) -> Result<Option<Vec<Colour>>, D::Error>
where
D: Deserializer<'de>,
Expand Down
9 changes: 1 addition & 8 deletions src/model/webhook.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
//! Webhook model and implementations.

#[cfg(feature = "model")]
use secrecy::ExposeSecret;
use secrecy::SecretString;

use super::utils::secret;
#[cfg(feature = "model")]
use crate::builder::{EditWebhook, EditWebhookMessage, ExecuteWebhook};
#[cfg(feature = "cache")]
Expand Down Expand Up @@ -76,7 +71,6 @@ pub struct Webhook {
/// This can be temporarily overridden via [`ExecuteWebhook::avatar_url`].
pub avatar: Option<ImageHash>,
/// The webhook's secure token.
#[serde(with = "secret", default)]
pub token: Option<SecretString>,
/// The bot/OAuth2 application that created this webhook.
pub application_id: Option<ApplicationId>,
Expand All @@ -87,7 +81,6 @@ pub struct Webhook {
/// [`WebhookType::ChannelFollower`]).
pub source_channel: Option<WebhookChannel>,
/// The url used for executing the webhook (returned by the webhooks OAuth2 flow).
#[serde(with = "secret", default)]
pub url: Option<SecretString>,
}

Expand Down Expand Up @@ -314,7 +307,7 @@ impl Webhook {
///
/// Or may return an [`Error::Json`] if there is an error in deserialising Discord's response.
pub async fn edit(&mut self, http: &Http, builder: EditWebhook<'_>) -> Result<()> {
let token = self.token.as_ref().map(ExposeSecret::expose_secret).map(String::as_str);
let token = self.token.as_ref().map(SecretString::expose_secret);
*self = builder.execute(http, self.id, token).await?;
Ok(())
}
Expand Down
41 changes: 41 additions & 0 deletions src/secret_string.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use std::sync::Arc;

/// A cheaply clonable, zeroed on drop, String.
///
/// This is a simple newtype of `Arc<str>` that uses [`zeroize::Zeroize`] on last drop to avoid
/// keeping it around in memory.
#[derive(Clone, serde::Deserialize, serde::Serialize)]
pub struct SecretString(Arc<str>);

impl SecretString {
#[must_use]
pub fn new(inner: Arc<str>) -> Self {
Self(inner)
}

#[must_use]
pub fn expose_secret(&self) -> &str {
&self.0
}
}

impl std::fmt::Debug for SecretString {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fmt.debug_tuple(std::any::type_name::<Self>()).field(&"<secret>").finish()
}
}

impl zeroize::Zeroize for SecretString {
fn zeroize(&mut self) {
if let Some(string) = Arc::get_mut(&mut self.0) {
string.zeroize();
}
}
}

#[cfg(feature = "typesize")]
impl typesize::TypeSize for SecretString {
fn extra_size(&self) -> usize {
self.0.len() + (size_of::<usize>() * 2)
}
}
Loading