From 76c6d132fbc8341bdf14537bba7f00b073eddbc1 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Fri, 24 Jul 2026 20:23:12 +0900 Subject: fix(package): rewrite dist-reference SHA regex without look-around Package::set_source_dist_references and LockTransaction's dist-url mirroring both used {(?<=/|sha=)[a-f0-9]{40}(?=/|$)}i, but the regex crate has no look-around support at all and panics compiling it. Per docs/dev/regex-porting.md, rewrote the boundary assertions into capturing groups and switched to Preg::replace_callback, which re-emits the captured delimiters around the replaced reference. --- .../src/dependency_resolver/lock_transaction.rs | 27 +++++++++++++++++++--- crates/shirabe/src/package/package.rs | 24 ++++++++++++++++--- crates/shirabe/tests/installer_test.rs | 4 ++-- 3 files changed, 47 insertions(+), 8 deletions(-) (limited to 'crates') diff --git a/crates/shirabe/src/dependency_resolver/lock_transaction.rs b/crates/shirabe/src/dependency_resolver/lock_transaction.rs index 1beedd24..d78db851 100644 --- a/crates/shirabe/src/dependency_resolver/lock_transaction.rs +++ b/crates/shirabe/src/dependency_resolver/lock_transaction.rs @@ -166,9 +166,30 @@ impl LockTransaction { &package.get_dist_url().unwrap(), ) { - let new_dist_url = Preg::replace( - php_regex!(r"{(?<=/|sha=)[a-f0-9]{40}(?=/|$)}i"), - &present_package.get_dist_reference().unwrap(), + // Regex pattern compatibility: + // The `regex` crate has no look-around, so `(?<=/|sha=)[a-f0-9]{40}(?=/|$)` is + // rewritten to capture the boundary delimiters instead of asserting them, and the + // callback re-emits them around the replaced reference. Unlike the zero-width + // lookaround, the capturing version consumes its boundary delimiter, so two + // 40-hex SHAs sharing a single `/` between them would not both match; harmless + // here since a dist URL never carries more than one SHA reference. + let dist_reference = present_package.get_dist_reference().unwrap(); + let new_dist_url = Preg::replace_callback( + php_regex!(r"{(/|sha=)[a-f0-9]{40}(/|$)}i"), + |m: &indexmap::IndexMap< + shirabe_external_packages::composer::pcre::CaptureKey, + String, + >| + -> String { + let get = |i: usize| -> String { + m.get( + &shirabe_external_packages::composer::pcre::CaptureKey::ByIndex(i), + ) + .cloned() + .unwrap_or_default() + }; + format!("{}{}{}", get(1), dist_reference, get(2)) + }, &package.get_dist_url().unwrap(), ); present_package.set_dist_url(Some(new_dist_url)); diff --git a/crates/shirabe/src/package/package.rs b/crates/shirabe/src/package/package.rs index e0bee0e6..ea415b9c 100644 --- a/crates/shirabe/src/package/package.rs +++ b/crates/shirabe/src/package/package.rs @@ -424,9 +424,27 @@ impl Package { ) { self.set_dist_reference(Some(reference.clone())); - self.set_dist_url(Some(Preg::replace( - php_regex!("{(?<=/|sha=)[a-f0-9]{40}(?=/|$)}i"), - &reference, + // Regex pattern compatibility: + // The `regex` crate has no look-around, so `(?<=/|sha=)[a-f0-9]{40}(?=/|$)` is + // rewritten to capture the boundary delimiters instead of asserting them, and the + // callback re-emits them around the replaced reference. Unlike the zero-width + // lookaround, the capturing version consumes its boundary delimiter, so two 40-hex + // SHAs sharing a single `/` between them would not both match; harmless here since a + // dist URL never carries more than one SHA reference. + self.set_dist_url(Some(Preg::replace_callback( + php_regex!("{(/|sha=)[a-f0-9]{40}(/|$)}i"), + |m: &indexmap::IndexMap< + shirabe_external_packages::composer::pcre::CaptureKey, + String, + >| + -> String { + let get = |i: usize| -> String { + m.get(&shirabe_external_packages::composer::pcre::CaptureKey::ByIndex(i)) + .cloned() + .unwrap_or_default() + }; + format!("{}{}{}", get(1), reference, get(2)) + }, &self.get_dist_url().unwrap_or_default(), ))); } else if self.get_dist_reference().is_some() { diff --git a/crates/shirabe/tests/installer_test.rs b/crates/shirabe/tests/installer_test.rs index 13e4fc28..2a0c4872 100644 --- a/crates/shirabe/tests/installer_test.rs +++ b/crates/shirabe/tests/installer_test.rs @@ -1502,7 +1502,7 @@ pool_optimizer_test! { pool_optimizer_update_allow_list_with_dependencies => "update-allow-list-with-dependencies.test"; pool_optimizer_update_allow_list_with_dependency_conflict => "update-allow-list-with-dependency-conflict.test"; pool_optimizer_update_allow_list => "update-allow-list.test"; - pool_optimizer_update_changes_url => "update-changes-url.test", ignore = "TODO(phase-d): known-failing fixture under COMPOSER_POOL_OPTIMIZER=1"; + pool_optimizer_update_changes_url => "update-changes-url.test"; pool_optimizer_update_dev_ignores_providers => "update-dev-ignores-providers.test"; pool_optimizer_update_dev_packages_updates_repo_url => "update-dev-packages-updates-repo-url.test"; pool_optimizer_update_dev_to_new_ref_picks_up_changes => "update-dev-to-new-ref-picks-up-changes.test"; @@ -1692,7 +1692,7 @@ raw_pool_test! { raw_pool_update_allow_list_with_dependencies => "update-allow-list-with-dependencies.test"; raw_pool_update_allow_list_with_dependency_conflict => "update-allow-list-with-dependency-conflict.test"; raw_pool_update_allow_list => "update-allow-list.test"; - raw_pool_update_changes_url => "update-changes-url.test", ignore = "TODO(phase-d): known-failing fixture under COMPOSER_POOL_OPTIMIZER=0"; + raw_pool_update_changes_url => "update-changes-url.test"; raw_pool_update_dev_ignores_providers => "update-dev-ignores-providers.test"; raw_pool_update_dev_packages_updates_repo_url => "update-dev-packages-updates-repo-url.test"; raw_pool_update_dev_to_new_ref_picks_up_changes => "update-dev-to-new-ref-picks-up-changes.test"; -- cgit v1.3.1