Skip to content

Commit

Permalink
Get rid of unsafe (#2686)
Browse files Browse the repository at this point in the history
A discord bot library should not be using the tools reserved for low
level OS interaction/data structure libraries.
  • Loading branch information
GnomedDev committed Mar 25, 2024
1 parent e6bb028 commit b817f1e
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 31 deletions.
7 changes: 3 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,9 @@ your code.

## Unsafe

Code that defines or uses `unsafe` functions must be reasoned with comments.
`unsafe` code can pose a potential for undefined behaviour related bugs and other
kinds of bugs to sprout if misused, weakening security. If you commit code containing
`unsafe`, you should confirm that its usage is necessary and correct.
Unsafe code is forbidden, and safe alternatives must be found. This can be mitigated by using
a third party crate to offload the burden of justifying the unsafe code, or finding a safe
alternative.

# Comment / Documentation style

Expand Down
2 changes: 1 addition & 1 deletion command_attr/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl LitExt for Lit {
fn to_str(&self) -> String {
match self {
Self::Str(s) => s.value(),
Self::ByteStr(s) => unsafe { String::from_utf8_unchecked(s.value()) },
Self::ByteStr(s) => String::from_utf8_lossy(&s.value()).into_owned(),
Self::Char(c) => c.value().to_string(),
Self::Byte(b) => (b.value() as char).to_string(),
_ => panic!("values must be a (byte)string or a char"),
Expand Down
22 changes: 15 additions & 7 deletions src/http/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::model::id::*;

/// Used to group requests together for ratelimiting.
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct RatelimitingBucket(Option<(std::mem::Discriminant<Route<'static>>, Option<NonZeroU64>)>);
pub struct RatelimitingBucket(Option<(RouteKind, Option<NonZeroU64>)>);

impl RatelimitingBucket {
#[must_use]
Expand Down Expand Up @@ -40,7 +40,20 @@ macro_rules! routes {
)+
}

#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
enum RouteKind {
$($name,)+
}

impl<$lt> Route<$lt> {
fn kind(&self) -> RouteKind {
match self {
$(
Self::$name {..} => RouteKind::$name,
)+
}
}

#[must_use]
pub fn path(self) -> Cow<'static, str> {
match self {
Expand All @@ -59,17 +72,12 @@ macro_rules! routes {
)+
};

// To avoid adding a lifetime on RatelimitingBucket and causing lifetime infection,
// we transmute the Discriminant<Route<'a>> to Discriminant<Route<'static>>.
// SAFETY: std::mem::discriminant erases lifetimes.
let discriminant = unsafe { std::mem::transmute(std::mem::discriminant(self)) };

RatelimitingBucket(ratelimiting_kind.map(|r| {
let id = match r {
RatelimitingKind::PathAndId(id) => Some(id),
RatelimitingKind::Path => None,
};
(discriminant, id)
(self.kind(), id)
}))
}

Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
//! [gateway docs]: crate::gateway
#![doc(html_root_url = "https://docs.rs/serenity/*")]
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![forbid(unsafe_code)]
#![warn(
unused,
rust_2018_idioms,
Expand Down
74 changes: 56 additions & 18 deletions src/model/guild/audit_log/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
//! Audit log types for administrative actions within guilds.
use std::mem::transmute;

use nonmax::{NonMaxU32, NonMaxU64};
use serde::ser::{Serialize, Serializer};

Expand Down Expand Up @@ -70,22 +68,62 @@ impl Action {
pub fn from_value(value: u8) -> Action {
match value {
1 => Action::GuildUpdate,
10..=12 => Action::Channel(unsafe { transmute(value) }),
13..=15 => Action::ChannelOverwrite(unsafe { transmute(value) }),
20..=28 => Action::Member(unsafe { transmute(value) }),
30..=32 => Action::Role(unsafe { transmute(value) }),
40..=42 => Action::Invite(unsafe { transmute(value) }),
50..=52 => Action::Webhook(unsafe { transmute(value) }),
60..=62 => Action::Emoji(unsafe { transmute(value) }),
72..=75 => Action::Message(unsafe { transmute(value) }),
80..=82 => Action::Integration(unsafe { transmute(value) }),
83..=85 => Action::StageInstance(unsafe { transmute(value) }),
90..=92 => Action::Sticker(unsafe { transmute(value) }),
100..=102 => Action::ScheduledEvent(unsafe { transmute(value) }),
110..=112 => Action::Thread(unsafe { transmute(value) }),
140..=145 => Action::AutoMod(unsafe { transmute(value) }),
150..=151 => Action::CreatorMonetization(unsafe { transmute(value) }),
192..=193 => Action::VoiceChannelStatus(unsafe { transmute(value) }),
10 => Action::Channel(ChannelAction::Create),
11 => Action::Channel(ChannelAction::Update),
12 => Action::Channel(ChannelAction::Delete),
13 => Action::ChannelOverwrite(ChannelOverwriteAction::Create),
14 => Action::ChannelOverwrite(ChannelOverwriteAction::Update),
15 => Action::ChannelOverwrite(ChannelOverwriteAction::Delete),
20 => Action::Member(MemberAction::Kick),
21 => Action::Member(MemberAction::Prune),
22 => Action::Member(MemberAction::BanAdd),
23 => Action::Member(MemberAction::BanRemove),
24 => Action::Member(MemberAction::Update),
25 => Action::Member(MemberAction::RoleUpdate),
26 => Action::Member(MemberAction::MemberMove),
27 => Action::Member(MemberAction::MemberDisconnect),
28 => Action::Member(MemberAction::BotAdd),
30 => Action::Role(RoleAction::Create),
31 => Action::Role(RoleAction::Update),
32 => Action::Role(RoleAction::Delete),
40 => Action::Invite(InviteAction::Create),
41 => Action::Invite(InviteAction::Update),
42 => Action::Invite(InviteAction::Delete),
50 => Action::Webhook(WebhookAction::Create),
51 => Action::Webhook(WebhookAction::Update),
52 => Action::Webhook(WebhookAction::Delete),
60 => Action::Emoji(EmojiAction::Create),
61 => Action::Emoji(EmojiAction::Update),
62 => Action::Emoji(EmojiAction::Delete),
72 => Action::Message(MessageAction::Delete),
73 => Action::Message(MessageAction::BulkDelete),
74 => Action::Message(MessageAction::Pin),
75 => Action::Message(MessageAction::Unpin),
80 => Action::Integration(IntegrationAction::Create),
81 => Action::Integration(IntegrationAction::Update),
82 => Action::Integration(IntegrationAction::Delete),
83 => Action::StageInstance(StageInstanceAction::Create),
84 => Action::StageInstance(StageInstanceAction::Update),
85 => Action::StageInstance(StageInstanceAction::Delete),
90 => Action::Sticker(StickerAction::Create),
91 => Action::Sticker(StickerAction::Update),
92 => Action::Sticker(StickerAction::Delete),
100 => Action::ScheduledEvent(ScheduledEventAction::Create),
101 => Action::ScheduledEvent(ScheduledEventAction::Update),
102 => Action::ScheduledEvent(ScheduledEventAction::Delete),
110 => Action::Thread(ThreadAction::Create),
111 => Action::Thread(ThreadAction::Update),
112 => Action::Thread(ThreadAction::Delete),
140 => Action::AutoMod(AutoModAction::RuleCreate),
141 => Action::AutoMod(AutoModAction::RuleUpdate),
142 => Action::AutoMod(AutoModAction::RuleDelete),
143 => Action::AutoMod(AutoModAction::BlockMessage),
144 => Action::AutoMod(AutoModAction::FlagToChannel),
145 => Action::AutoMod(AutoModAction::UserCommunicationDisabled),
150 => Action::CreatorMonetization(CreatorMonetizationAction::RequestCreated),
151 => Action::CreatorMonetization(CreatorMonetizationAction::TermsAccepted),
192 => Action::VoiceChannelStatus(VoiceChannelStatusAction::StatusUpdate),
193 => Action::VoiceChannelStatus(VoiceChannelStatusAction::StatusDelete),
_ => Action::Unknown(value),
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/model/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ macro_rules! id_u64 {

impl From<$name> for NonZeroI64 {
fn from(id: $name) -> NonZeroI64 {
unsafe {NonZeroI64::new_unchecked(id.get() as i64)}
NonZeroI64::new(id.get() as i64).unwrap()
}
}

Expand Down

0 comments on commit b817f1e

Please sign in to comment.