diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-08-02 20:11:51 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-08-02 20:11:51 +0900 |
| commit | c2a2bd3a2573f585c902c39bd28b8c3cad10c317 (patch) | |
| tree | aa8138a0570966f26e3708b500041258074dcd24 /crates/shirabe/src/repository/filesystem_repository.rs | |
| parent | 7db937af313d857d0f66bebaf8ac72d518559bac (diff) | |
| download | php-shirabe-c2a2bd3a2573f585c902c39bd28b8c3cad10c317.tar.gz php-shirabe-c2a2bd3a2573f585c902c39bd28b8c3cad10c317.tar.zst php-shirabe-c2a2bd3a2573f585c902c39bd28b8c3cad10c317.zip | |
fix(repository): resolve remaining late-binding hazards from the audit
Three fixes for the ComposerRepository/FilesystemRepository/PlatformRepository
hazards where inner-composition delegation skipped PHP's late-bound virtual
dispatch:
- ComposerRepository::has_package now builds its packageMap through the
late-bound getPackages() equivalent, so lazy-providers repos surface the
LogicException and available-packages repos load their package list, as in
PHP, instead of silently answering false from the raw array.
- RepositoryInterface::get_repo_name returns anyhow::Result<String>: PHP's
getRepoName() counts through the late-bound initialize(), which is fallible
in file-reading subclasses. FilesystemRepository and PackageRepository now
run that initialization instead of freezing the inner array repository to an
empty state (which also made a later write() truncate installed.json).
Supporting changes keep the initialization chain callable from &self:
JsonFile::read takes &self (indent moved into a RefCell), FilesystemRepository
dev_mode became a Cell, and WritableArrayRepository dev_package_names a
RefCell.
- PlatformRepository::new routes constructor packages through its own
add_package so the override handling and full platform initialization run
as they do via PHP's parent constructor; the inner find_package/add_package
delegations inside add_package (and ComposerRepository::add_package) gained
the same is_initialized guard, since the constructor path would otherwise
freeze the repository.
Same defect class as 7db937af, 97b5211a and 3e367f78.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Diffstat (limited to 'crates/shirabe/src/repository/filesystem_repository.rs')
| -rw-r--r-- | crates/shirabe/src/repository/filesystem_repository.rs | 27 |
1 files changed, 17 insertions, 10 deletions
diff --git a/crates/shirabe/src/repository/filesystem_repository.rs b/crates/shirabe/src/repository/filesystem_repository.rs index 88317be2..151cf2d3 100644 --- a/crates/shirabe/src/repository/filesystem_repository.rs +++ b/crates/shirabe/src/repository/filesystem_repository.rs @@ -38,7 +38,9 @@ pub struct FilesystemRepository { /// @var Filesystem filesystem: std::rc::Rc<std::cell::RefCell<Filesystem>>, /// @var bool|null - dev_mode: Option<bool>, + // Cell so that initialize() can stay `&self` (late-bound initialization out of shared + // contexts such as getRepoName). + dev_mode: std::cell::Cell<Option<bool>>, } impl FilesystemRepository { @@ -67,24 +69,27 @@ impl FilesystemRepository { dump_versions, root_package, filesystem, - dev_mode: None, + dev_mode: std::cell::Cell::new(None), }) } /// @return bool|null true if dev requirements were installed, false if --no-dev was used, null if yet unknown pub fn get_dev_mode(&self) -> Option<bool> { - self.dev_mode + self.dev_mode.get() } pub fn set_self_handle(&self, weak: crate::repository::RepositoryInterfaceWeakHandle) { self.inner.set_self_handle(weak); } - pub fn get_repo_name(&self) -> String { + pub fn get_repo_name(&self) -> anyhow::Result<String> { + // PHP: ArrayRepository::getRepoName() counts through the late-bound $this->initialize(), + // which resolves to FilesystemRepository::initialize (reading the file). + self.ensure_initialized()?; self.inner.get_repo_name() } - fn ensure_initialized(&mut self) -> anyhow::Result<()> { + fn ensure_initialized(&self) -> anyhow::Result<()> { if !self.inner.is_initialized() { self.initialize()?; } @@ -92,7 +97,7 @@ impl FilesystemRepository { } /// Initializes repository (reads file, or remote address). - pub(crate) fn initialize(&mut self) -> anyhow::Result<()> { + pub(crate) fn initialize(&self) -> anyhow::Result<()> { self.inner.initialize(); if !self.file.exists() { @@ -124,7 +129,7 @@ impl FilesystemRepository { self.inner.set_dev_package_names(dev_names); } if let Some(dev) = m.get("dev") { - self.dev_mode = dev.as_bool(); + self.dev_mode.set(dev.as_bool()); } } @@ -203,7 +208,7 @@ impl FilesystemRepository { self.inner.set_dev_package_names(dev_package_names); } - pub fn get_dev_package_names(&self) -> &Vec<String> { + pub fn get_dev_package_names(&self) -> Vec<String> { self.inner.get_dev_package_names() } @@ -281,6 +286,7 @@ impl FilesystemRepository { &PhpMixed::List( self.inner .dev_package_names + .borrow() .iter() .map(|s| PhpMixed::String(s.clone())) .collect(), @@ -436,6 +442,7 @@ impl FilesystemRepository { let dev_packages = array_flip(&PhpMixed::List( self.inner .dev_package_names + .borrow() .iter() .map(|s| PhpMixed::String(s.clone())) .collect(), @@ -793,8 +800,8 @@ impl RepositoryInterface for FilesystemRepository { self.inner.get_providers(package_name) } - fn get_repo_name(&self) -> String { - self.inner.get_repo_name() + fn get_repo_name(&self) -> anyhow::Result<String> { + FilesystemRepository::get_repo_name(self) } fn as_any(&self) -> &dyn std::any::Any { |
