From 303694ed20648025e1a5fde0a4bdc6f268983293 Mon Sep 17 00:00:00 2001 From: Michael Herstine Date: Thu, 28 Dec 2023 16:19:17 -0800 Subject: [PATCH] [#9] Correct the way quoting is done when sending commands. This commit will cause the client to properly quote string parameters in commands. --- mpdpopm/src/clients.rs | 42 +++++++++++++++++++++++++++------------ mpdpopm/src/playcounts.rs | 15 ++++++-------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/mpdpopm/src/clients.rs b/mpdpopm/src/clients.rs index fcd0312..5bdf15c 100644 --- a/mpdpopm/src/clients.rs +++ b/mpdpopm/src/clients.rs @@ -658,7 +658,7 @@ impl Client { where ::Err: std::error::Error + Sync + Send + 'static, { - let msg = format!("sticker get song \"{}\" \"{}\"", file, sticker_name); + let msg = format!("sticker get song {} {}", quote(file), quote(sticker_name)); let text = self.stream.req(&msg).await?; debug!("Sent message `{}'; got `{}'", &msg, &text); @@ -689,9 +689,12 @@ impl Client { sticker_name: &str, sticker_value: &T, ) -> Result<()> { + let value_as_str = format!("{}", sticker_value); let msg = format!( - "sticker set song \"{}\" \"{}\" \"{}\"", - file, sticker_name, sticker_value + "sticker set song {} {} {}", + quote(file), + quote(sticker_name), + quote(&value_as_str) ); let text = self.stream.req(&msg).await?; debug!("Sent `{}'; got `{}'", &msg, &text); @@ -701,7 +704,7 @@ impl Client { /// Send a file to a playlist pub async fn send_to_playlist(&mut self, file: &str, pl: &str) -> Result<()> { - let msg = format!("playlistadd \"{}\" \"{}\"", pl, file); + let msg = format!("playlistadd {} {}", quote(pl), quote(file)); let text = self.stream.req(&msg).await?; debug!("Sent `{}'; got `{}'.", &msg, &text); @@ -955,7 +958,7 @@ mod client_tests { #[tokio::test] async fn client_smoke_test() { let mock = Box::new(Mock::new(&[( - "sticker get song \"foo.mp3\" \"stick\"", + "sticker get song foo.mp3 stick", "sticker: stick=splat\nOK\n", )])); let mut cli = Client::new(mock).unwrap(); @@ -1063,20 +1066,24 @@ state: no-idea!?", async fn test_get_sticker() { let mock = Box::new(Mock::new(&[ ( - "sticker get song \"foo.mp3\" \"stick\"", + "sticker get song foo.mp3 stick", // On success, should get something like this... "sticker: stick=2\nOK\n", ), ( - "sticker get song \"foo.mp3\" \"stick\"", + "sticker get song foo.mp3 stick", // and on failure, something like this: "ACK [50@0] {sticker} no such sticker\n", ), ( - "sticker get song \"foo.mp3\" \"stick\"", + "sticker get song foo.mp3 stick", // Finally, let's try something nuts "", ), + ( + "sticker get song \"filename_with\\\"doublequotes\\\".flac\" unwoundstack.com:playcount", + "sticker: unwoundstack.com:playcount=11\nOK\n", + ), ])); let mut cli = Client::new(mock).unwrap(); let val = cli @@ -1094,19 +1101,28 @@ state: no-idea!?", .get_sticker::("foo.mp3", "stick") .await .unwrap_err(); + let val = cli + .get_sticker::( + "filename_with\"doublequotes\".flac", + "unwoundstack.com:playcount", + ) + .await + .unwrap() + .unwrap(); + assert_eq!(val, "11"); } /// Test the `set_sticker' method #[tokio::test] async fn test_set_sticker() { let mock = Box::new(Mock::new(&[ - ("sticker set song \"foo.mp3\" \"stick\" \"2\"", "OK\n"), + ("sticker set song foo.mp3 stick 2", "OK\n"), ( - "sticker set song \"foo.mp3\" \"stick\" \"2\"", + "sticker set song foo.mp3 stick 2", "ACK [50@0] {sticker} some error", ), ( - "sticker set song \"foo.mp3\" \"stick\" \"2\"", + "sticker set song foo.mp3 stick 2", "this makes no sense as a response", ), ])); @@ -1120,9 +1136,9 @@ state: no-idea!?", #[tokio::test] async fn test_send_to_playlist() { let mock = Box::new(Mock::new(&[ - ("playlistadd \"foo.m3u\" \"foo.mp3\"", "OK\n"), + ("playlistadd foo.m3u foo.mp3", "OK\n"), ( - "playlistadd \"foo.m3u\" \"foo.mp3\"", + "playlistadd foo.m3u foo.mp3", "ACK [101@0] {playlist} some error\n", ), ])); diff --git a/mpdpopm/src/playcounts.rs b/mpdpopm/src/playcounts.rs index fce5b04..efa14e0 100644 --- a/mpdpopm/src/playcounts.rs +++ b/mpdpopm/src/playcounts.rs @@ -206,12 +206,12 @@ mod pc_lp_tests { #[tokio::test] async fn pc_smoke() { let mock = Box::new(Mock::new(&[ - ("sticker get song \"a\" \"pc\"", "sticker: pc=11\nOK\n"), + ("sticker get song a pc", "sticker: pc=11\nOK\n"), ( - "sticker get song \"a\" \"pc\"", + "sticker get song a pc", "ACK [50@0] {sticker} no such sticker\n", ), - ("sticker get song \"a\" \"pc\"", "splat!"), + ("sticker get song a pc", "splat!"), ])); let mut cli = Client::new(mock).unwrap(); @@ -474,12 +474,12 @@ OK ", ), ( - "sticker get song \"E/Enya - Wild Child.mp3\" \"pc\"", + "sticker get song \"E/Enya - Wild Child.mp3\" pc", "sticker: pc=11\nOK\n", ), ( &format!( - "sticker set song \"E/Enya - Wild Child.mp3\" \"lp\" \"{}\"", + "sticker set song \"E/Enya - Wild Child.mp3\" lp {}", SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) .unwrap() @@ -487,10 +487,7 @@ OK ), "OK\n", ), - ( - "sticker set song \"E/Enya - Wild Child.mp3\" \"pc\" \"12\"", - "OK\n", - ), + ("sticker set song \"E/Enya - Wild Child.mp3\" pc 12", "OK\n"), ])); let mut cli = Client::new(mock).unwrap();