From 06d2c5c884eee0b5b663730f4d379a7ceae3a8e4 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sat, 1 Aug 2026 05:26:16 +0900 Subject: feat(php-shim): implement Phar/PharData and the zlib/bzip2 functions Adopt the tar, flate2, and bzip2 crates to fill in the phar.rs and compress.rs todos: PharData tar/zip reading, building, and whole-archive compression, plus a native .phar reader that follows the php.net file-format manual and verifies hash-based signatures. Callers now propagate the constructor/extract errors PHP throws, and fwrite accepts byte strings so gzread no longer needs lossy UTF-8. The native .phar writing API stays todo!() (no call sites; Composer's Compiler is not ported) and OPENSSL phar signatures are accepted unverified (TODO(phase-c)). This unblocks Tar::getComposerJson and the tar/phar/gzip downloaders; tar_test (7), artifact_repository_test (2), and phar_archiver_test zip (1) are un-ignored. The archive command itself still panics because ArchiveManager::archive always generates glob excludes whose look-ahead regexes the regex crate cannot compile; converting those patterns to regex-compatible ones is a separate, still-undecided work item. Co-Authored-By: Claude Fable 5 --- crates/shirabe/src/downloader/gzip_downloader.rs | 5 ++-- crates/shirabe/src/downloader/phar_downloader.rs | 4 +-- crates/shirabe/src/downloader/tar_downloader.rs | 4 +-- crates/shirabe/src/io/buffer_io.rs | 2 +- .../package/archiver/archivable_files_filter.rs | 5 ++-- .../shirabe/src/package/archiver/phar_archiver.rs | 8 +++--- crates/shirabe/src/util/error_handler.rs | 2 +- crates/shirabe/src/util/perforce.rs | 30 ++++++++++------------ crates/shirabe/src/util/remote_filesystem.rs | 6 +++-- crates/shirabe/src/util/tar.rs | 17 +++++++++--- .../tests/package/archiver/archive_manager_test.rs | 8 ++---- .../tests/package/archiver/phar_archiver_test.rs | 6 +++-- .../tests/repository/artifact_repository_test.rs | 4 +-- crates/shirabe/tests/util/tar_test.rs | 7 ----- 14 files changed, 54 insertions(+), 54 deletions(-) (limited to 'crates/shirabe') diff --git a/crates/shirabe/src/downloader/gzip_downloader.rs b/crates/shirabe/src/downloader/gzip_downloader.rs index 4ea8f3b9..6b5f3872 100644 --- a/crates/shirabe/src/downloader/gzip_downloader.rs +++ b/crates/shirabe/src/downloader/gzip_downloader.rs @@ -50,14 +50,15 @@ impl GzipDownloader { } fn extract_using_ext(&self, file: &str, target_filepath: &str) { - let archive_file = gzopen(file, "rb"); + // PHP does not check the gzopen result either; gzread(false, ...) is a fatal TypeError. + let archive_file = gzopen(file, "rb").unwrap(); let target_file = fopen(target_filepath, "wb").unwrap(); loop { let string = gzread(archive_file.clone(), 4096); if string.is_empty() { break; } - fwrite(&target_file, &string, Some(Platform::strlen(&string))); + fwrite(&target_file, &string, Some(string.len() as i64)); } gzclose(archive_file); fclose(&target_file); diff --git a/crates/shirabe/src/downloader/phar_downloader.rs b/crates/shirabe/src/downloader/phar_downloader.rs index cbbb8252..b33fa74a 100644 --- a/crates/shirabe/src/downloader/phar_downloader.rs +++ b/crates/shirabe/src/downloader/phar_downloader.rs @@ -62,8 +62,8 @@ impl ArchiveDownloader for PharDownloader { path: &str, ) -> anyhow::Result> { // Can throw an UnexpectedValueException - let archive = Phar::new(file.to_string()); - archive.extract_to(path, None, true); + let archive = Phar::new(file.to_string())?; + archive.extract_to(path, None, true)?; // TODO: handle openssl signed phars // https://github.com/composer/composer/pull/33#issuecomment-2250768 // https://github.com/koto/phar-util diff --git a/crates/shirabe/src/downloader/tar_downloader.rs b/crates/shirabe/src/downloader/tar_downloader.rs index dfb825e4..ae600971 100644 --- a/crates/shirabe/src/downloader/tar_downloader.rs +++ b/crates/shirabe/src/downloader/tar_downloader.rs @@ -61,8 +61,8 @@ impl ArchiveDownloader for TarDownloader { file: &str, path: &str, ) -> anyhow::Result> { - let archive = PharData::new(file.to_string()); - archive.extract_to(path, None, true); + let archive = PharData::new(file.to_string())?; + archive.extract_to(path, None, true)?; Ok(None) } diff --git a/crates/shirabe/src/io/buffer_io.rs b/crates/shirabe/src/io/buffer_io.rs index f00ebe25..491cd316 100644 --- a/crates/shirabe/src/io/buffer_io.rs +++ b/crates/shirabe/src/io/buffer_io.rs @@ -142,7 +142,7 @@ impl BufferIO { }; for input in inputs { - fwrite(&stream, &format!("{}{}", input, PHP_EOL), None); + fwrite(&stream, format!("{}{}", input, PHP_EOL), None); } rewind(&stream); diff --git a/crates/shirabe/src/package/archiver/archivable_files_filter.rs b/crates/shirabe/src/package/archiver/archivable_files_filter.rs index ee39ba37..02a622f7 100644 --- a/crates/shirabe/src/package/archiver/archivable_files_filter.rs +++ b/crates/shirabe/src/package/archiver/archivable_files_filter.rs @@ -24,11 +24,12 @@ impl ArchivableFilesFilter { true } - pub fn add_empty_dir(&self, phar: &PharData, sources: &str) { + pub fn add_empty_dir(&self, phar: &PharData, sources: &str) -> anyhow::Result<()> { for filepath in &self.dirs { let localname = filepath.replace(&format!("{}/", sources), ""); - phar.add_empty_dir(&localname); + phar.add_empty_dir(&localname)?; } + Ok(()) } } diff --git a/crates/shirabe/src/package/archiver/phar_archiver.rs b/crates/shirabe/src/package/archiver/phar_archiver.rs index 1bac060b..55e4cb95 100644 --- a/crates/shirabe/src/package/archiver/phar_archiver.rs +++ b/crates/shirabe/src/package/archiver/phar_archiver.rs @@ -73,11 +73,11 @@ impl ArchiverInterface for PharArchiver { FilesystemIterator::KEY_AS_PATHNAME | FilesystemIterator::CURRENT_AS_FILEINFO, "", *formats.get(format.as_str()).unwrap_or(&Phar::TAR), - ); + )?; let files = ArchivableFilesFinder::new(&sources, excludes, ignore_filters)?; let mut files_only = ArchivableFilesFilter::new(Box::new(files)); - phar.build_from_iterator(&mut files_only, &sources); - files_only.add_empty_dir(&phar, &sources); + phar.build_from_iterator(&mut files_only, &sources)?; + files_only.add_empty_dir(&phar, &sources)?; if !file_exists(&target) { let target = format!("{}.{}", filename, format); @@ -137,7 +137,7 @@ impl ArchiverInterface for PharArchiver { unlink(&target); - phar.compress(compress_algo); + phar.compress(compress_algo)?; let target = format!("{}.{}", filename, format); return Ok(target); diff --git a/crates/shirabe/src/util/error_handler.rs b/crates/shirabe/src/util/error_handler.rs index 7a9cf9c2..2e574fe0 100644 --- a/crates/shirabe/src/util/error_handler.rs +++ b/crates/shirabe/src/util/error_handler.rs @@ -125,7 +125,7 @@ impl ErrorHandler { } if output_even_without_io { - fwrite(&STDERR, &format!("Warning: {}{}", message, PHP_EOL), None); + fwrite(&STDERR, format!("Warning: {}{}", message, PHP_EOL), None); } } } diff --git a/crates/shirabe/src/util/perforce.rs b/crates/shirabe/src/util/perforce.rs index fa448b35..9cd804a4 100644 --- a/crates/shirabe/src/util/perforce.rs +++ b/crates/shirabe/src/util/perforce.rs @@ -417,12 +417,12 @@ impl Perforce { pub fn write_client_spec_to_file(&mut self, spec: &PhpResource) { fwrite( spec, - &format!("Client: {}{}{}", self.get_client(), PHP_EOL, PHP_EOL), + format!("Client: {}{}{}", self.get_client(), PHP_EOL, PHP_EOL), None, ); fwrite( spec, - &format!( + format!( "Update: {}{}{}", date("Y/m/d H:i:s", None), PHP_EOL, @@ -432,12 +432,12 @@ impl Perforce { ); fwrite( spec, - &format!("Access: {}{}", date("Y/m/d H:i:s", None), PHP_EOL), + format!("Access: {}{}", date("Y/m/d H:i:s", None), PHP_EOL), None, ); fwrite( spec, - &format!( + format!( "Owner: {}{}{}", self.get_user().unwrap_or_default(), PHP_EOL, @@ -445,10 +445,10 @@ impl Perforce { ), None, ); - fwrite(spec, &format!("Description:{}", PHP_EOL), None); + fwrite(spec, format!("Description:{}", PHP_EOL), None); fwrite( spec, - &format!( + format!( " Created by {} from composer.{}{}", self.get_user().unwrap_or_default(), PHP_EOL, @@ -458,12 +458,12 @@ impl Perforce { ); fwrite( spec, - &format!("Root: {}{}{}", self.get_path(), PHP_EOL, PHP_EOL), + format!("Root: {}{}{}", self.get_path(), PHP_EOL, PHP_EOL), None, ); fwrite( spec, - &format!( + format!( "Options: noallwrite noclobber nocompress unlocked modtime rmdir{}{}", PHP_EOL, PHP_EOL ), @@ -471,20 +471,16 @@ impl Perforce { ); fwrite( spec, - &format!("SubmitOptions: revertunchanged{}{}", PHP_EOL, PHP_EOL), - None, - ); - fwrite( - spec, - &format!("LineEnd: local{}{}", PHP_EOL, PHP_EOL), + format!("SubmitOptions: revertunchanged{}{}", PHP_EOL, PHP_EOL), None, ); + fwrite(spec, format!("LineEnd: local{}{}", PHP_EOL, PHP_EOL), None); if self.is_stream() { - fwrite(spec, &format!("Stream:{}", PHP_EOL), None); + fwrite(spec, format!("Stream:{}", PHP_EOL), None); let stream_clone = self.p4_stream.clone().unwrap_or_default(); fwrite( spec, - &format!( + format!( " {}{}", self.get_stream_without_label(&stream_clone), PHP_EOL @@ -496,7 +492,7 @@ impl Perforce { let client = self.get_client(); fwrite( spec, - &format!("View: {}/... //{}/... {}", stream, client, PHP_EOL), + format!("View: {}/... //{}/... {}", stream, client, PHP_EOL), None, ); } diff --git a/crates/shirabe/src/util/remote_filesystem.rs b/crates/shirabe/src/util/remote_filesystem.rs index a244c9f9..2dafc0d3 100644 --- a/crates/shirabe/src/util/remote_filesystem.rs +++ b/crates/shirabe/src/util/remote_filesystem.rs @@ -1036,10 +1036,12 @@ impl RemoteFilesystem { .unwrap_or(false); if decode { - let decoded = zlib_decode(result.as_deref().unwrap_or("")); + let decoded = zlib_decode(result.as_deref().unwrap_or("").as_bytes()); result = match decoded { - Some(d) => Some(d), + // TODO(phase-e): byte-string semantics — the response body travels through + // RemoteFilesystem as a String; from_utf8_lossy can corrupt binary payloads + Some(d) => Some(String::from_utf8_lossy(&d).into_owned()), None => { return Err(anyhow::anyhow!(TransportException::new( "Failed to decode zlib stream".to_string(), diff --git a/crates/shirabe/src/util/tar.rs b/crates/shirabe/src/util/tar.rs index 55b6a960..792d1c4f 100644 --- a/crates/shirabe/src/util/tar.rs +++ b/crates/shirabe/src/util/tar.rs @@ -7,7 +7,7 @@ pub struct Tar; impl Tar { pub fn get_composer_json(path_to_archive: &str) -> anyhow::Result> { - let phar = PharData::new(path_to_archive.to_string()); + let phar = PharData::new(path_to_archive.to_string())?; if !phar.valid() { return Ok(None); @@ -16,9 +16,20 @@ impl Tar { Ok(Some(Self::extract_composer_json_from_folder(&phar)?)) } + /// The content bytes are decoded strictly: a composer.json that is not valid + /// UTF-8 could never survive the JSON parsing that follows in PHP either. + fn content_to_string(content: Vec) -> anyhow::Result { + String::from_utf8(content).map_err(|_| { + anyhow::anyhow!(RuntimeException { + message: "composer.json in the archive is not valid UTF-8".to_string(), + code: 0, + }) + }) + } + fn extract_composer_json_from_folder(phar: &PharData) -> anyhow::Result { if let Some(file) = phar.get("composer.json") { - return Ok(file.get_content()); + return Self::content_to_string(file.get_content()); } let mut top_level_paths: IndexMap = IndexMap::new(); @@ -49,7 +60,7 @@ impl Tar { if !top_level_paths.is_empty() && let Some(file) = phar.get(&composer_json_path) { - return Ok(file.get_content()); + return Self::content_to_string(file.get_content()); } Err(anyhow::anyhow!(RuntimeException { diff --git a/crates/shirabe/tests/package/archiver/archive_manager_test.rs b/crates/shirabe/tests/package/archiver/archive_manager_test.rs index d85fac10..3db1e734 100644 --- a/crates/shirabe/tests/package/archiver/archive_manager_test.rs +++ b/crates/shirabe/tests/package/archiver/archive_manager_test.rs @@ -166,12 +166,8 @@ fn test_unknown_format() { } // ref: ArchiveManagerTest::testArchiveTar / testArchiveCustomFileName. -// -// These drive ArchiveManager::archive end-to-end for the 'tar' format, which dispatches to -// PharArchiver::archive. That builds the archive via PharData, whose build_from_iterator is -// todo!() in the php-shim, so the archiving path cannot run yet. #[test] -#[ignore = "needs PharData tar archiving (new_with_format/build_from_iterator are todo!() in the php-shim) for ArchiveManager::archive('tar', ...)"] +#[ignore = "ArchiveManager::archive always passes buildExcludePatterns' glob excludes (e.g. 'name-*.zip'), which BaseExcludeFilter::generate_pattern turns into look-ahead regexes the regex crate cannot compile"] fn test_archive_tar() { if !git_is_executable() { return; @@ -208,7 +204,7 @@ fn test_archive_tar() { } #[test] -#[ignore = "needs PharData tar archiving (new_with_format/build_from_iterator are todo!() in the php-shim) for ArchiveManager::archive('tar', ...)"] +#[ignore = "ArchiveManager::archive always passes buildExcludePatterns' glob excludes (e.g. 'name-*.zip'), which BaseExcludeFilter::generate_pattern turns into look-ahead regexes the regex crate cannot compile"] fn test_archive_custom_file_name() { if !git_is_executable() { return; diff --git a/crates/shirabe/tests/package/archiver/phar_archiver_test.rs b/crates/shirabe/tests/package/archiver/phar_archiver_test.rs index fe6185d8..39c5087a 100644 --- a/crates/shirabe/tests/package/archiver/phar_archiver_test.rs +++ b/crates/shirabe/tests/package/archiver/phar_archiver_test.rs @@ -1,5 +1,6 @@ //! ref: composer/tests/Composer/Test/Package/Archiver/PharArchiverTest.php +use serial_test::serial; use shirabe::package::archiver::{ArchiverInterface, PharArchiver}; use shirabe::package::handle::CompletePackageHandle; use shirabe::util::{Filesystem, Platform}; @@ -63,8 +64,9 @@ impl ArchiverTestCase { } } -#[ignore = "PharArchiver::archive builds the archive via PharData, which is todo!() in the php-shim"] +#[ignore = "the excludes passed here make BaseExcludeFilter::generate_pattern emit look-ahead regexes ((?=$|/) and Glob's (?=[^\\.])) that the regex crate cannot compile"] #[test] +#[serial] fn test_tar_archive() { let mut test_case = ArchiverTestCase::set_up(); @@ -98,8 +100,8 @@ fn test_tar_archive() { .unwrap(); } -#[ignore = "PharArchiver::archive builds the archive via PharData, which is todo!() in the php-shim"] #[test] +#[serial] fn test_zip_archive() { let mut test_case = ArchiverTestCase::set_up(); diff --git a/crates/shirabe/tests/repository/artifact_repository_test.rs b/crates/shirabe/tests/repository/artifact_repository_test.rs index 2bfe85de..b13f23e9 100644 --- a/crates/shirabe/tests/repository/artifact_repository_test.rs +++ b/crates/shirabe/tests/repository/artifact_repository_test.rs @@ -33,7 +33,6 @@ fn create_repo(url: &str) -> ArtifactRepository { } #[test] -#[ignore = "the artifacts fixtures dir contains a .tar file (jsonInRootTarFile); scanning it routes through Tar::get_composer_json -> PharData::new which is todo!()"] fn test_extracts_configs_from_zip_archives() { if set_up() { return; @@ -83,7 +82,6 @@ fn test_extracts_configs_from_zip_archives() { } #[test] -#[ignore = "the artifacts fixtures dir contains a .tar file (jsonInRootTarFile); scanning it routes through Tar::get_composer_json -> PharData::new which is todo!()"] fn test_absolute_repo_url_creates_absolute_url_packages() { if set_up() { return; @@ -104,7 +102,7 @@ fn test_absolute_repo_url_creates_absolute_url_packages() { } #[test] -#[ignore = "the relative url is resolved from the process cwd (the crate manifest dir under cargo, not the composer test root), so the artifacts dir is not found; additionally the dir contains a .tar file routing through PharData::new which is todo!()"] +#[ignore = "the relative url is resolved from the process cwd (the crate manifest dir under cargo, not the composer test root), so the artifacts dir is not found"] fn test_relative_repo_url_creates_relative_url_packages() { if set_up() { return; diff --git a/crates/shirabe/tests/util/tar_test.rs b/crates/shirabe/tests/util/tar_test.rs index 7a172a7a..50951202 100644 --- a/crates/shirabe/tests/util/tar_test.rs +++ b/crates/shirabe/tests/util/tar_test.rs @@ -12,7 +12,6 @@ fn fixture(name: &str) -> String { } #[test] -#[ignore = "PharData::new() (crates/shirabe-php-shim/src/phar.rs:91) is still todo!(), which Tar::get_composer_json depends on for every fixture"] fn test_returns_nullif_the_tar_is_not_found() { let result = Tar::get_composer_json(&fixture("invalid.zip")).unwrap(); @@ -20,26 +19,22 @@ fn test_returns_nullif_the_tar_is_not_found() { } #[test] -#[ignore = "PharData::new() (crates/shirabe-php-shim/src/phar.rs:91) is still todo!(), which Tar::get_composer_json depends on for every fixture"] fn test_returns_null_if_the_tar_is_empty() { let result = Tar::get_composer_json(&fixture("empty.tar.gz")).unwrap(); assert_eq!(None, result); } #[test] -#[ignore = "PharData::new() (crates/shirabe-php-shim/src/phar.rs:91) is still todo!(), which Tar::get_composer_json depends on for every fixture"] fn test_throws_exception_if_the_tar_has_no_composer_json() { assert!(Tar::get_composer_json(&fixture("nojson.tar.gz")).is_err()); } #[test] -#[ignore = "PharData::new() (crates/shirabe-php-shim/src/phar.rs:91) is still todo!(), which Tar::get_composer_json depends on for every fixture"] fn test_throws_exception_if_the_composer_json_is_in_a_sub_subfolder() { assert!(Tar::get_composer_json(&fixture("subfolders.tar.gz")).is_err()); } #[test] -#[ignore = "PharData::new() (crates/shirabe-php-shim/src/phar.rs:91) is still todo!(), which Tar::get_composer_json depends on for every fixture"] fn test_returns_composer_json_in_tar_root() { let result = Tar::get_composer_json(&fixture("root.tar.gz")).unwrap(); assert_eq!( @@ -49,7 +44,6 @@ fn test_returns_composer_json_in_tar_root() { } #[test] -#[ignore = "PharData::new() (crates/shirabe-php-shim/src/phar.rs:91) is still todo!(), which Tar::get_composer_json depends on for every fixture"] fn test_returns_composer_json_in_first_folder() { let result = Tar::get_composer_json(&fixture("folder.tar.gz")).unwrap(); assert_eq!( @@ -59,7 +53,6 @@ fn test_returns_composer_json_in_first_folder() { } #[test] -#[ignore = "PharData::new() (crates/shirabe-php-shim/src/phar.rs:91) is still todo!(), which Tar::get_composer_json depends on for every fixture"] fn test_multiple_top_level_dirs_is_invalid() { assert!(Tar::get_composer_json(&fixture("multiple.tar.gz")).is_err()); } -- cgit v1.3.1