diff --git a/rust_backend/crates/core/src/fs.rs b/rust_backend/crates/core/src/fs.rs new file mode 100644 index 00000000..05d9525f --- /dev/null +++ b/rust_backend/crates/core/src/fs.rs @@ -0,0 +1,85 @@ +//! Filesystem path helpers shared by the native and extension backends. + +use std::path::{Component, Path, PathBuf}; + +/// Lexically cleans a path like Go's `filepath.Clean`: drops `.` components, +/// folds `name/..`, keeps leading `..` on relative paths, drops `..` above the +/// root, and returns `.` for an empty result. The filesystem is not consulted. +pub fn clean(path: &Path) -> PathBuf { + let mut result = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir => { + if result.file_name().is_some_and(|name| name != "..") { + result.pop(); + } else if !result.has_root() { + result.push(".."); + } + } + _ => result.push(component), + } + } + if result.as_os_str().is_empty() { + ".".into() + } else { + result + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // The copy `isrc::native_files` used before it shared this helper. + fn previous_isrc_clean(path: &Path) -> PathBuf { + let mut output = PathBuf::new(); + for component in path.components() { + match component { + Component::CurDir => {} + Component::ParentDir if output.file_name().is_some_and(|name| name != "..") => { + output.pop(); + } + Component::ParentDir if output.has_root() => {} + other => output.push(other.as_os_str()), + } + } + if output.as_os_str().is_empty() { + output.push("."); + } + output + } + + #[test] + fn clean_matches_previous_isrc_copy_and_go_results() { + let cases = [ + ("", "."), + (".", "."), + ("./", "."), + ("..", ".."), + ("../..", "../.."), + ("../../a", "../../a"), + ("a/..", "."), + ("a/../..", ".."), + ("a/b/../../..", ".."), + ("a/./b/", "a/b"), + ("a//b///c", "a/b/c"), + ("a/b/", "a/b"), + ("/", "/"), + ("/..", "/"), + ("/../a", "/a"), + ("/a/b/../c/./d/", "/a/c/d"), + ("/a/..", "/"), + ("//a", "/a"), + ("a/...", "a/..."), + ("a/.../..", "a"), + ("..a/../b", "b"), + ("\u{00e9}/\u{1f3b5}/../x", "\u{00e9}/x"), + ]; + for (input, expected) in cases { + let input = Path::new(input); + assert_eq!(clean(input), Path::new(expected), "{input:?}"); + assert_eq!(clean(input), previous_isrc_clean(input), "{input:?}"); + } + } +} diff --git a/rust_backend/crates/core/src/isrc/native_files.rs b/rust_backend/crates/core/src/isrc/native_files.rs index 44b25c69..0eb4896e 100644 --- a/rust_backend/crates/core/src/isrc/native_files.rs +++ b/rust_backend/crates/core/src/isrc/native_files.rs @@ -1,6 +1,7 @@ use super::{FileStamp, IndexFiles}; +use crate::fs::clean; use std::fs::{self, OpenOptions}; -use std::path::{Component, Path, PathBuf}; +use std::path::{Path, PathBuf}; use std::time::UNIX_EPOCH; /// Ambient filesystem access for the trusted native application. JavaScript @@ -90,21 +91,3 @@ fn stamp(path: &Path, metadata: &fs::Metadata) -> FileStamp { directory: metadata.is_dir(), } } - -fn clean(path: &Path) -> PathBuf { - let mut output = PathBuf::new(); - for component in path.components() { - match component { - Component::CurDir => {} - Component::ParentDir if output.file_name().is_some_and(|name| name != "..") => { - output.pop(); - } - Component::ParentDir if output.has_root() => {} - other => output.push(other.as_os_str()), - } - } - if output.as_os_str().is_empty() { - output.push("."); - } - output -} diff --git a/rust_backend/crates/core/src/lib.rs b/rust_backend/crates/core/src/lib.rs index f00f1fd0..dbc5a78b 100644 --- a/rust_backend/crates/core/src/lib.rs +++ b/rust_backend/crates/core/src/lib.rs @@ -11,6 +11,7 @@ pub mod cover; pub mod cue; pub mod downloads; pub mod filename; +pub mod fs; pub mod isrc; pub mod json; pub mod lyrics; @@ -24,4 +25,4 @@ pub mod resolver; pub mod tags; mod text; pub mod xml; -pub use text::json_surrogates as normalize_json_surrogates; +pub use text::{json_surrogates as normalize_json_surrogates, lower_hex}; diff --git a/rust_backend/crates/core/src/tags/id3.rs b/rust_backend/crates/core/src/tags/id3.rs index f0560701..a5591693 100644 --- a/rust_backend/crates/core/src/tags/id3.rs +++ b/rust_backend/crates/core/src/tags/id3.rs @@ -501,7 +501,7 @@ fn v1(reader: &mut (impl Read + Seek)) -> Result { }) } -fn syncsafe(data: &[u8]) -> u64 { +pub(crate) fn syncsafe(data: &[u8]) -> u64 { u64::from(data[0]) << 21 | u64::from(data[1]) << 14 | u64::from(data[2]) << 7 diff --git a/rust_backend/crates/core/src/tags/write/id3.rs b/rust_backend/crates/core/src/tags/write/id3.rs index 8c565671..c285aa50 100644 --- a/rust_backend/crates/core/src/tags/write/id3.rs +++ b/rust_backend/crates/core/src/tags/write/id3.rs @@ -211,10 +211,7 @@ fn syncsafe(size: usize) -> [u8; 4] { fn size(data: &[u8], syncsafe: bool) -> usize { if syncsafe { - ((data[0] as usize) << 21) - | ((data[1] as usize) << 14) - | ((data[2] as usize) << 7) - | data[3] as usize + crate::tags::id3::syncsafe(data) as usize } else { u32::from_be_bytes(data.try_into().unwrap()) as usize } @@ -379,3 +376,38 @@ fn read_frames( } Ok((frames, audio_start)) } + +#[cfg(test)] +mod tests { + use super::*; + + // The decoder `size` inlined before it shared the reader's `syncsafe`. + fn previous_syncsafe_size(data: &[u8]) -> usize { + ((data[0] as usize) << 21) + | ((data[1] as usize) << 14) + | ((data[2] as usize) << 7) + | data[3] as usize + } + + #[test] + fn syncsafe_size_matches_previous_decoder_across_bit_boundaries() { + assert_eq!(size(&[0, 0, 0, 0], true), 0); + assert_eq!(size(&[0, 0, 0, 0x7f], true), 0x7f); + assert_eq!(size(&[0, 0, 1, 0], true), 0x80); + assert_eq!(size(&[0x7f, 0x7f, 0x7f, 0x7f], true), 0x0fff_ffff); + assert_eq!(size(&[0, 0, 0, 0x80], true), 0x80); + assert_eq!(size(&[0xff, 0xff, 0xff, 0xff], true), 0x1fff_ffff); + assert_eq!(size(&[0, 0, 1, 0, 9], true), 0x80); + let values = [0x00, 0x01, 0x7f, 0x80, 0xff]; + for a in values { + for b in values { + for c in values { + for d in values { + let data = [a, b, c, d]; + assert_eq!(size(&data, true), previous_syncsafe_size(&data), "{data:?}"); + } + } + } + } + } +} diff --git a/rust_backend/crates/core/src/text.rs b/rust_backend/crates/core/src/text.rs index fc390b84..e0c939dc 100644 --- a/rust_backend/crates/core/src/text.rs +++ b/rust_backend/crates/core/src/text.rs @@ -67,3 +67,28 @@ pub fn json_surrogates(json: &str) -> std::borrow::Cow<'_, str> { std::borrow::Cow::Owned(output) } } + +/// Lowercase hexadecimal, two digits per byte (Go's `hex.EncodeToString`). +pub fn lower_hex(bytes: &[u8]) -> String { + use std::fmt::Write; + let mut output = String::with_capacity(bytes.len() * 2); + for byte in bytes { + let _ = write!(output, "{byte:02x}"); + } + output +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn lower_hex_matches_per_byte_format_for_every_byte() { + assert_eq!(lower_hex(&[]), ""); + assert_eq!(lower_hex(&[0x00, 0x0f, 0x7f, 0x80, 0xff]), "000f7f80ff"); + let every: Vec = (0..=u8::MAX).collect(); + // The `format!` per byte form used by the mobile digest helpers. + let previous: String = every.iter().map(|byte| format!("{byte:02x}")).collect(); + assert_eq!(lower_hex(&every), previous); + } +} diff --git a/rust_backend/crates/extensions/src/binary.rs b/rust_backend/crates/extensions/src/binary.rs index 4e55bb3a..ec47a1cf 100644 --- a/rust_backend/crates/extensions/src/binary.rs +++ b/rust_backend/crates/extensions/src/binary.rs @@ -139,14 +139,7 @@ pub(crate) fn decode_option_bytes( pub(crate) fn encode(bytes: &[u8], encoding: &str) -> Result { match encoding.trim().to_lowercase().as_str() { "" | "base64" => Ok(STANDARD.encode(bytes)), - "hex" => { - use std::fmt::Write; - let mut value = String::with_capacity(bytes.len() * 2); - for byte in bytes { - let _ = write!(value, "{byte:02x}"); - } - Ok(value) - } + "hex" => Ok(spotiflac_core::lower_hex(bytes)), "utf8" | "utf-8" | "text" => Ok(decode_go_utf8(bytes)), _ => Err(format!("unsupported byte encoding: {encoding}")), } diff --git a/rust_backend/crates/extensions/src/files.rs b/rust_backend/crates/extensions/src/files.rs index a1e508dd..7fbe5c9f 100644 --- a/rust_backend/crates/extensions/src/files.rs +++ b/rust_backend/crates/extensions/src/files.rs @@ -4,6 +4,7 @@ mod index; use cap_std::ambient_authority; use cap_std::fs::{Dir, File, OpenOptions}; +pub(crate) use spotiflac_core::fs::clean; use std::collections::BTreeMap; use std::io; use std::path::{Component, Path, PathBuf}; @@ -679,27 +680,5 @@ impl Drop for OutputGuard { } } -pub(crate) fn clean(path: &Path) -> PathBuf { - let mut result = PathBuf::new(); - for component in path.components() { - match component { - Component::CurDir => {} - Component::ParentDir => { - if result.file_name().is_some_and(|name| name != "..") { - result.pop(); - } else if !result.has_root() { - result.push(".."); - } - } - _ => result.push(component), - } - } - if result.as_os_str().is_empty() { - ".".into() - } else { - result - } -} - #[cfg(test)] mod tests; diff --git a/rust_backend/crates/extensions/src/repository.rs b/rust_backend/crates/extensions/src/repository.rs index b70318c5..1094b6c9 100644 --- a/rust_backend/crates/extensions/src/repository.rs +++ b/rust_backend/crates/extensions/src/repository.rs @@ -500,22 +500,14 @@ fn read_registry( let status = stream.response.status; let mut body = Vec::new(); if status == 200 { - let mut buffer = [0; 64 * 1024]; - loop { - let limit = buffer.len().min(MAX_REGISTRY_BYTES + 1 - body.len()); - let count = stream - .read(&mut buffer[..limit], check) - .map_err(|e| cause("failed to read registry", e))?; - if count == 0 { - break; - } - body.extend_from_slice(&buffer[..count]); - if body.len() > MAX_REGISTRY_BYTES { - return Err(error(format!( + body = stream + .read_limited(MAX_REGISTRY_BYTES, &mut [0; 64 * 1024], check) + .map_err(|e| cause("failed to read registry", e))? + .ok_or_else(|| { + error(format!( "registry response exceeds {MAX_REGISTRY_BYTES} bytes" - ))); - } - } + )) + })?; } Ok((status, stream.response.headers, body)) } diff --git a/rust_backend/crates/extensions/src/signed_session/protocol.rs b/rust_backend/crates/extensions/src/signed_session/protocol.rs index 7b5626c8..6672f888 100644 --- a/rust_backend/crates/extensions/src/signed_session/protocol.rs +++ b/rust_backend/crates/extensions/src/signed_session/protocol.rs @@ -4,6 +4,7 @@ use hmac::{Hmac, KeyInit, Mac}; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; use sha2::{Digest, Sha256}; +use spotiflac_core::lower_hex as hex; use spotiflac_network::{HttpResponse, query, url::UrlParts}; use std::collections::BTreeMap; use std::time::{Duration, UNIX_EPOCH}; @@ -374,15 +375,6 @@ pub fn random_hex(count: usize) -> Result { Ok(hex(&bytes)) } -fn hex(bytes: &[u8]) -> String { - use std::fmt::Write; - let mut output = String::with_capacity(bytes.len() * 2); - for byte in bytes { - let _ = write!(output, "{byte:02x}"); - } - output -} - fn hmac(key: &[u8], message: &[u8]) -> String { let mut mac = Hmac::::new_from_slice(key).expect("HMAC accepts any key length"); mac.update(message); diff --git a/rust_backend/crates/mobile/src/data_jobs.rs b/rust_backend/crates/mobile/src/data_jobs.rs index a515dff0..106e872d 100644 --- a/rust_backend/crates/mobile/src/data_jobs.rs +++ b/rust_backend/crates/mobile/src/data_jobs.rs @@ -179,11 +179,7 @@ pub(crate) fn hash_file( hash.update(&buffer[..count]); } check()?; - Ok(hash - .finalize() - .iter() - .map(|byte| format!("{byte:02x}")) - .collect()) + Ok(spotiflac_core::lower_hex(&hash.finalize())) } #[cfg(test)] diff --git a/rust_backend/crates/mobile/src/native_backup.rs b/rust_backend/crates/mobile/src/native_backup.rs index 027a89bc..a83e04d2 100644 --- a/rust_backend/crates/mobile/src/native_backup.rs +++ b/rust_backend/crates/mobile/src/native_backup.rs @@ -4,10 +4,8 @@ mod archive; mod collections; mod legacy; -use crate::native_library::dart_date; -use crate::native_library::path_keys::{ - build, dart_lower, dart_trim, dart_upper, regex_whitespace, -}; +use crate::native_library::path_keys::{build, dart_trim, dart_upper, regex_whitespace}; +use crate::native_library::{dart_date, normalized}; use rusqlite::{Connection, OpenFlags, TransactionBehavior, params, params_from_iter}; use serde_json::{Map, Value, json}; use std::{ @@ -140,9 +138,6 @@ fn number(value: &Value) -> Option { .as_i64() .or_else(|| value.as_f64().filter(|v| v.is_finite()).map(|v| v as i64)) } -fn normalized(value: &str) -> String { - dart_lower(dart_trim(value)) -} fn date(value: &str) -> Option<(String, i64)> { let parsed = dart_date::parse(value)?; Some(( diff --git a/rust_backend/crates/mobile/src/native_library.rs b/rust_backend/crates/mobile/src/native_library.rs index ec2b76c7..7c23a0fd 100644 --- a/rust_backend/crates/mobile/src/native_library.rs +++ b/rust_backend/crates/mobile/src/native_library.rs @@ -159,7 +159,7 @@ pub(crate) fn scan_incremental( import(request, Some(&delta), check) } -fn normalized(value: &str) -> String { +pub(crate) fn normalized(value: &str) -> String { path_keys::dart_lower(path_keys::dart_trim(value)) } diff --git a/rust_backend/crates/mobile/src/native_library/path_keys.rs b/rust_backend/crates/mobile/src/native_library/path_keys.rs index 67c9315c..611e178e 100644 --- a/rust_backend/crates/mobile/src/native_library/path_keys.rs +++ b/rust_backend/crates/mobile/src/native_library/path_keys.rs @@ -15,8 +15,13 @@ const EXTENSIONS: &[&str] = &[ ".flac", ".m4a", ".mp3", ".opus", ".ogg", ".wav", ".aiff", ".aif", ".aac", ]; +/// Dart's `String.trim` set: Unicode White_Space plus the U+FEFF BOM. +pub(crate) fn dart_whitespace(ch: char) -> bool { + ch.is_whitespace() || ch == '\u{feff}' +} + pub(crate) fn dart_trim(value: &str) -> &str { - value.trim_matches(|ch: char| ch.is_whitespace() || ch == '\u{feff}') + value.trim_matches(dart_whitespace) } /// Dart applies Unicode simple casing, without multi-character expansions or @@ -328,4 +333,51 @@ mod tests { .any(|key| key == "/storage/emulated/0/Music/Café.flac") ); } + + // The explicit set `native_lyrics` used before it shared `dart_trim`. + fn previous_lyrics_whitespace(c: char) -> bool { + matches!( + c, + '\t' | '\n' + | '\u{000b}' + | '\u{000c}' + | '\r' + | ' ' + | '\u{0085}' + | '\u{00a0}' + | '\u{1680}' + | '\u{2028}' + | '\u{2029}' + | '\u{202f}' + | '\u{205f}' + | '\u{3000}' + | '\u{feff}' + ) || ('\u{2000}'..='\u{200a}').contains(&c) + } + + #[test] + fn dart_whitespace_equals_previous_lyrics_set_for_every_scalar() { + for c in (0..=char::MAX as u32).filter_map(char::from_u32) { + assert_eq!(dart_whitespace(c), previous_lyrics_whitespace(c), "{c:?}"); + } + for (input, expected) in [ + ("", ""), + ("\u{feff}", ""), + ("\u{feff} a b \u{3000}", "a b"), + ("\u{0085}x\u{200a}", "x"), + ("\u{200b}x\u{200b}", "\u{200b}x\u{200b}"), + ("\u{180e}x", "\u{180e}x"), + ( + "\t\n\u{000b}\u{000c}\r Caf\u{00e9} \u{1f3b5}\u{00a0}", + "Caf\u{00e9} \u{1f3b5}", + ), + ] { + assert_eq!(dart_trim(input), expected, "{input:?}"); + assert_eq!( + dart_trim(input), + input.trim_matches(previous_lyrics_whitespace), + "{input:?}" + ); + } + } } diff --git a/rust_backend/crates/mobile/src/native_lyrics/text.rs b/rust_backend/crates/mobile/src/native_lyrics/text.rs index b4a58aa3..6aae89f0 100644 --- a/rust_backend/crates/mobile/src/native_lyrics/text.rs +++ b/rust_backend/crates/mobile/src/native_lyrics/text.rs @@ -18,28 +18,9 @@ static VOICE_PREFIX: LazyLock = LazyLock::new(|| { Regex::new(&format!(r"(?i)^({WS}*(?:<[0-9]{{1,3}}:[0-9]{{1,2}}(?:[.:][0-9]{{1,3}})?>{WS}*)*)(v[1-9][0-9]*):[ \t]*")).unwrap() }); -pub(super) fn whitespace(c: char) -> bool { - matches!( - c, - '\t' | '\n' - | '\u{000b}' - | '\u{000c}' - | '\r' - | ' ' - | '\u{0085}' - | '\u{00a0}' - | '\u{1680}' - | '\u{2028}' - | '\u{2029}' - | '\u{202f}' - | '\u{205f}' - | '\u{3000}' - | '\u{feff}' - ) || ('\u{2000}'..='\u{200a}').contains(&c) -} -pub(super) fn trim(s: &str) -> &str { - s.trim_matches(whitespace) -} +pub(super) use crate::native_library::path_keys::{ + dart_trim as trim, dart_whitespace as whitespace, +}; pub(super) fn trim_end(s: &str) -> &str { s.trim_end_matches(whitespace) } diff --git a/rust_backend/crates/mobile/src/native_playlists.rs b/rust_backend/crates/mobile/src/native_playlists.rs index b0b73173..4558d519 100644 --- a/rust_backend/crates/mobile/src/native_playlists.rs +++ b/rust_backend/crates/mobile/src/native_playlists.rs @@ -1,5 +1,6 @@ //! The existing playlist import contract, executed on the native data worker. //! CSV remains line-based for compatibility with existing exports/imports. +use crate::native_library::path_keys::dart_trim as trim; use serde_json::{Value, json}; use std::{ collections::HashMap, @@ -89,10 +90,6 @@ fn lines(content: &str) -> Vec<&str> { result } -fn trim(value: &str) -> &str { - value.trim_matches(|character: char| character.is_whitespace() || character == '\u{feff}') -} - fn track( id: String, title: &str, diff --git a/rust_backend/crates/mobile/src/network_tags.rs b/rust_backend/crates/mobile/src/network_tags.rs index 2699823c..a2697a04 100644 --- a/rust_backend/crates/mobile/src/network_tags.rs +++ b/rust_backend/crates/mobile/src/network_tags.rs @@ -3,7 +3,7 @@ use base64::Engine; use serde_json::{Value, json}; use sha2::{Digest, Sha256}; -use spotiflac_core::tags; +use spotiflac_core::{lower_hex, tags}; use spotiflac_network::{ HttpRequest, NetworkService, NetworkSession, policy::NetworkPermissions, url::UrlParts, }; @@ -125,11 +125,7 @@ pub(crate) fn matches_upload( if received != length { return Ok(json!({"matches": false})); } - let remote: String = hash - .finalize() - .iter() - .map(|byte| format!("{byte:02x}")) - .collect(); + let remote = lower_hex(&hash.finalize()); let local = crate::data_jobs::hash_file(path, check)?; check()?; Ok(json!({"matches": remote == local})) @@ -191,10 +187,7 @@ fn cache_cover( return Err("Artwork cache must be absolute".into()); } fs::create_dir_all(directory).map_err(|error| error.to_string())?; - let digest: String = Sha256::digest(bytes) - .iter() - .map(|byte| format!("{byte:02x}")) - .collect(); + let digest = lower_hex(&Sha256::digest(bytes)); let target = directory.join(format!("network_cover_{digest}.image")); let mut staged = tempfile::NamedTempFile::new_in(directory).map_err(|error| error.to_string())?; diff --git a/rust_backend/crates/network/src/client/stream.rs b/rust_backend/crates/network/src/client/stream.rs index 5995febc..8828c0cd 100644 --- a/rust_backend/crates/network/src/client/stream.rs +++ b/rust_backend/crates/network/src/client/stream.rs @@ -87,6 +87,30 @@ impl HttpStream { result } + /// Collect the rest of the body through `buffer`, returning `Ok(None)` as + /// soon as it exceeds `limit` bytes. Each read requests at most one byte + /// past the limit, so an oversized body is never buffered further. Read + /// errors are returned unchanged for the caller to map. + pub fn read_limited( + &mut self, + limit: usize, + buffer: &mut [u8], + check: impl Fn() -> Result<(), String>, + ) -> Result>, String> { + let mut body = Vec::new(); + loop { + let capacity = buffer.len().min(limit + 1 - body.len()); + let count = self.read(&mut buffer[..capacity], &check)?; + if count == 0 { + return Ok(Some(body)); + } + body.extend_from_slice(&buffer[..count]); + if body.len() > limit { + return Ok(None); + } + } + } + /// Release an unwanted body (a range probe or retryable error). A small /// HTTP/1.1 body is finished within 20 ms so its connection can return to /// the pool; HTTP/2, compressed, large or slow bodies are dropped at once. diff --git a/rust_backend/crates/network/src/client/tests.rs b/rust_backend/crates/network/src/client/tests.rs index 1d627a2d..3cc78c92 100644 --- a/rust_backend/crates/network/src/client/tests.rs +++ b/rust_backend/crates/network/src/client/tests.rs @@ -86,6 +86,81 @@ fn direct_media_does_not_send_metadata_cookies_or_follow_redirects() { server.join().unwrap(); } +#[test] +fn read_limited_matches_the_bounded_read_loops_it_replaced() { + use std::io::{Read, Write}; + const BODY: &[u8] = b"0123456789"; + // (limit, buffer length): exact fit, one byte over, single-chunk and zero. + let cases = [(10, 4), (9, 4), (10, 64 << 10), (9, 64 << 10), (0, 4)]; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let server = std::thread::spawn(move || { + for _ in 0..cases.len() * 2 { + let (mut stream, _) = listener.accept().unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(3))) + .unwrap(); + let mut request = Vec::new(); + while !request.ends_with(b"\r\n\r\n") { + let mut byte = [0]; + stream.read_exact(&mut byte).unwrap(); + request.push(byte[0]); + } + stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 10\r\nConnection: close\r\n\r\n") + .unwrap(); + stream.write_all(BODY).unwrap(); + } + }); + let service = NetworkService::new().unwrap(); + service.set_allow_private_network(true); + let session = service.session( + NetworkPermissions { + domains: vec!["127.0.0.1".into()], + allow_http: true, + }, + Duration::from_secs(3), + ); + let open = || { + let request = + serde_json::from_value(serde_json::json!({"url":format!("{base}/body")})).unwrap(); + session + .open_stream( + request, + Duration::from_secs(3), + Duration::from_secs(3), + || Ok(()), + ) + .unwrap() + }; + // The loop shared by the lyrics, resolver and registry readers. + let previous = |stream: &mut HttpStream, limit: usize, buffer: &mut [u8]| { + let mut body = Vec::new(); + loop { + let capacity = buffer.len().min(limit + 1 - body.len()); + let count = stream.read(&mut buffer[..capacity], || Ok(()))?; + if count == 0 { + break; + } + body.extend_from_slice(&buffer[..count]); + if body.len() > limit { + return Ok::<_, String>(None); + } + } + Ok(Some(body)) + }; + for (limit, length) in cases { + let actual = open().read_limited(limit, &mut vec![0; length], || Ok(())); + let expected = previous(&mut open(), limit, &mut vec![0; length]); + assert_eq!(actual, expected, "limit {limit} buffer {length}"); + assert_eq!( + actual.unwrap(), + (limit >= BODY.len()).then(|| BODY.to_vec()) + ); + } + server.join().unwrap(); +} + #[test] fn cancellation_after_operation_is_pending_drops_it_before_return() { struct Dropped<'a>(&'a AtomicBool); diff --git a/rust_backend/crates/providers/src/lyrics/http.rs b/rust_backend/crates/providers/src/lyrics/http.rs index 0a3de5e2..246d9736 100644 --- a/rust_backend/crates/providers/src/lyrics/http.rs +++ b/rust_backend/crates/providers/src/lyrics/http.rs @@ -159,23 +159,10 @@ fn transport_error(error: String, check: &Check<'_>) -> LyricsError { } fn read(stream: &mut HttpStream, limit: usize, check: &Check<'_>) -> Result, LyricsError> { - let mut result = Vec::new(); - let mut buffer = [0; 16 << 10]; - loop { - let capacity = buffer.len().min(limit + 1 - result.len()); - let count = stream - .read(&mut buffer[..capacity], check) - .map_err(|error| transport_error(error, check))?; - if count == 0 { - break; - } - result.extend_from_slice(&buffer[..count]); - if result.len() > limit { - return Err(LyricsError::Other(format!( - "response exceeds {limit} bytes" - ))); - } - } + let result = stream + .read_limited(limit, &mut [0; 16 << 10], check) + .map_err(|error| transport_error(error, check))? + .ok_or_else(|| LyricsError::Other(format!("response exceeds {limit} bytes")))?; check().map_err(LyricsError::Cancelled)?; Ok(result) } diff --git a/rust_backend/crates/providers/src/resolver/http.rs b/rust_backend/crates/providers/src/resolver/http.rs index 964090ee..fa397555 100644 --- a/rust_backend/crates/providers/src/resolver/http.rs +++ b/rust_backend/crates/providers/src/resolver/http.rs @@ -166,23 +166,10 @@ impl ResolverHttp { } else { 2 << 20 }; - let mut body = Vec::new(); - let mut buffer = [0; 16 << 10]; - loop { - let capacity = buffer.len().min(limit + 1 - body.len()); - let read = stream - .read(&mut buffer[..capacity], check) - .map_err(failure)?; - if read == 0 { - break; - } - body.extend_from_slice(&buffer[..read]); - if body.len() > limit { - return Err(ResolverError::Failed(format!( - "response exceeds {limit} bytes" - ))); - } - } + let body = stream + .read_limited(limit, &mut [0; 16 << 10], check) + .map_err(failure)? + .ok_or_else(|| ResolverError::Failed(format!("response exceeds {limit} bytes")))?; check_active(check)?; if body.is_empty() && language.is_none() { return Err(ResolverError::Failed("response body is empty".into()));