From aaa4b2b7c2f461335078367bd61587708e2edaf8 Mon Sep 17 00:00:00 2001 From: ffccites <99155080+PDGGK@users.noreply.github.com> Date: Tue, 11 Aug 2026 04:20:07 +1000 Subject: [PATCH 1/2] fix(services/hf): correct the refs/convert revision split parse_revision sliced rev_and_path[..14 + slash] to recover the revision, but "refs/convert/" is 13 bytes, so the separator that terminates the revision segment was pulled into the revision itself. A URI like datasets/squad@refs/convert/parquet/default/train/0000.parquet yielded the revision "refs/convert/parquet/" rather than "refs/convert/parquet", and that trailing slash is carried into the operator config and percent-encoded into every request path as refs%2Fconvert%2Fparquet%2F. Rebuild the revision with format! the way the refs/pr/ arm immediately below already does, so there is no offset to get wrong. --- core/services/hf/src/core.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/core/services/hf/src/core.rs b/core/services/hf/src/core.rs index dff4d84bdc81..9ffb4e44e1e4 100644 --- a/core/services/hf/src/core.rs +++ b/core/services/hf/src/core.rs @@ -1058,10 +1058,8 @@ mod uri { // Match special refs: refs/(convert|pr)/ if let Some(rest) = rev_and_path.strip_prefix("refs/convert/") { return if let Some(slash) = rest.find('/') { - ( - rev_and_path[..14 + slash].to_string(), - rest[slash + 1..].to_string(), - ) + let revision = format!("refs/convert/{}", &rest[..slash]); + (revision, rest[slash + 1..].to_string()) } else { (rev_and_path.to_string(), String::new()) }; @@ -1267,6 +1265,15 @@ mod uri { assert_eq!(p.path, ""); } + #[test] + fn resolve_refs_convert_revision_with_path() { + let p = resolve("datasets/squad@refs/convert/parquet/default/train/0000.parquet"); + assert_eq!(p.repo.repo_type, HfRepoType::Dataset); + assert_eq!(p.repo.repo_id, "squad"); + assert_eq!(p.repo.revision.as_deref(), Some("refs/convert/parquet")); + assert_eq!(p.path, "default/train/0000.parquet"); + } + #[test] fn resolve_refs_pr_revision() { let p = resolve("username/my_model@refs/pr/10"); From 35f23d305b99a95c52aede57593bef86b9dde2e2 Mon Sep 17 00:00:00 2001 From: ffccites <99155080+PDGGK@users.noreply.github.com> Date: Tue, 11 Aug 2026 11:13:49 +1000 Subject: [PATCH 2/2] refactor(services/hf): use split_once for the special-ref split Applies the reviewer's suggestion. Both the refs/convert/ and refs/pr/ arms now use split_once instead of find + manual slicing, which removes the index arithmetic entirely and keeps the two adjacent arms symmetric. No behaviour change: split_once yields exactly the same two halves the find + slice pair did. --- core/services/hf/src/core.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/core/services/hf/src/core.rs b/core/services/hf/src/core.rs index 9ffb4e44e1e4..26ab4cdf10da 100644 --- a/core/services/hf/src/core.rs +++ b/core/services/hf/src/core.rs @@ -1057,19 +1057,15 @@ mod uri { // Match special refs: refs/(convert|pr)/ if let Some(rest) = rev_and_path.strip_prefix("refs/convert/") { - return if let Some(slash) = rest.find('/') { - let revision = format!("refs/convert/{}", &rest[..slash]); - (revision, rest[slash + 1..].to_string()) - } else { - (rev_and_path.to_string(), String::new()) + return match rest.split_once('/') { + Some((segment, path)) => (format!("refs/convert/{segment}"), path.to_string()), + None => (rev_and_path.to_string(), String::new()), }; } if let Some(rest) = rev_and_path.strip_prefix("refs/pr/") { - return if let Some(slash) = rest.find('/') { - let revision = format!("refs/pr/{}", &rest[..slash]); - (revision, rest[slash + 1..].to_string()) - } else { - (rev_and_path.to_string(), String::new()) + return match rest.split_once('/') { + Some((segment, path)) => (format!("refs/pr/{segment}"), path.to_string()), + None => (rev_and_path.to_string(), String::new()), }; }