Remove unsafe code (#940)

Remove unsafe code
This commit is contained in:
Jason Gray 2022-01-23 12:02:04 -06:00 committed by GitHub
parent c6e97a7f8a
commit ceebb374f0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 295 additions and 150 deletions

View file

@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [main] Prevent hang when discovery is disabled and there are no credentials or when bad credentials are given.
- [main] Don't panic when parsing options. Instead list valid values and exit.
- [main] `--alsa-mixer-device` and `--alsa-mixer-index` now fallback to the card and index specified in `--device`.
- [core] Removed unsafe code (breaking)
### Removed
- [playback] `alsamixer`: previously deprecated option `mixer-card` has been removed.

View file

@ -351,12 +351,17 @@ impl Cache {
}
fn file_path(&self, file: FileId) -> Option<PathBuf> {
self.audio_location.as_ref().map(|location| {
let name = file.to_base16();
match file.to_base16() {
Ok(name) => self.audio_location.as_ref().map(|location| {
let mut path = location.join(&name[0..2]);
path.push(&name[2..]);
path
})
}),
Err(e) => {
warn!("Invalid FileId: {}", e.utf8_error());
None
}
}
}
pub fn file(&self, file: FileId) -> Option<File> {

View file

@ -2,6 +2,7 @@
use std::convert::TryInto;
use std::fmt;
use std::string::FromUtf8Error;
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum SpotifyAudioType {
@ -136,7 +137,7 @@ impl SpotifyId {
/// Returns the `SpotifyId` as a base16 (hex) encoded, `SpotifyId::SIZE_BASE16` (32)
/// character long `String`.
pub fn to_base16(&self) -> String {
pub fn to_base16(&self) -> Result<String, FromUtf8Error> {
to_base16(&self.to_raw(), &mut [0u8; SpotifyId::SIZE_BASE16])
}
@ -144,7 +145,7 @@ impl SpotifyId {
/// character long `String`.
///
/// [canonically]: https://developer.spotify.com/documentation/web-api/#spotify-uris-and-ids
pub fn to_base62(&self) -> String {
pub fn to_base62(&self) -> Result<String, FromUtf8Error> {
let mut dst = [0u8; 22];
let mut i = 0;
let n = self.id;
@ -182,10 +183,7 @@ impl SpotifyId {
dst.reverse();
unsafe {
// Safety: We are only dealing with ASCII characters.
String::from_utf8_unchecked(dst.to_vec())
}
String::from_utf8(dst.to_vec())
}
/// Returns a copy of the `SpotifyId` as an array of `SpotifyId::SIZE` (16) bytes in
@ -202,7 +200,7 @@ impl SpotifyId {
/// be encoded as `unknown`.
///
/// [Spotify URI]: https://developer.spotify.com/documentation/web-api/#spotify-uris-and-ids
pub fn to_uri(&self) -> String {
pub fn to_uri(&self) -> Result<String, FromUtf8Error> {
// 8 chars for the "spotify:" prefix + 1 colon + 22 chars base62 encoded ID = 31
// + unknown size audio_type.
let audio_type: &str = self.audio_type.into();
@ -210,9 +208,10 @@ impl SpotifyId {
dst.push_str("spotify:");
dst.push_str(audio_type);
dst.push(':');
dst.push_str(&self.to_base62());
let base62 = self.to_base62()?;
dst.push_str(&base62);
dst
Ok(dst)
}
}
@ -220,7 +219,7 @@ impl SpotifyId {
pub struct FileId(pub [u8; 20]);
impl FileId {
pub fn to_base16(&self) -> String {
pub fn to_base16(&self) -> Result<String, FromUtf8Error> {
to_base16(&self.0, &mut [0u8; 40])
}
}
@ -233,12 +232,12 @@ impl fmt::Debug for FileId {
impl fmt::Display for FileId {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(&self.to_base16())
f.write_str(&self.to_base16().unwrap_or_default())
}
}
#[inline]
fn to_base16(src: &[u8], buf: &mut [u8]) -> String {
fn to_base16(src: &[u8], buf: &mut [u8]) -> Result<String, FromUtf8Error> {
let mut i = 0;
for v in src {
buf[i] = BASE16_DIGITS[(v >> 4) as usize];
@ -246,10 +245,7 @@ fn to_base16(src: &[u8], buf: &mut [u8]) -> String {
i += 2;
}
unsafe {
// Safety: We are only dealing with ASCII characters.
String::from_utf8_unchecked(buf.to_vec())
}
String::from_utf8(buf.to_vec())
}
#[cfg(test)]
@ -366,7 +362,7 @@ mod tests {
audio_type: c.kind,
};
assert_eq!(id.to_base62(), c.base62);
assert_eq!(id.to_base62().unwrap(), c.base62);
}
}
@ -389,7 +385,7 @@ mod tests {
audio_type: c.kind,
};
assert_eq!(id.to_base16(), c.base16);
assert_eq!(id.to_base16().unwrap(), c.base16);
}
}
@ -415,7 +411,7 @@ mod tests {
audio_type: c.kind,
};
assert_eq!(id.to_uri(), c.uri);
assert_eq!(id.to_uri().unwrap(), c.uri);
}
}

View file

@ -7,12 +7,12 @@ extern crate log;
extern crate async_trait;
pub mod cover;
use std::collections::HashMap;
use std::string::FromUtf8Error;
use librespot_core::mercury::MercuryError;
use librespot_core::session::Session;
use librespot_core::spotify_id::{FileId, SpotifyAudioType, SpotifyId};
use librespot_core::spotify_id::{FileId, SpotifyAudioType, SpotifyId, SpotifyIdError};
use librespot_protocol as protocol;
use protobuf::Message;
@ -83,10 +83,16 @@ trait AudioFiles {
#[async_trait]
impl AudioFiles for Track {
async fn get_audio_item(session: &Session, id: SpotifyId) -> Result<AudioItem, MercuryError> {
match id.to_base62() {
Err(e) => {
warn!("Invalid Track SpotifyId: {}", e);
Err(MercuryError)
}
Ok(uri) => {
let item = Self::get(session, id).await?;
Ok(AudioItem {
id,
uri: format!("spotify:track:{}", id.to_base62()),
uri: format!("spotify:track:{}", uri),
files: item.files,
name: item.name,
duration: item.duration,
@ -95,15 +101,22 @@ impl AudioFiles for Track {
})
}
}
}
}
#[async_trait]
impl AudioFiles for Episode {
async fn get_audio_item(session: &Session, id: SpotifyId) -> Result<AudioItem, MercuryError> {
match id.to_base62() {
Err(e) => {
warn!("Invalid Episode SpotifyId: {}", e);
Err(MercuryError)
}
Ok(uri) => {
let item = Self::get(session, id).await?;
Ok(AudioItem {
id,
uri: format!("spotify:episode:{}", id.to_base62()),
uri: format!("spotify:episode:{}", uri),
files: item.files,
name: item.name,
duration: item.duration,
@ -112,21 +125,45 @@ impl AudioFiles for Episode {
})
}
}
}
}
#[async_trait]
pub trait Metadata: Send + Sized + 'static {
type Message: protobuf::Message;
fn request_url(id: SpotifyId) -> String;
fn parse(msg: &Self::Message, session: &Session) -> Self;
fn request_url(id: SpotifyId) -> Result<String, FromUtf8Error>;
fn parse(msg: &Self::Message, session: &Session) -> Result<Self, SpotifyIdError>;
async fn get(session: &Session, id: SpotifyId) -> Result<Self, MercuryError> {
let uri = Self::request_url(id);
match Self::request_url(id) {
Err(e) => {
warn!("Invalid SpotifyId: {}", e);
Err(MercuryError)
}
Ok(uri) => {
let response = session.mercury().get(uri).await?;
let data = response.payload.first().expect("Empty payload");
let msg = Self::Message::parse_from_bytes(data).unwrap();
Ok(Self::parse(&msg, session))
match response.payload.first() {
None => {
warn!("Empty payload");
Err(MercuryError)
}
Some(data) => match Self::Message::parse_from_bytes(data) {
Err(e) => {
warn!("Error parsing message from bytes: {}", e);
Err(MercuryError)
}
Ok(msg) => match Self::parse(&msg, session) {
Err(e) => {
warn!("Error parsing message: {:?}", e);
Err(MercuryError)
}
Ok(parsed_msg) => Ok(parsed_msg),
},
},
}
}
}
}
}
@ -192,101 +229,125 @@ pub struct Artist {
impl Metadata for Track {
type Message = protocol::metadata::Track;
fn request_url(id: SpotifyId) -> String {
format!("hm://metadata/3/track/{}", id.to_base16())
fn request_url(id: SpotifyId) -> Result<String, FromUtf8Error> {
let id = id.to_base16()?;
Ok(format!("hm://metadata/3/track/{}", id))
}
fn parse(msg: &Self::Message, session: &Session) -> Self {
fn parse(msg: &Self::Message, session: &Session) -> Result<Self, SpotifyIdError> {
let country = session.country();
let artists = msg
.get_artist()
.iter()
.filter(|artist| artist.has_gid())
.map(|artist| SpotifyId::from_raw(artist.get_gid()).unwrap())
.collect::<Vec<_>>();
.filter_map(|artist| {
if artist.has_gid() {
SpotifyId::from_raw(artist.get_gid()).ok()
} else {
None
}
})
.collect();
let files = msg
.get_file()
.iter()
.filter(|file| file.has_file_id())
.map(|file| {
.filter_map(|file| {
if file.has_file_id() {
let mut dst = [0u8; 20];
dst.clone_from_slice(file.get_file_id());
(file.get_format(), FileId(dst))
Some((file.get_format(), FileId(dst)))
} else {
None
}
})
.collect();
Track {
id: SpotifyId::from_raw(msg.get_gid()).unwrap(),
Ok(Track {
id: SpotifyId::from_raw(msg.get_gid())?,
name: msg.get_name().to_owned(),
duration: msg.get_duration(),
album: SpotifyId::from_raw(msg.get_album().get_gid()).unwrap(),
album: SpotifyId::from_raw(msg.get_album().get_gid())?,
artists,
files,
alternatives: msg
.get_alternative()
.iter()
.map(|alt| SpotifyId::from_raw(alt.get_gid()).unwrap())
.filter_map(|alt| SpotifyId::from_raw(alt.get_gid()).ok())
.collect(),
available: parse_restrictions(msg.get_restriction(), &country, "premium"),
}
})
}
}
impl Metadata for Album {
type Message = protocol::metadata::Album;
fn request_url(id: SpotifyId) -> String {
format!("hm://metadata/3/album/{}", id.to_base16())
fn request_url(id: SpotifyId) -> Result<String, FromUtf8Error> {
let id = id.to_base16()?;
Ok(format!("hm://metadata/3/album/{}", id))
}
fn parse(msg: &Self::Message, _: &Session) -> Self {
fn parse(msg: &Self::Message, _: &Session) -> Result<Self, SpotifyIdError> {
let artists = msg
.get_artist()
.iter()
.filter(|artist| artist.has_gid())
.map(|artist| SpotifyId::from_raw(artist.get_gid()).unwrap())
.collect::<Vec<_>>();
.filter_map(|artist| {
if artist.has_gid() {
SpotifyId::from_raw(artist.get_gid()).ok()
} else {
None
}
})
.collect();
let tracks = msg
.get_disc()
.iter()
.flat_map(|disc| disc.get_track())
.filter(|track| track.has_gid())
.map(|track| SpotifyId::from_raw(track.get_gid()).unwrap())
.collect::<Vec<_>>();
.filter_map(|track| {
if track.has_gid() {
SpotifyId::from_raw(track.get_gid()).ok()
} else {
None
}
})
.collect();
let covers = msg
.get_cover_group()
.get_image()
.iter()
.filter(|image| image.has_file_id())
.map(|image| {
.filter_map(|image| {
if image.has_file_id() {
let mut dst = [0u8; 20];
dst.clone_from_slice(image.get_file_id());
FileId(dst)
Some(FileId(dst))
} else {
None
}
})
.collect::<Vec<_>>();
.collect();
Album {
id: SpotifyId::from_raw(msg.get_gid()).unwrap(),
Ok(Album {
id: SpotifyId::from_raw(msg.get_gid())?,
name: msg.get_name().to_owned(),
artists,
tracks,
covers,
}
})
}
}
impl Metadata for Playlist {
type Message = protocol::playlist4changes::SelectedListContent;
fn request_url(id: SpotifyId) -> String {
format!("hm://playlist/v2/playlist/{}", id.to_base62())
fn request_url(id: SpotifyId) -> Result<String, FromUtf8Error> {
let id = id.to_base62()?;
Ok(format!("hm://playlist/v2/playlist/{}", id))
}
fn parse(msg: &Self::Message, _: &Session) -> Self {
fn parse(msg: &Self::Message, _: &Session) -> Result<Self, SpotifyIdError> {
let tracks = msg
.get_contents()
.get_items()
@ -306,23 +367,24 @@ impl Metadata for Playlist {
);
}
Playlist {
Ok(Playlist {
revision: msg.get_revision().to_vec(),
name: msg.get_attributes().get_name().to_owned(),
tracks,
user: msg.get_owner_username().to_string(),
}
})
}
}
impl Metadata for Artist {
type Message = protocol::metadata::Artist;
fn request_url(id: SpotifyId) -> String {
format!("hm://metadata/3/artist/{}", id.to_base16())
fn request_url(id: SpotifyId) -> Result<String, FromUtf8Error> {
let id = id.to_base16()?;
Ok(format!("hm://metadata/3/artist/{}", id))
}
fn parse(msg: &Self::Message, session: &Session) -> Self {
fn parse(msg: &Self::Message, session: &Session) -> Result<Self, SpotifyIdError> {
let country = session.country();
let top_tracks: Vec<SpotifyId> = match msg
@ -333,17 +395,22 @@ impl Metadata for Artist {
Some(tracks) => tracks
.get_track()
.iter()
.filter(|track| track.has_gid())
.map(|track| SpotifyId::from_raw(track.get_gid()).unwrap())
.collect::<Vec<_>>(),
.filter_map(|track| {
if track.has_gid() {
SpotifyId::from_raw(track.get_gid()).ok()
} else {
None
}
})
.collect(),
None => Vec::new(),
};
Artist {
id: SpotifyId::from_raw(msg.get_gid()).unwrap(),
Ok(Artist {
id: SpotifyId::from_raw(msg.get_gid())?,
name: msg.get_name().to_owned(),
top_tracks,
}
})
}
}
@ -351,11 +418,12 @@ impl Metadata for Artist {
impl Metadata for Episode {
type Message = protocol::metadata::Episode;
fn request_url(id: SpotifyId) -> String {
format!("hm://metadata/3/episode/{}", id.to_base16())
fn request_url(id: SpotifyId) -> Result<String, FromUtf8Error> {
let id = id.to_base16()?;
Ok(format!("hm://metadata/3/episode/{}", id))
}
fn parse(msg: &Self::Message, session: &Session) -> Self {
fn parse(msg: &Self::Message, session: &Session) -> Result<Self, SpotifyIdError> {
let country = session.country();
let files = msg
@ -379,9 +447,9 @@ impl Metadata for Episode {
dst.clone_from_slice(image.get_file_id());
FileId(dst)
})
.collect::<Vec<_>>();
.collect();
Episode {
Ok(Episode {
id: SpotifyId::from_raw(msg.get_gid()).unwrap(),
name: msg.get_name().to_owned(),
external_url: msg.get_external_url().to_owned(),
@ -392,24 +460,30 @@ impl Metadata for Episode {
files,
available: parse_restrictions(msg.get_restriction(), &country, "premium"),
explicit: msg.get_explicit().to_owned(),
}
})
}
}
impl Metadata for Show {
type Message = protocol::metadata::Show;
fn request_url(id: SpotifyId) -> String {
format!("hm://metadata/3/show/{}", id.to_base16())
fn request_url(id: SpotifyId) -> Result<String, FromUtf8Error> {
let id = id.to_base16()?;
Ok(format!("hm://metadata/3/show/{}", id))
}
fn parse(msg: &Self::Message, _: &Session) -> Self {
fn parse(msg: &Self::Message, _: &Session) -> Result<Self, SpotifyIdError> {
let episodes = msg
.get_episode()
.iter()
.filter(|episode| episode.has_gid())
.map(|episode| SpotifyId::from_raw(episode.get_gid()).unwrap())
.collect::<Vec<_>>();
.filter_map(|episode| {
if episode.has_gid() {
SpotifyId::from_raw(episode.get_gid()).ok()
} else {
None
}
})
.collect();
let covers = msg
.get_covers()
@ -421,15 +495,15 @@ impl Metadata for Show {
dst.clone_from_slice(image.get_file_id());
FileId(dst)
})
.collect::<Vec<_>>();
.collect();
Show {
Ok(Show {
id: SpotifyId::from_raw(msg.get_gid()).unwrap(),
name: msg.get_name().to_owned(),
publisher: msg.get_publisher().to_owned(),
episodes,
covers,
}
})
}
}

View file

@ -740,7 +740,10 @@ impl PlayerTrackLoader {
let audio = match self.find_available_alternative(audio).await {
Some(audio) => audio,
None => {
warn!("<{}> is not available", spotify_id.to_uri());
warn!(
"<{}> is not available",
spotify_id.to_uri().unwrap_or_default()
);
return None;
}
};
@ -748,7 +751,7 @@ impl PlayerTrackLoader {
if audio.duration < 0 {
error!(
"Track duration for <{}> cannot be {}",
spotify_id.to_uri(),
spotify_id.to_uri().unwrap_or_default(),
audio.duration
);
return None;

View file

@ -5,6 +5,7 @@ use tokio::process::{Child as AsyncChild, Command as AsyncCommand};
use std::collections::HashMap;
use std::io;
use std::io::{Error, ErrorKind};
use std::process::{Command, ExitStatus};
pub fn run_program_on_events(event: PlayerEvent, onevent: &str) -> Option<io::Result<AsyncChild>> {
@ -13,45 +14,110 @@ pub fn run_program_on_events(event: PlayerEvent, onevent: &str) -> Option<io::Re
PlayerEvent::Changed {
old_track_id,
new_track_id,
} => {
} => match old_track_id.to_base62() {
Err(e) => {
return Some(Err(Error::new(
ErrorKind::InvalidData,
format!(
"PlayerEvent::Changed: Invalid old track id: {}",
e.utf8_error()
),
)))
}
Ok(old_id) => match new_track_id.to_base62() {
Err(e) => {
return Some(Err(Error::new(
ErrorKind::InvalidData,
format!(
"PlayerEvent::Changed: Invalid new track id: {}",
e.utf8_error()
),
)))
}
Ok(new_id) => {
env_vars.insert("PLAYER_EVENT", "changed".to_string());
env_vars.insert("OLD_TRACK_ID", old_track_id.to_base62());
env_vars.insert("TRACK_ID", new_track_id.to_base62());
env_vars.insert("OLD_TRACK_ID", old_id);
env_vars.insert("TRACK_ID", new_id);
}
PlayerEvent::Started { track_id, .. } => {
},
},
PlayerEvent::Started { track_id, .. } => match track_id.to_base62() {
Err(e) => {
return Some(Err(Error::new(
ErrorKind::InvalidData,
format!("PlayerEvent::Started: Invalid track id: {}", e.utf8_error()),
)))
}
Ok(id) => {
env_vars.insert("PLAYER_EVENT", "started".to_string());
env_vars.insert("TRACK_ID", track_id.to_base62());
env_vars.insert("TRACK_ID", id);
}
PlayerEvent::Stopped { track_id, .. } => {
},
PlayerEvent::Stopped { track_id, .. } => match track_id.to_base62() {
Err(e) => {
return Some(Err(Error::new(
ErrorKind::InvalidData,
format!("PlayerEvent::Stopped: Invalid track id: {}", e.utf8_error()),
)))
}
Ok(id) => {
env_vars.insert("PLAYER_EVENT", "stopped".to_string());
env_vars.insert("TRACK_ID", track_id.to_base62());
env_vars.insert("TRACK_ID", id);
}
},
PlayerEvent::Playing {
track_id,
duration_ms,
position_ms,
..
} => {
} => match track_id.to_base62() {
Err(e) => {
return Some(Err(Error::new(
ErrorKind::InvalidData,
format!("PlayerEvent::Playing: Invalid track id: {}", e.utf8_error()),
)))
}
Ok(id) => {
env_vars.insert("PLAYER_EVENT", "playing".to_string());
env_vars.insert("TRACK_ID", track_id.to_base62());
env_vars.insert("TRACK_ID", id);
env_vars.insert("DURATION_MS", duration_ms.to_string());
env_vars.insert("POSITION_MS", position_ms.to_string());
}
},
PlayerEvent::Paused {
track_id,
duration_ms,
position_ms,
..
} => {
} => match track_id.to_base62() {
Err(e) => {
return Some(Err(Error::new(
ErrorKind::InvalidData,
format!("PlayerEvent::Paused: Invalid track id: {}", e.utf8_error()),
)))
}
Ok(id) => {
env_vars.insert("PLAYER_EVENT", "paused".to_string());
env_vars.insert("TRACK_ID", track_id.to_base62());
env_vars.insert("TRACK_ID", id);
env_vars.insert("DURATION_MS", duration_ms.to_string());
env_vars.insert("POSITION_MS", position_ms.to_string());
}
PlayerEvent::Preloading { track_id, .. } => {
env_vars.insert("PLAYER_EVENT", "preloading".to_string());
env_vars.insert("TRACK_ID", track_id.to_base62());
},
PlayerEvent::Preloading { track_id, .. } => match track_id.to_base62() {
Err(e) => {
return Some(Err(Error::new(
ErrorKind::InvalidData,
format!(
"PlayerEvent::Preloading: Invalid track id: {}",
e.utf8_error()
),
)))
}
Ok(id) => {
env_vars.insert("PLAYER_EVENT", "preloading".to_string());
env_vars.insert("TRACK_ID", id);
}
},
PlayerEvent::VolumeSet { volume } => {
env_vars.insert("PLAYER_EVENT", "volume_set".to_string());
env_vars.insert("VOLUME", volume.to_string());