refactor(rust): consolidate small duplicate helpers

Keep one copy of helpers that were implemented several times, each proven
equivalent to the copies it replaces by a test that runs the removed
implementation (kept inline in the test module) against the survivor:

- Go-style lexical path clean: new `core::fs::clean`, used by
  `isrc::native_files` and re-exported as `extensions::files::clean`.
- Dart `String.trim` whitespace: `path_keys::dart_whitespace` and
  `dart_trim` now back the lyrics parser's explicit character list and the
  playlist importer's private `trim`; the test compares every Unicode
  scalar value.
- `normalized()` (Dart trim then simple lowercase): the native backup
  importer uses the identical `native_library::normalized`.
- Lowercase hex encoding: `spotiflac_core::lower_hex` replaces the per-byte
  `format!` collectors in data_jobs and network_tags, the signed-session
  `hex` and the `hex` arm of `binary::encode`; checked for all 256 bytes.
- ID3 syncsafe decode: the tag writer's `size` reuses the reader's
  `tags::id3::syncsafe`; checked across the 0x7F/0x80 boundaries per byte.
- Bounded body reads: `HttpStream::read_limited` replaces the read-to-limit
  loops of the lyrics HTTP client, the resolver HTTP client and the
  extension registry reader. Callers keep their buffer sizes (16 KiB,
  16 KiB, 64 KiB), their own error mapping and their messages; a loopback
  test compares it with the previous loop at and around the limit.

The mobile ID3 `synchsafe` is not merged: it validates the length and the
high bits and returns `None`, unlike the core decoder.
This commit is contained in:
zarzet committed 2026-10-19 00:36:29 +07:00
1 parent 088bd875dd
commit f7a15cbd43
21 files changed
+332 -163

No files matched your search

+85
View File
@@ -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:?}");
}
}
}
@@ -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
}
+2 -1
View File
@@ -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};
+1 -1
View File
@@ -501,7 +501,7 @@ fn v1(reader: &mut (impl Read + Seek)) -> Result<AudioMetadata, String> {
})
}
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
+36 -4
View File
@@ -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:?}");
}
}
}
}
}
}
+25
View File
@@ -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<u8> = (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);
}
}
+1 -8
View File
@@ -139,14 +139,7 @@ pub(crate) fn decode_option_bytes(
pub(crate) fn encode(bytes: &[u8], encoding: &str) -> Result<String, String> {
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}")),
}
+1 -22
View File
@@ -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;
@@ -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))
}
@@ -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<String, String> {
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::<Sha256>::new_from_slice(key).expect("HMAC accepts any key length");
mac.update(message);
+1 -5
View File
@@ -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)]
@@ -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<i64> {
.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((
@@ -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))
}
@@ -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:?}"
);
}
}
}
@@ -18,28 +18,9 @@ static VOICE_PREFIX: LazyLock<Regex> = 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)
}
@@ -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,
+3 -10
View File
@@ -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())?;
@@ -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<Option<Vec<u8>>, 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.
@@ -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);
@@ -159,23 +159,10 @@ fn transport_error(error: String, check: &Check<'_>) -> LyricsError {
}
fn read(stream: &mut HttpStream, limit: usize, check: &Check<'_>) -> Result<Vec<u8>, 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)
}
@@ -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()));