From 5b23146a685a9a57c11f4435116e12520b519d81 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 12 Oct 2026 19:20:09 +0400 Subject: [PATCH 1/5] feat(software): read software definitions from Composer packages feat(software): address the entry of a package as `#` and report shadowed entries fix(binary): run a binary without a shell, passing every argument as is fix(installer): keep binary names and file renames inside the destination directory fix(installer): install a binary named like `a.b` as `a.b`, not `a.b.b` fix(downloader): save an asset under a file name whatever the release API calls it fix(cli): strip console tags and terminal control characters from registry text A framework or CMS can ship the binaries it needs without every project copying them into its dload.xml, and without a DLoad release for each tool. Only the root package and its direct dependencies are read, and their entries never override dload.xml or the official registry (#140). Package entries are third-party input that `dload show` acts on, so the core stopped trusting any config: a version-command used to reach the shell, a binary name could leave the target directory, and on Windows escapeshellarg() corrupted `"`, `%` and `!` in arguments. Package entries are held to a stricter shape on top: one version option, file names, valid regexes. Assisted-By: Claude Opus 5.5 --- README-es.md | 59 +- README-ru.md | 59 +- README-zh.md | 56 +- README.md | 59 +- dload.xsd | 13 +- resources/software.schema.json | 4 +- skills/dload-fetch-tool/SKILL.md | 6 +- .../references/package-registry.md | 46 ++ .../references/registry-entry.md | 4 +- src/Bootstrap.php | 15 + src/Command/Get.php | 7 +- src/Command/ListSoftware.php | 37 +- src/Command/Show.php | 87 +-- src/DLoad.php | 3 + src/Module/Binary/Internal/AbstractBinary.php | 7 +- src/Module/Binary/Internal/BinaryExecutor.php | 33 +- .../Binary/Internal/BinaryProviderImpl.php | 12 +- src/Module/Common/FileSystem/FS.php | 34 ++ .../Config/Schema/CustomSoftwareRegistry.php | 4 + src/Module/Downloader/Downloader.php | 4 +- src/Module/Installer/Installer.php | 2 +- .../Installer/Internal/RuleExtraction.php | 7 +- src/Module/Software/ComposerPackages.php | 386 ++++++++++++ .../Internal/Source/PackageSource.php | 37 ++ src/Module/Software/ShadowReport.php | 138 +++++ src/Module/Software/SoftwareCollection.php | 45 +- src/Module/Software/SoftwareCollector.php | 14 +- src/Service/Logger.php | 18 +- .../Command/PackageRegistryCommandsTest.php | 364 +++++++++++ .../Software/ComposerPackagesBindingTest.php | 144 +++++ .../Binary/Internal/AbstractBinaryTest.php | 60 ++ .../Binary/Internal/BinaryExecutorTest.php | 73 +++ .../Internal/BinaryProviderImplTest.php | 87 +++ .../Unit/Module/Common/FileSystem/FSTest.php | 84 +++ .../Unit/Module/Downloader/DownloaderTest.php | 22 + tests/Unit/Module/Installer/InstallerTest.php | 81 +++ .../Module/Software/ComposerPackagesTest.php | 567 ++++++++++++++++++ .../Unit/Module/Software/ShadowReportTest.php | 195 ++++++ .../Software/SoftwareCollectionTest.php | 78 +++ .../Module/Software/SoftwareCollectorTest.php | 79 +++ tests/Unit/Service/LoggerTest.php | 77 +++ 41 files changed, 3018 insertions(+), 89 deletions(-) create mode 100644 skills/dload-fetch-tool/references/package-registry.md create mode 100644 src/Module/Software/ComposerPackages.php create mode 100644 src/Module/Software/Internal/Source/PackageSource.php create mode 100644 src/Module/Software/ShadowReport.php create mode 100644 tests/Integration/Command/PackageRegistryCommandsTest.php create mode 100644 tests/Integration/Module/Software/ComposerPackagesBindingTest.php create mode 100644 tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php create mode 100644 tests/Unit/Module/Binary/Internal/BinaryProviderImplTest.php create mode 100644 tests/Unit/Module/Common/FileSystem/FSTest.php create mode 100644 tests/Unit/Module/Software/ComposerPackagesTest.php create mode 100644 tests/Unit/Module/Software/ShadowReportTest.php create mode 100644 tests/Unit/Service/LoggerTest.php diff --git a/README-es.md b/README-es.md index 5d446d57..ba16ceb9 100644 --- a/README-es.md +++ b/README-es.md @@ -61,6 +61,7 @@ Con DLoad puedes: - [Definir Software](#definir-software) - [Elementos de Software](#elementos-de-software) - [Servidores Autoalojados](#servidores-autoalojados) + - [Software desde Paquetes Composer](#software-desde-paquetes-composer) - [Casos de Uso](#casos-de-uso) - [Configurar Entorno de Desarrollo](#configurar-entorno-de-desarrollo) - [Configurar Nuevo Proyecto](#configurar-nuevo-proyecto) @@ -597,8 +598,11 @@ El binario de RoadRunner construido incluirá solo los plugins especificados en #### Elementos Binary -- **name**: Nombre del binario para referencia +- **name**: Nombre del binario para referencia; un nombre de archivo, no una ruta - **pattern**: Patrón regex para hacer match con el binario en los assets +- **version-command**: Argumentos con los que el binario imprime su versión, como `--version`; sin él la versión no se comprueba. + Se pasan al binario tal cual, sin shell: las comillas y las tuberías no se interpretan. + En las entradas de paquetes Composer es una sola opción como `--version`, o `version` - Maneja automáticamente el filtrado por SO/arquitectura #### Elementos File @@ -657,6 +661,59 @@ el token de un servidor solo va a ese esquema, host y puerto exactos, y se desca Se permite `http` sin cifrar, p. ej. para una API falsa local en tests (`uri="http://127.0.0.1:8080/owner/repo"`), pero el token solo se envía por él a `localhost`, `127.0.0.0/8` y `[::1]`. +### Software desde Paquetes Composer + +Un framework o un CMS puede incluir las definiciones de los binarios que necesita, para que los proyectos que lo usan +no tengan que copiarlas en su `dload.xml`. El paquete apunta a un archivo XML propio desde su `composer.json`: + +```json +{ + "extra": { + "dload": { + "registry": "resources/dload.xml" + } + } +} +``` + +El archivo tiene el formato de `dload.xml`, así que el IDE lo autocompleta y lo valida; solo se lee su sección ``. +Puede ser un archivo dedicado o el propio `dload.xml` del paquete. Mantenlo en el dist del paquete: no le apliques `export-ignore`. + +```xml + + + + + + + + +``` + +DLoad busca `composer.json` junto al `dload.xml` cargado (en el directorio de trabajo si no hay uno) y lee +los paquetes instalados de `vendor/composer/installed.json` (se respetan `COMPOSER_VENDOR_DIR` y `config.vendor-dir`, solo Composer 2). +Funciona igual con el paquete de Composer, el PHAR y el binario. + +- **Quién puede declarar**: el paquete raíz y sus dependencias directas (`require`, `require-dev`). + Una dependencia transitiva no puede añadir software. +- **Dónde puede estar el archivo**: solo dentro del paquete. Una ruta que sale de él, mediante `..`, + una ruta absoluta o un enlace simbólico, se rechaza. Los archivos remotos no están soportados. +- **Prioridad**: `dload.xml` > el registro oficial > los paquetes. Un paquete nunca reemplaza una entrada de los otros dos; + cuando el registro oficial oculta una entrada de un paquete que se está descargando, DLoad avisa de ello. +- **Elección explícita**: `#` selecciona la entrada de un paquete, aunque otra la oculte: + `dload get acme/framework#gh:^2.0` o ``. +- **Varios paquetes, un mismo id**: si los paquetes apuntan a los mismos repositorios, coinciden y no hay nada que avisar. + Si no, gana el paquete raíz, luego los paquetes en el orden de `require` y `require-dev`, + y al descargar ese id se muestran todos los `#` entre los que elegir. +- **Las declaraciones erróneas** se notifican como advertencias y se omiten; nunca detienen DLoad. Esto incluye un archivo inexistente, + un XML inválido, un atributo obligatorio vacío, una ruta donde se espera un nombre de archivo, una regex inválida + y un `version-command` que no sea una sola opción como `--version`, o `version`. +- **Desactivación**: `` descarta el software de los paquetes y mantiene el registro oficial; + `` descarta ambos. + +`dload software` muestra de dónde viene cada entrada: `official`, `dload.xml` o el nombre del paquete. Una entrada que oculta otras las lista en `Shadows:`, las de paquetes como `#`. + ## Casos de Uso ### Configurar Entorno de Desarrollo diff --git a/README-ru.md b/README-ru.md index e501b8a8..87e5f925 100644 --- a/README-ru.md +++ b/README-ru.md @@ -62,6 +62,7 @@ DLoad решает распространённую проблему в PHP-пр - [Определение ПО](#определение-по) - [Элементы ПО](#элементы-по) - [Собственные серверы](#собственные-серверы) + - [ПО из Composer-пакетов](#по-из-composer-пакетов) - [Сценарии использования](#сценарии-использования) - [Настройка среды разработки](#настройка-среды-разработки) - [Настройка нового проекта](#настройка-нового-проекта) @@ -599,8 +600,11 @@ DLoad автоматически управляет процессом сбор #### Элементы Binary -- **name**: Имя бинарника для ссылки +- **name**: Имя бинарника для ссылки; имя файла, а не путь - **pattern**: Regex-паттерн для соответствия бинарнику в ресурсах +- **version-command**: Аргументы, с которыми бинарник выводит свою версию, например `--version`; без него версия не проверяется. + Они передаются бинарнику как есть, без shell: кавычки и пайпы не интерпретируются. + В записях Composer-пакетов это одна опция вроде `--version` или слово `version` - Автоматически обрабатывает фильтрацию по ОС/архитектуре #### Элементы File @@ -659,6 +663,59 @@ DLOAD_TOKEN_GHE_EXAMPLE_COM=your_token_here ./vendor/bin/dload get Обычный `http` допускается, например, для локального фейкового API в тестах (`uri="http://127.0.0.1:8080/owner/repo"`), но токен передаётся по нему только на `localhost`, `127.0.0.0/8` и `[::1]`. +### ПО из Composer-пакетов + +Фреймворк или CMS может поставлять описания нужных ему бинарников, чтобы использующим его проектам не приходилось +копировать их в свой `dload.xml`. Пакет указывает на XML-файл внутри себя в своём `composer.json`: + +```json +{ + "extra": { + "dload": { + "registry": "resources/dload.xml" + } + } +} +``` + +Файл имеет формат `dload.xml`, поэтому IDE его дополняет и проверяет; читается только секция ``. +Это может быть отдельный файл или собственный `dload.xml` пакета. Файл должен попасть в дистрибутив пакета: не исключайте его через `export-ignore`. + +```xml + + + + + + + + +``` + +DLoad ищет `composer.json` рядом с загруженным `dload.xml` (а если его нет — в рабочем каталоге) и читает +установленные пакеты из `vendor/composer/installed.json` (`COMPOSER_VENDOR_DIR` и `config.vendor-dir` учитываются, поддерживается только Composer 2). +Это работает одинаково для Composer-пакета, PHAR и бинарника. + +- **Кто может объявлять ПО**: корневой пакет и его прямые зависимости (`require`, `require-dev`). + Транзитивная зависимость добавить ПО не может. +- **Где может лежать файл**: только внутри пакета. Путь, ведущий за его пределы через `..`, + абсолютный путь или символическую ссылку, отклоняется. Удалённые файлы не поддерживаются. +- **Приоритет**: `dload.xml` > официальный реестр > пакеты. Пакет никогда не заменяет запись из двух других источников; + если официальный реестр перекрывает загружаемую запись пакета, DLoad предупреждает об этом. +- **Явный выбор**: `#` выбирает запись пакета, что бы её ни перекрывало: + `dload get acme/framework#gh:^2.0` или ``. +- **Несколько пакетов, один id**: если пакеты указывают на одни и те же репозитории, они согласованы и сообщать не о чем. + Иначе побеждает корневой пакет, затем пакеты в порядке `require` и `require-dev`, + а при загрузке по этому id выводятся все варианты `#`, из которых можно выбрать. +- **Ошибочные объявления** выводятся как предупреждения и пропускаются; они никогда не останавливают DLoad. Это отсутствующий файл, + невалидный XML, пустой обязательный атрибут, путь там, где ожидается имя файла, невалидное регулярное выражение + и `version-command`, который не является одной опцией вроде `--version` или словом `version`. +- **Отключение**: `` отбрасывает ПО из пакетов и сохраняет официальный реестр; + `` отбрасывает и то и другое. + +`dload software` показывает, откуда взята каждая запись: `official`, `dload.xml` или имя пакета. Если запись перекрывает другие, они перечислены в строке `Shadows:`, записи пакетов — в виде `#`. + ## Сценарии использования ### Настройка среды разработки diff --git a/README-zh.md b/README-zh.md index 5f10681c..2904b85a 100644 --- a/README-zh.md +++ b/README-zh.md @@ -61,6 +61,7 @@ DLoad 解决了 PHP 项目中的一个实际问题：如何在分发 PHP 代码 - [定义软件](#定义软件) - [软件要素](#软件要素) - [自托管服务器](#自托管服务器) + - [来自 Composer 包的软件](#来自-composer-包的软件) - [使用场景](#使用场景) - [开发环境配置](#开发环境配置) - [新项目创建](#新项目创建) @@ -590,8 +591,11 @@ DLoad 会自动处理构建过程： #### 二进制要素 -- **name**：用于引用的二进制文件名 +- **name**：用于引用的二进制文件名；必须是文件名，而不是路径 - **pattern**：匹配资源中二进制文件的正则表达式模式 +- **version-command**：让二进制文件输出版本的参数，例如 `--version`；未设置时不检查版本。 + 参数会原样传给二进制文件，不经过 shell：引号和管道不会被解释。 + 在 Composer 包的条目中只能是单个选项（如 `--version`）或 `version` - 自动处理操作系统/架构过滤 #### 文件要素 @@ -646,6 +650,56 @@ DLOAD_TOKEN_GHE_EXAMPLE_COM=your_token_here ./vendor/bin/dload get 允许使用普通的 `http`，例如用于测试中的本地模拟 API（`uri="http://127.0.0.1:8080/owner/repo"`），但令牌 只会通过它发送到 `localhost`、`127.0.0.0/8` 和 `[::1]`。 +### 来自 Composer 包的软件 + +框架或 CMS 可以随包附带它所需二进制文件的定义，这样使用它的项目就不必把这些定义复制到自己的 `dload.xml` 中。 +包在其 `composer.json` 中指向包内的一个 XML 文件： + +```json +{ + "extra": { + "dload": { + "registry": "resources/dload.xml" + } + } +} +``` + +该文件采用 `dload.xml` 格式，因此 IDE 可以为其提供补全和校验；只会读取其中的 `` 部分。 +它可以是专用文件，也可以是包自身的 `dload.xml`。请将其保留在包的 dist 中：不要对它使用 `export-ignore`。 + +```xml + + + + + + + + +``` + +DLoad 在已加载的 `dload.xml` 旁边查找 `composer.json`（没有 `dload.xml` 时在工作目录中查找），并从 +`vendor/composer/installed.json` 读取已安装的包（会遵循 `COMPOSER_VENDOR_DIR` 和 `config.vendor-dir`，仅支持 Composer 2）。 +无论是 Composer 包、PHAR 还是二进制文件，其行为都相同。 + +- **谁可以声明**：根包及其直接依赖（`require`、`require-dev`）。传递依赖不能添加软件。 +- **文件可以放在哪里**：只能在包内部。通过 `..`、绝对路径或符号链接指向包外的路径会被拒绝。不支持远程文件。 +- **优先级**：`dload.xml` > 官方注册表 > 包。包永远不会替换另外两者中的条目； + 当官方注册表遮蔽了正在下载的某个包条目时，DLoad 会发出警告。 +- **显式选择**：`#` 选择某个包的条目，无论它被什么遮蔽： + `dload get acme/framework#gh:^2.0` 或 ``。 +- **多个包使用同一个 id**：指向相同仓库的包彼此一致，无需报告。 + 否则根包优先，然后按 `require` 和 `require-dev` 中的顺序选择包， + 并且下载该 id 时会列出所有可供选择的 `#`。 +- **有问题的声明**会以警告形式报告并被跳过；它们永远不会使 DLoad 停止运行。包括：文件不存在、XML 无效、 + 必填属性为空、需要文件名的地方写了路径、无效的正则表达式，以及既不是单个选项（如 `--version`）也不是 `version` 的 `version-command`。 +- **关闭**：`` 丢弃来自包的软件并保留官方注册表； + `` 两者都丢弃。 + +`dload software` 会显示每个条目的来源：`official`、`dload.xml` 或包名。遮盖了其他条目的条目会在 `Shadows:` 中列出它们，包中的条目以 `#` 形式显示。 + ## 使用场景 ### 开发环境配置 diff --git a/README.md b/README.md index 6700098d..095e9431 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,7 @@ With DLoad, you can: - [Defining Software](#defining-software) - [Software Elements](#software-elements) - [Self-Hosted Servers](#self-hosted-servers) + - [Software from Composer Packages](#software-from-composer-packages) - [Use Cases](#use-cases) - [Development Environment Setup](#development-environment-setup) - [New Project Setup](#new-project-setup) @@ -602,8 +603,11 @@ This ensures consistent Velox versions across different environments and team me #### Binary Elements -- **name**: Binary name for reference +- **name**: Binary name for reference; a file name, not a path - **pattern**: Regex pattern to match binary in assets +- **version-command**: Arguments that make the binary print its version, e.g. `--version`; without it the version is not checked. + They are passed to the binary as they are, without a shell: quotes and pipes are not interpreted. + In entries of Composer packages it is one option such as `--version`, or `version` - Automatically handles OS/architecture filtering #### File Elements @@ -662,6 +666,59 @@ a server's token goes only to that exact scheme, host and port, and is dropped w Plain `http` is allowed, e.g. for a local fake API in tests (`uri="http://127.0.0.1:8080/owner/repo"`), but a token is sent over it only to `localhost`, `127.0.0.0/8` and `[::1]`. +### Software from Composer Packages + +A framework or a CMS can ship the definitions of the binaries it needs, so the projects that use it +do not copy them into their `dload.xml`. The package points to an XML file inside it in its `composer.json`: + +```json +{ + "extra": { + "dload": { + "registry": "resources/dload.xml" + } + } +} +``` + +The file has the `dload.xml` format, so the IDE completes and validates it; only its `` section is read. +It can be a dedicated file or the package's own `dload.xml`. Keep it in the package dist: do not `export-ignore` it. + +```xml + + + + + + + + +``` + +DLoad finds `composer.json` next to the loaded `dload.xml` (the working directory without one) and reads +the installed packages from `vendor/composer/installed.json` (`COMPOSER_VENDOR_DIR` and `config.vendor-dir` are honoured, Composer 2 only). +It works the same for the Composer package, the PHAR and the binary. + +- **Who can declare**: the root package and its direct dependencies (`require`, `require-dev`). + A transitive dependency cannot add software. +- **Where the file can be**: only inside the package. A path that leads out of it, through `..`, + an absolute path or a symlink, is rejected. Remote files are not supported. +- **Priority**: `dload.xml` > the official registry > packages. A package never replaces an entry of the other two; + when the official registry shadows a package entry that is being downloaded, DLoad warns about it. +- **Explicit choice**: `#` picks the entry of a package, whatever shadows it: + `dload get acme/framework#gh:^2.0` or ``. +- **Several packages, one id**: packages pointing to the same repositories agree, nothing to report. + Otherwise the root package wins, then the packages in the order of `require` and `require-dev`, + and a download of that id prints every `#` to choose from. +- **Broken declarations** are reported as warnings and skipped; they never stop DLoad. That covers a missing file, + an invalid XML, a required attribute left empty, a path where a file name is expected, an invalid regex, + and a `version-command` other than one option such as `--version`, or `version`. +- **Switching off**: `` drops the software of packages and keeps the official registry; + `` drops both. + +`dload software` shows where each entry comes from: `official`, `dload.xml` or the package name. An entry that hides others lists them under `Shadows:`, package ones as `#`. + ## Use Cases ### Development Environment Setup diff --git a/dload.xsd b/dload.xsd index a5b524da..e907a97c 100644 --- a/dload.xsd +++ b/dload.xsd @@ -21,7 +21,7 @@ - Software identifier to download + Software identifier to download. `package#id`, e.g. `acme/framework#gh`, picks the entry declared by that Composer package @@ -183,7 +183,7 @@ - Binary executable name + Binary executable name: a file name, not a path @@ -193,7 +193,7 @@ - Command argument to check binary version (e.g. "--version") + Arguments to check the binary version (e.g. "--version"), separated by spaces and passed without a shell @@ -246,7 +246,12 @@ - Replace the built-in software collection with custom ones + Replace the built-in software collection with custom ones; the software declared by Composer packages is dropped too + + + + + Add the software declared by the root package and the direct Composer dependencies in `extra.dload.registry` of their composer.json diff --git a/resources/software.schema.json b/resources/software.schema.json index 5c51c8e4..549cb64c 100644 --- a/resources/software.schema.json +++ b/resources/software.schema.json @@ -44,7 +44,7 @@ "properties": { "name": { "type": "string", - "description": "Binary executable name" + "description": "Binary executable name: a file name, not a path" }, "pattern": { "type": "string", @@ -52,7 +52,7 @@ }, "version-command": { "type": "string", - "description": "Command argument to check binary version (e.g. '--version')" + "description": "Arguments to check the binary version (e.g. '--version'), separated by spaces and passed without a shell" } } }, diff --git a/skills/dload-fetch-tool/SKILL.md b/skills/dload-fetch-tool/SKILL.md index 8ed0c559..a31f1d4a 100644 --- a/skills/dload-fetch-tool/SKILL.md +++ b/skills/dload-fetch-tool/SKILL.md @@ -15,7 +15,7 @@ Everything happens in the consuming project's own `dload.xml` (dload itself is i ./vendor/bin/dload software ``` -Prints every built-in tool with its alias. Tool listed → Step 3. Not listed → Step 2. +Prints every tool dload knows with its alias and, in brackets, its source: `official`, `dload.xml`, or the Composer package that declares it. Tool listed → Step 3. Not listed → Step 2. ## Step 2 — describe a missing tool inline @@ -23,6 +23,8 @@ Add a `` block to the `` element of the pr Done when the block has a `` whose `asset-pattern` matches every OS/arch variant of the tool and nothing else in the release. +When the tool is needed by a Composer package (a framework, a CMS) rather than by one project, the package declares the block for every project that installs it: read [`references/package-registry.md`](references/package-registry.md). + ## Step 3 — add the `` action ```xml @@ -44,7 +46,7 @@ Done when the block has a `` whose `asset-pattern` matches every OS/ | Attribute | Purpose | |---|---| -| `software` | Alias or name of the tool (built-in or inline). Required. | +| `software` | Alias or name of the tool (built-in or inline). `#` (`acme/framework#gh`) picks the entry of that Composer package, whatever shadows it. Required. | | `version` | Composer-style constraint: `^2025.1`, `~1.0.0`, `^2.12.0@beta`, `^2.12.0-hotfix@alpha` (feature builds, alpha ones included), one pre-release like `2025.1.0-rc.2`, or a range from one like `^2025.1.0-rc.1`. Omit for latest stable. Pre-releases order by stability (`nightly < snapshot < dev < unstable < alpha < preview < beta < pre < RC < stable`), then by number; `stable` by default. A pre-release in a constraint sets the minimum stability for any operator, even `<3.5.0-beta.2`; lower it with `@alpha`/`@dev`. A keyword without a number is a bound too: `>2.0-beta` takes `2.0.0-RC1` and `2.0.0`. An exact version with a pre-release or another suffix is that one tag: `2.0-beta` skips `2.0.0-beta.3`, `2.12.0-hotfix` skips `2.12.0-hotfix.2`; after `^`, `~` or a comparison a non-stability suffix is a feature the tag must contain. A tagged build like `2025.1.0-rc.2-linux` orders right after its pre-release: an exact `2025.1.0-rc.2` skips it, `>2025.1.0-rc.2` takes it. Numbers may have more than three parts (`1.2.3.4`), in constraints too (`>=1.2.3.4.5`); trailing zero parts and build metadata don't count (`1.2.3` = `1.2.3.0` = `1.2.3+build.5`). Case doesn't matter (`-ARM64` = `-arm64`, `^1.0-Linux` takes `1.0.0-linux`), and a pre-release may be glued to the number as in tags (`^2.0.0rc1`). | | `extract-path` | Target folder (default: project root). | | `type` | `binary` (default), `phar` (required for PHAR — skips extraction), `archive` (unpack the whole asset keeping its folder layout). | diff --git a/skills/dload-fetch-tool/references/package-registry.md b/skills/dload-fetch-tool/references/package-registry.md new file mode 100644 index 00000000..d3c0a78d --- /dev/null +++ b/skills/dload-fetch-tool/references/package-registry.md @@ -0,0 +1,46 @@ +# Registry entries shipped by a Composer package + +A package declares `` blocks once, and every project that requires it can `dload get` them without copying anything into its own `dload.xml`. + +## Declare + +In the package's `composer.json`, point to an XML file inside the package: + +```json +{ + "extra": { + "dload": { + "registry": "resources/dload.xml" + } + } +} +``` + +The file has the `dload.xml` format; only its `` section is read, so the package's own `dload.xml` works when its registry is what consumers need. Write the blocks as in [`registry-entry.md`](registry-entry.md). + +- The path stays inside the package: `..`, an absolute path or a symlink out of it is rejected. Remote files are not supported. +- The file ships in the dist: keep it out of `export-ignore` in `.gitattributes`. +- The ids must not contain `#` or `:`, nor start or end with a space. + +Done when `dload software`, run in a project that requires the package (or in the package itself), lists the entry with the package name in brackets — or, when `dload.xml` or the official registry takes the id, lists `#` under that entry's `Shadows:`. + +## Who is read + +DLoad reads `composer.json` next to the loaded `dload.xml` (the working directory without one) and the installed packages from `/composer/installed.json` (Composer 2). Entries come from the root package and its direct dependencies (`require`, `require-dev`); a transitive dependency adds nothing. A broken declaration — missing file, invalid XML, an element without a required attribute such as `name`, `uri` or `binary.name`, a path in `binary.name` or `file.rename`, an invalid regex, a `version-command` other than one option such as `--version` or `version`, an id with `#`, `:` or surrounding spaces — is a warning and is skipped. + +## Priority + +`dload.xml` > official registry > packages. Among packages: the root package, then the order of `require`, then `require-dev`. + +| Situation | What DLoad does on download | +|---|---| +| `dload.xml` declares the same id | Uses it, silently. | +| The official registry declares the same id | Uses the official one and warns, naming `#` to get the package's one. | +| Several packages declare the id with the same repositories | Uses the first, silently. | +| Several packages declare the id with different repositories | Uses the first and lists every `#` to choose from. | + +Warnings appear only for a tool that is actually downloaded. `#` (`dload get acme/framework#gh:^2.0`, ``) always picks that package's entry. + +## Switch off + +In the project's `dload.xml`: `` drops package entries and keeps the official registry; `` drops both. diff --git a/skills/dload-fetch-tool/references/registry-entry.md b/skills/dload-fetch-tool/references/registry-entry.md index d6912755..38388d81 100644 --- a/skills/dload-fetch-tool/references/registry-entry.md +++ b/skills/dload-fetch-tool/references/registry-entry.md @@ -1,6 +1,6 @@ # Inline registry entry -How to describe a tool dload does not ship, as a `` block inside `` in the project's `dload.xml`. `overwrite="false"` (the default) merges the entry with the built-in registry; `overwrite="true"` replaces the built-in registry entirely. +How to describe a tool dload does not ship, as a `` block inside `` in the project's `dload.xml`. `overwrite="false"` (the default) merges the entry with the built-in registry; `overwrite="true"` replaces the built-in registry entirely, along with the entries of [Composer packages](package-registry.md). ## Facts to collect @@ -12,7 +12,7 @@ How to describe a tool dload does not ship, as a `` block inside `readXml($xml); + $configFile = $xml === null || \str_starts_with($xml, 'absolute(); // Register config hydration as an inflector: every object produced by the container passes // through it, and it hydrates the ones marked with #[InflectableConfig]. @@ -154,6 +157,18 @@ static function (Container $container): VersionRegistry { ); }, ); + $this->container->bind( + SoftwareCollector::class, + static fn(Container $container): SoftwareCollector => new SoftwareCollector( + $container->get(CustomSoftwareRegistry::class), + new ComposerPackages( + $configFile?->parent() ?? Path::create((string) \getcwd()), + $configFile, + $container->get(Logger::class), + $environment['COMPOSER_VENDOR_DIR'] ?? null, + ), + ), + ); $this->container->bind( SoftwareCollection::class, static fn(Container $container): SoftwareCollection => $container->get(SoftwareCollector::class)->collect(), diff --git a/src/Command/Get.php b/src/Command/Get.php index 81730046..49392a95 100644 --- a/src/Command/Get.php +++ b/src/Command/Get.php @@ -13,6 +13,7 @@ use Internal\DLoad\Module\Config\Schema\Action\Download as DownloadConfig; use Internal\DLoad\Module\Config\Schema\Actions; use Internal\DLoad\Module\Downloader\Exception\DownloadFailed; +use Internal\DLoad\Service\Logger; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\InvalidArgumentException; @@ -72,7 +73,7 @@ public function configure(): void $this->addArgument( self::ARG_SOFTWARE, InputArgument::OPTIONAL | InputArgument::IS_ARRAY, - 'Software name, e.g. "rr", "dolt", "temporal" etc.', + 'Software name, e.g. "rr", "dolt", "temporal" etc., or "#" for the one a Composer package declares', ); $this->addOption('path', null, InputOption::VALUE_OPTIONAL, 'Path to store the binary, e.g. "./bin"', "."); $this->addOption('arch', null, InputOption::VALUE_OPTIONAL, 'Architecture, e.g. "amd64", "arm64" etc.'); @@ -281,8 +282,8 @@ private function reportFailures(OutputInterface $output, array $failures, int $t { foreach ($failures as [$software, $error]) { $output->writeln(''); - $output->writeln(\sprintf(' Failed to download `%s` ', OutputFormatter::escape($software))); - $output->writeln(OutputFormatter::escape(self::describeFailure($error))); + $output->writeln(\sprintf(' Failed to download `%s` ', OutputFormatter::escape(Logger::printable($software)))); + $output->writeln(OutputFormatter::escape(Logger::printable(self::describeFailure($error)))); $output->isVeryVerbose() and $this->logger->exception($error, important: true); } diff --git a/src/Command/ListSoftware.php b/src/Command/ListSoftware.php index cb982127..4c6a4143 100644 --- a/src/Command/ListSoftware.php +++ b/src/Command/ListSoftware.php @@ -4,10 +4,13 @@ namespace Internal\DLoad\Command; -use Internal\DLoad\Module\Config\Schema\Embed\Software; +use Internal\DLoad\Module\Software\Entry; +use Internal\DLoad\Module\Software\OriginKind; use Internal\DLoad\Module\Software\SoftwareCollection; +use Internal\DLoad\Service\Logger; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -56,16 +59,36 @@ protected function execute( $output->writeln('There are ' . $registry->count() . ' software available:'); $output->writeln(''); - /** @var Software $software */ - foreach ($registry->getIterator() as $software) { - $output->writeln("{$software->getId()} $software->name"); - $software->homepage and $output->writeln("Homepage: $software->homepage"); + // Definitions may come from Composer packages: their text must turn neither into console tags + // nor into terminal control sequences + $e = static fn(string $text): string => OutputFormatter::escape(Logger::printable($text)); + + foreach ($registry->entries() as $entry) { + $software = $entry->software; + $output->writeln(\sprintf( + '%s %s [%s]', + $e($software->getId()), + $e($software->name), + $e((string) $entry->origin), + )); + + $shadowed = \array_map( + static fn(Entry $other): string => $other->origin->kind === OriginKind::Package + ? "{$other->origin}#{$other->getId()}" + : (string) $other->origin, + \array_filter( + $registry->candidates($software->getId()), + static fn(Entry $other): bool => $other !== $entry, + ), + ); + $shadowed === [] or $output->writeln('Shadows: ' . $e(\implode(', ', $shadowed)) . ''); + $software->homepage and $output->writeln('Homepage: ' . $e($software->homepage) . ''); foreach ($software->repositories as $repo) { - $output->writeln("{$repo->type}: {$repo->uri}"); + $output->writeln('' . $e("{$repo->type}: {$repo->uri}") . ''); } - $software->description and $output->writeln("" . \wordwrap($software->description, 78) . ""); + $software->description and $output->writeln('' . $e(\wordwrap($software->description, 78)) . ''); $output->writeln(''); } diff --git a/src/Command/Show.php b/src/Command/Show.php index 88b44a75..591e8291 100644 --- a/src/Command/Show.php +++ b/src/Command/Show.php @@ -9,10 +9,13 @@ use Internal\DLoad\Module\Common\Input\Destination; use Internal\DLoad\Module\Config\Schema\Action\Download; use Internal\DLoad\Module\Config\Schema\Actions; +use Internal\DLoad\Module\Software\OriginKind; use Internal\DLoad\Module\Software\SoftwareCollection; +use Internal\DLoad\Service\Logger; use Internal\Path; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -68,6 +71,15 @@ protected function execute( return $this->listAllSoftware($collection, $binaryProvider, $destination, $actions, $input, $output); } + /** + * Makes registry text safe to print: definitions may come from Composer packages, and their text + * must turn neither into console tags nor into terminal control sequences. + */ + private static function text(string $text): string + { + return OutputFormatter::escape(Logger::printable($text)); + } + private function listAllSoftware( SoftwareCollection $collection, BinaryProvider $binaryProvider, @@ -86,44 +98,43 @@ private function listAllSoftware( $configDownloads[$download->software] = $download; } } - $configSoftwareIds = \array_keys($configDownloads); + // Definitions the downloads resolve to: `acme/fw#gh` may be a shadowed one + $configSoftware = []; + foreach (\array_keys($configDownloads) as $id) { + $software = $collection->findSoftware($id); + $software === null or $configSoftware[] = $software; + } // Track downloaded software to avoid showing them twice - $downloadedSoftwareIds = []; + $downloadedSoftware = []; // BLOCK 1: Software configured in project $output->writeln('Configured software:'); - $configSoftwareIds === [] and $output + $configDownloads === [] and $output ->writeln(' No configured software found'); - if ($configSoftwareIds !== []) { + if ($configDownloads !== []) { $foundConfigured = false; - foreach ($collection as $software) { - if ($software->binary === null) { - continue; - } - - if (!\in_array($software->getId(), $configSoftwareIds, true)) { + foreach ($configDownloads as $id => $download) { + $software = $collection->findSoftware($id); + if ($software === null || $software->binary === null) { continue; } - $perDownloadPath = $this->resolveDestinationPath( - $destination, - $configDownloads[$software->getId()] ?? null, - ); + $perDownloadPath = $this->resolveDestinationPath($destination, $download); $binary = $binaryProvider->getLocalBinary($perDownloadPath, $software->binary, $software->name); if ($binary === null) { continue; } $foundConfigured = true; - $downloadedSoftwareIds[] = $software->getId(); + $downloadedSoftware[] = $software; $output->writeln(\sprintf( ' %s (%s) %s', - $software->getId(), - $binary->getVersionString() ?? 'unknown', + self::text($id), + self::text($binary->getVersionString() ?? 'unknown'), $binary->getPath(), )); } @@ -140,18 +151,24 @@ private function listAllSoftware( $output->writeln('Downloaded software (not configured):'); $foundDownloaded = false; - foreach ($collection as $software) { + foreach ($collection->entries() as $entry) { + $software = $entry->software; if ($software->binary === null) { continue; } + // Checking the version runs the binary, and nobody asked for a package's software here + if ($entry->origin->kind === OriginKind::Package) { + continue; + } + // Skip software already shown in configured block - if (\in_array($software->getId(), $downloadedSoftwareIds, true)) { + if (\in_array($software, $downloadedSoftware, true)) { continue; } // Skip software in project config - if (\in_array($software->getId(), $configSoftwareIds, true)) { + if (\in_array($software, $configSoftware, true)) { continue; } @@ -161,12 +178,12 @@ private function listAllSoftware( } $foundDownloaded = true; - $downloadedSoftwareIds[] = $software->getId(); + $downloadedSoftware[] = $software; $output->writeln(\sprintf( ' %s (%s) %s', - $software->getId(), - $binary->getVersionString() ?? 'unknown', + self::text($software->getId()), + self::text($binary->getVersionString() ?? 'unknown'), $binary->getPath(), )); } @@ -187,7 +204,7 @@ private function listAllSoftware( } // Skip software already shown in downloaded blocks - if (\in_array($software->getId(), $downloadedSoftwareIds, true)) { + if (\in_array($software, $downloadedSoftware, true)) { continue; } @@ -195,8 +212,8 @@ private function listAllSoftware( $output->writeln(\sprintf( ' %s %s', - $software->getId(), - $software->description ? '- ' . $software->description : '', + self::text($software->getId()), + self::text($software->description ? '- ' . $software->description : ''), )); } @@ -222,12 +239,13 @@ private function showSoftwareDetails( ?Actions $actions, OutputInterface $output, ): int { - $software = $collection->findSoftware($softwareName); + $entry = $collection->find($softwareName); - if ($software === null) { + if ($entry === null) { $output->writeln(\sprintf('Software "%s" not found in registry', $softwareName)); return Command::FAILURE; } + $software = $entry->software; if ($software->binary === null) { $output->writeln(\sprintf('Software "%s" does not have a binary', $softwareName)); @@ -242,7 +260,7 @@ private function showSoftwareDetails( if ($actions !== null) { foreach ($actions->downloads as $download) { - if ($download->software === $software->getId()) { + if ($collection->find($download->software) === $entry) { $inConfig = true; $configConstraints = $download->version; $configExtractPath = $download->extractPath; @@ -255,14 +273,15 @@ private function showSoftwareDetails( $destinationPath = $this->resolveDestinationPath($destination, $matchedDownload); // Display detailed information - $output->writeln(\sprintf('Software: %s', $software->name)); + $output->writeln(\sprintf('Software: %s', self::text($software->name))); $software->alias === null or $software->alias === $software->name or $output - ->writeln(\sprintf('Alias: %s', $software->alias)); + ->writeln(\sprintf('Alias: %s', self::text($software->alias))); $software->description and $output - ->writeln(\sprintf('Description: %s', $software->description)); + ->writeln(\sprintf('Description: %s', self::text($software->description))); $software->homepage === null or $output - ->writeln(\sprintf('Homepage: %s', $software->homepage)); + ->writeln(\sprintf('Homepage: %s', self::text($software->homepage))); + $output->writeln(\sprintf('Source: %s', self::text((string) $entry->origin))); // Show project config information $output->writeln(''); @@ -308,7 +327,7 @@ private function displayBinaryDetails(?Binary $binary, OutputInterface $output): $output->writeln('Binary information:'); $output->writeln(\sprintf(' Full path: %s', $binaryPath->absolute())); - $output->writeln(\sprintf(' Version: %s', $binary->getVersionString() ?? 'unknown')); + $output->writeln(\sprintf(' Version: %s', self::text($binary->getVersionString() ?? 'unknown'))); $output->writeln(\sprintf(' Size: %s', $this->formatSize($binary->getSize()))); $mtime = $binary->getMTime(); diff --git a/src/DLoad.php b/src/DLoad.php index 28bab5f5..d5f2c372 100644 --- a/src/DLoad.php +++ b/src/DLoad.php @@ -14,6 +14,7 @@ use Internal\DLoad\Module\Downloader\Task\DownloadResult; use Internal\DLoad\Module\Downloader\Task\DownloadTask; use Internal\DLoad\Module\Installer\Installer; +use Internal\DLoad\Module\Software\ShadowReport; use Internal\DLoad\Module\Software\SoftwareCollection; use Internal\DLoad\Module\Task\Manager; use Internal\DLoad\Module\Version\Constraint; @@ -55,6 +56,7 @@ public function __construct( private readonly Installer $installer, private readonly Destination $configDestination, private readonly BinaryProvider $binaryProvider, + private readonly ShadowReport $shadowReport, ) {} /** @@ -128,6 +130,7 @@ public function addTask(DownloadConfig $action, bool $force = false): PromiseInt } add_task: + $this->shadowReport->report($this->softwareCollection, $action->software); return $this->taskManager->addTask(function () use ($software, $action): DloadResult { // Create a Download task diff --git a/src/Module/Binary/Internal/AbstractBinary.php b/src/Module/Binary/Internal/AbstractBinary.php index 94848d2c..cbd47b3b 100644 --- a/src/Module/Binary/Internal/AbstractBinary.php +++ b/src/Module/Binary/Internal/AbstractBinary.php @@ -31,9 +31,7 @@ public function getName(): string public function execute(string ...$args): array { - $args = \array_map(static fn(string $arg): string => \escapeshellarg($arg), $args); - - return $this->executor->execute($this->getPath(), \implode(' ', $args)); + return $this->executor->execute($this->getPath(), ...$args); } public function exists(): bool @@ -80,7 +78,8 @@ public function getVersion(): ?BinaryVersion } try { - $output = $this->executor->execute($this->getPath(), $this->config->versionCommand); + $arguments = \preg_split('/\s+/', \trim($this->config->versionCommand), -1, \PREG_SPLIT_NO_EMPTY); + $output = $this->executor->execute($this->getPath(), ...($arguments === false ? [] : $arguments)); return $this->versionOutput = BinaryVersion::fromBinaryOutput(\implode("\n", $output)); } catch (\Throwable) { return $this->versionOutput = BinaryVersion::empty(); diff --git a/src/Module/Binary/Internal/BinaryExecutor.php b/src/Module/Binary/Internal/BinaryExecutor.php index f9290aec..d813a09e 100644 --- a/src/Module/Binary/Internal/BinaryExecutor.php +++ b/src/Module/Binary/Internal/BinaryExecutor.php @@ -23,24 +23,35 @@ public function __construct( * Executes a binary with the specified command and returns the output. * * @param Path $binaryPath Full path to binary executable - * @param string $command Command argument(s) to execute + * @param string ...$arguments Each one reaches the binary as is: quotes, pipes and other shell + * syntax are not interpreted. * @return list Command output * @throws BinaryExecutionException If execution fails */ - public function execute(Path $binaryPath, string $command): array + public function execute(Path $binaryPath, string ...$arguments): array { - // Escape command for shell execution - $escapedPath = \escapeshellarg((string) $binaryPath); + $command = \implode(' ', $arguments); + $this->logger->debug('Executing command: %s %s', (string) $binaryPath, $command); - /** @var list $output */ - $output = []; - $returnCode = 0; + // An array command starts the binary without a shell: escapeshellarg() on Windows would replace + // `"`, `%` and `!` in arguments with spaces. The closed stdin keeps an interactive binary from waiting. + $process = \proc_open( + [(string) $binaryPath, ...$arguments], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['redirect', 1]], + $pipes, + ); + $process === false and throw new BinaryExecutionException( + \sprintf('Failed to execute binary "%s" with command "%s".', $binaryPath, $command), + ); - $cmd = "$escapedPath $command 2>&1"; - $this->logger->debug('Executing command: %s', $cmd); + \fclose($pipes[0]); + $stdout = (string) \stream_get_contents($pipes[1]); + \fclose($pipes[1]); + $returnCode = \proc_close($process); - // Execute with both stdout and stderr redirected to output - \exec($cmd, $output, $returnCode); + // Lines without trailing whitespace, as exec() returns them + $stdout = \rtrim($stdout); + $output = $stdout === '' ? [] : \array_map(\rtrim(...), \preg_split('/\r?\n/', $stdout) ?: []); // If command failed, throw exception if ($returnCode !== 0) { diff --git a/src/Module/Binary/Internal/BinaryProviderImpl.php b/src/Module/Binary/Internal/BinaryProviderImpl.php index 451cdfca..10c4413f 100644 --- a/src/Module/Binary/Internal/BinaryProviderImpl.php +++ b/src/Module/Binary/Internal/BinaryProviderImpl.php @@ -6,6 +6,7 @@ use Internal\DLoad\Module\Binary\Binary; use Internal\DLoad\Module\Binary\BinaryProvider; +use Internal\DLoad\Module\Common\FileSystem\FS; use Internal\DLoad\Module\Common\OperatingSystem; use Internal\DLoad\Module\Config\Schema\Embed\Binary as BinaryConfig; use Internal\Path; @@ -24,6 +25,10 @@ public function __construct( public function getLocalBinary(Path|string $destinationPath, BinaryConfig $config, ?string $name = null): ?Binary { + if (!FS::isFileName($config->name)) { + return null; + } + // Get binary path $binaryPath = $this->buildBinaryPath($destinationPath, $config); @@ -41,8 +46,13 @@ public function getLocalBinary(Path|string $destinationPath, BinaryConfig $confi public function getGlobalBinary(BinaryConfig $config, ?string $name = null): ?Binary { + $name ??= $config->name; + if (!FS::isFileName($name)) { + return null; + } + $binary = new GlobalBinary( - name: $name ?? $config->name, + name: $name, config: $config, executor: $this->executor, ); diff --git a/src/Module/Common/FileSystem/FS.php b/src/Module/Common/FileSystem/FS.php index df06fc2e..24cb6049 100644 --- a/src/Module/Common/FileSystem/FS.php +++ b/src/Module/Common/FileSystem/FS.php @@ -124,4 +124,38 @@ public static function moveFile(Path $from, Path $to, bool $overwrite = true): b return \rename($from->__toString(), $to->__toString()); } + + /** + * Checks that a name from a config is a bare file name, so a join with a directory stays in it. + * + * ```php + * FS::isFileName('rr'); // true + * FS::isFileName('../.git/hooks'); // false + * ``` + */ + public static function isFileName(string $name): bool + { + return \trim($name) !== '' + && !\in_array($name, ['.', '..'], true) + && \strpbrk($name, "/\\\0:") === false + // Windows drops trailing dots and spaces, and maps reserved names to devices + && \rtrim($name, '. ') === $name + && \preg_match('/^(CON|PRN|AUX|NUL|CONIN\$|CONOUT\$|COM[1-9¹²³]|LPT[1-9¹²³])(\..*)?$/iu', $name) !== 1; + } + + /** + * Turns free text, such as a release asset name, into a file name {@see self::isFileName()} accepts, + * keeping its extension. + * + * ```php + * FS::toFileName('Binary: tool.tar.gz'); // "Binary_ tool.tar.gz" + * FS::toFileName('../aux.zip'); // ".._aux.zip" + * ``` + */ + public static function toFileName(string $name): string + { + $name = \rtrim((string) \preg_replace('/[\/\\\\:\x00-\x1F\x7F]/', '_', $name), '. '); + + return self::isFileName($name) ? $name : '_' . $name; + } } diff --git a/src/Module/Config/Schema/CustomSoftwareRegistry.php b/src/Module/Config/Schema/CustomSoftwareRegistry.php index fe4694b7..ee4dde32 100644 --- a/src/Module/Config/Schema/CustomSoftwareRegistry.php +++ b/src/Module/Config/Schema/CustomSoftwareRegistry.php @@ -20,6 +20,10 @@ final class CustomSoftwareRegistry #[XPath('/dload/registry/@overwrite')] public bool $overwrite = false; + /** @var bool $fromPackages Add the software declared by Composer packages of the project */ + #[XPath('/dload/registry/@from-packages')] + public bool $fromPackages = true; + /** * @var \Internal\DLoad\Module\Config\Schema\Embed\Software[] $software Custom software definitions from the configuration */ diff --git a/src/Module/Downloader/Downloader.php b/src/Module/Downloader/Downloader.php index a8554c83..29cc0172 100644 --- a/src/Module/Downloader/Downloader.php +++ b/src/Module/Downloader/Downloader.php @@ -501,8 +501,8 @@ private function describeFormatFilter(DownloadConfig $actionOptions): string private function processAsset(DownloadContext $context): \Closure { return function () use ($context): \SplFileObject { - // Create a file - $temp = $context->tempDir->join($context->asset->getName()); + // The asset name is free text of the release API of a host the config names + $temp = $context->tempDir->join(FS::toFileName($context->asset->getName())); $file = new \SplFileObject((string) $temp, 'wb+'); $this->logger->info('Downloading into %s', (string) $temp); diff --git a/src/Module/Installer/Installer.php b/src/Module/Installer/Installer.php index 52564658..bb1e0d9c 100644 --- a/src/Module/Installer/Installer.php +++ b/src/Module/Installer/Installer.php @@ -106,7 +106,7 @@ private function binaryRule(?BinaryConfig $binary): ?File $result = new File(); $result->pattern = $binary->pattern - ?? "/^{$binary->name}{$this->os->getBinaryExtension()}$/"; + ?? '/^' . \preg_quote($binary->name . $this->os->getBinaryExtension(), '/') . '$/'; $result->rename = $binary->name; $result->chmod = 0o755; // Default permissions for binaries diff --git a/src/Module/Installer/Internal/RuleExtraction.php b/src/Module/Installer/Internal/RuleExtraction.php index 76e066dd..143de3bb 100644 --- a/src/Module/Installer/Internal/RuleExtraction.php +++ b/src/Module/Installer/Internal/RuleExtraction.php @@ -7,6 +7,7 @@ use Internal\DLoad\Module\Archive\ArchiveFactory; use Internal\DLoad\Module\Binary\BinaryProvider; use Internal\DLoad\Module\Common\DloadResult; +use Internal\DLoad\Module\Common\FileSystem\FS; use Internal\DLoad\Module\Config\Schema\Embed\File; use Internal\DLoad\Module\Downloader\Exception\NothingExtracted; use Internal\DLoad\Module\Installer\Installation; @@ -129,9 +130,13 @@ private static function shouldBeExtracted(\SplFileInfo $source, array $mapping, $newName = match (true) { $conf->rename === null => $source->getFilename(), - !$hasRealExtension => $conf->rename, + !$hasRealExtension, + \str_ends_with(\strtolower($conf->rename), '.' . \strtolower($extension)) => $conf->rename, default => $conf->rename . '.' . $extension, }; + $conf->rename === null or FS::isFileName($newName) or throw new \RuntimeException( + "Cannot install `{$source->getFilename()}` as `{$newName}`: the name must be a file name, not a path.", + ); return [new \SplFileInfo((string) $path->join($newName)), $conf]; } diff --git a/src/Module/Software/ComposerPackages.php b/src/Module/Software/ComposerPackages.php new file mode 100644 index 00000000..7381e0a8 --- /dev/null +++ b/src/Module/Software/ComposerPackages.php @@ -0,0 +1,386 @@ +` section is read. Packages come from + * `/composer/installed.json` (Composer 2 format), so no Composer plugin is involved. + * + * A broken declaration never fails the run: it is reported as a warning and skipped. + * + * ```php + * $packages = new ComposerPackages(Path::create('.'), Path::create('dload.xml'), $logger); + * foreach ($packages->entries() as $entry) { + * echo $entry->origin, ': ', $entry->getId(), "\n"; + * } + * ``` + * + * @internal + */ +final class ComposerPackages +{ + /** Name Composer gives a root package without one */ + private const ROOT_NAME = '__root__'; + + /** + * @param Path $projectDir Directory of the root `composer.json`. + * @param Path|null $configFile The loaded `dload.xml`: its registry is already in the collection, + * so a package pointing to it is not read a second time. + * @param string|null $vendorDir Overrides `config.vendor-dir`, as `COMPOSER_VENDOR_DIR` does for Composer. + */ + public function __construct( + private readonly Path $projectDir, + private readonly ?Path $configFile, + private readonly Logger $logger, + private readonly ?string $vendorDir = null, + ) {} + + /** + * @return list In the order of precedence between packages: the root package first, + * then the packages of the root `require`, then of `require-dev`, each in the order listed. + */ + public function entries(): array + { + $root = $this->readJson($this->projectDir->join('composer.json')); + if ($root === null) { + return []; + } + + $entries = $this->entriesOf(self::packageName($root) ?? self::ROOT_NAME, $root, $this->projectDir); + + $installed = $this->installedPackages($root); + // A package listed in both `require` and `require-dev` keeps its first place. + $names = \array_unique(\array_map( + static fn(int|string $name): string => \strtolower((string) $name), + [...\array_keys((array) ($root['require'] ?? [])), ...\array_keys((array) ($root['require-dev'] ?? []))], + )); + foreach ($names as $name) { + $package = $installed[$name] ?? null; + $package === null or \array_push($entries, ...$this->entriesOf($package[0], $package[1], $package[2])); + } + + return $entries; + } + + /** + * @return non-empty-string|null What makes the definition unusable, or null when it is usable. + */ + private static function problemOf(Software $software): ?string + { + $missing = self::missingAttribute($software); + if ($missing !== null) { + return \sprintf('`` has no `%s`', $missing); + } + + $id = $software->getId(); + if (\trim($id) === '') { + return '`` has an empty id'; + } + + if (\trim($id) !== $id || \strpbrk($id, '#:') !== false) { + return \sprintf('the id `%s` cannot be requested: it has spaces around it, `#` or `:`', $id); + } + + $children = [ + 'binary' => $software->binary === null ? [] : [$software->binary], + 'repository' => $software->repositories, + 'file' => $software->files, + ]; + foreach ($children as $element => $items) { + foreach ($items as $item) { + $missing = self::missingAttribute($item); + if ($missing !== null) { + return \sprintf('a `<%s>` of `%s` has no `%s`', $element, $id, $missing); + } + } + } + + $names = [ + $software->binary?->name, + ...\array_map(static fn(File $file): ?string => $file->rename, $software->files), + ]; + foreach ($names as $name) { + if ($name !== null && !FS::isFileName($name)) { + return \sprintf('`%s` of `%s` must be a file name, not a path', $name, $id); + } + } + + // Escaping keeps the shell out, not the binary: a package naming a binary the project already + // has would choose its arguments, e.g. `bun vendor/evil/script.js` or `rr reset`. + $versionCommand = $software->binary?->versionCommand; + if ($versionCommand !== null && \preg_match('/^(--?[A-Za-z][A-Za-z-]*|version)$/', $versionCommand) !== 1) { + return \sprintf('the version command `%s` of `%s` must be one option like `--version`, or `version`', $versionCommand, $id); + } + + $patterns = [ + $software->binary?->pattern, + ...\array_map(static fn(Repository $repository): string => $repository->assetPattern, $software->repositories), + ...\array_map(static fn(File $file): string => $file->pattern, $software->files), + ]; + foreach ($patterns as $pattern) { + if ($pattern !== null && @\preg_match($pattern, '') === false) { + return \sprintf('the pattern `%s` of `%s` is not a valid regular expression', $pattern, $id); + } + } + + return null; + } + + /** + * The hydrator leaves a required property uninitialized when its attribute is missing in the XML, + * and sets it to an empty string when the attribute is empty. + * + * @return non-empty-string|null Name of the first required property without a value. + */ + private static function missingAttribute(object $config): ?string + { + foreach ((new \ReflectionObject($config))->getProperties(\ReflectionProperty::IS_PUBLIC) as $property) { + if ($property->isStatic() || $property->hasDefaultValue()) { + continue; + } + + if (!$property->isInitialized($config)) { + return $property->getName(); + } + + /** @var mixed $value */ + $value = $property->getValue($config); + if (\is_string($value) && \trim($value) === '') { + return $property->getName(); + } + } + + return null; + } + + /** + * @return non-empty-string|null + */ + private static function packageName(array $package): ?string + { + /** @var mixed $name */ + $name = $package['name'] ?? null; + return \is_string($name) && $name !== '' ? $name : null; + } + + /** + * @return array Name, `composer.json` data and + * install path of each installed package, by lowercased name. + */ + private function installedPackages(array $root): array + { + /** @var mixed $vendorDir */ + $vendorDir = $this->vendorDir === null || $this->vendorDir === '' + ? ((array) ($root['config'] ?? []))['vendor-dir'] ?? 'vendor' + : $this->vendorDir; + if (!\is_string($vendorDir) || $vendorDir === '' || \str_contains($vendorDir, "\0")) { + $this->logger->warning('Skipping registries of Composer packages: `config.vendor-dir` is not a path.'); + return []; + } + + $vendorPath = Path::create($vendorDir); + $vendorPath->isAbsolute() or $vendorPath = $this->projectDir->join($vendorDir); + + $file = $vendorPath->join('composer', 'installed.json'); + if (!$file->exists()) { + $this->logger->debug('Composer packages are not installed: `%s` not found.', (string) $file); + return []; + } + + $json = $this->readJson($file); + if ($json === null) { + return []; + } + + if (!\is_array($json['packages'] ?? null)) { + $this->logger->warning( + 'Skipping registries of Composer packages: `%s` is not in the Composer 2 format.', + (string) $file, + ); + return []; + } + + $result = []; + /** @var mixed $package */ + foreach ($json['packages'] as $package) { + if (!\is_array($package)) { + continue; + } + + $name = self::packageName($package); + $installPath = $package['install-path'] ?? null; + if ($name === null || !\is_string($installPath) || $installPath === '' || \str_contains($installPath, "\0")) { + continue; + } + + // Composer writes the install path relative to the directory of installed.json. + $path = Path::create($installPath); + $path->isAbsolute() or $path = $file->parent()->join($installPath); + $result[\strtolower($name)] = [$name, $package, $path]; + } + + return $result; + } + + /** + * @param non-empty-string $name + * @param array $package Data of the package's `composer.json`. + * @return list + */ + private function entriesOf(string $name, array $package, Path $installPath): array + { + /** @var mixed $dload */ + $dload = ((array) ($package['extra'] ?? []))['dload'] ?? null; + if ($dload === null) { + return []; + } + + if (!\is_array($dload)) { + $this->logger->warning( + 'Package `%s`: `extra.dload` must be an object, e.g. `{"registry": "path/to/dload.xml"}`.', + $name, + ); + return []; + } + + /** @var mixed $registry */ + $registry = $dload['registry'] ?? null; + if ($registry === null) { + return []; + } + + if (!\is_string($registry) || $registry === '' || \str_contains($registry, "\0")) { + $this->logger->warning('Package `%s`: `extra.dload.registry` must be a path to a file.', $name); + return []; + } + + $file = $this->registryFile($name, $registry, $installPath); + if ($file === null) { + return []; + } + + $origin = Origin::package($name); + return \array_map( + static fn(Software $software): Entry => new Entry($software, $origin), + $this->readRegistry($name, $file), + ); + } + + /** + * Resolves the declared registry file, accepting only a file inside the package. + * + * Both sides are compared as real paths: the lexical check alone lets a symlink out of the + * package, and the install path of a Composer path repository is itself a symlink. + * + * @return Path|null Null when the file must not be read; the reason is already reported. + */ + private function registryFile(string $name, string $registry, Path $installPath): ?Path + { + $packageDir = \realpath((string) $installPath); + $file = Path::create($registry)->isAbsolute() ? null : \realpath((string) $installPath->join($registry)); + + if ($packageDir === false || $file === false) { + $this->logger->warning( + 'Package `%s`: registry file `%s` not found. Make sure it is not excluded from the package dist.', + $name, + $registry, + ); + return null; + } + + if ($file === null || !Path::create($file)->isWithin(Path::create($packageDir))) { + $this->logger->warning('Package `%s`: registry file `%s` is outside the package, skipped.', $name, $registry); + return null; + } + + if ($this->configFile !== null && $file === \realpath((string) $this->configFile)) { + return null; + } + + return Path::create($file); + } + + /** + * @return list Valid definitions of the `` section; invalid ones are reported and skipped. + */ + private function readRegistry(string $name, Path $file): array + { + $xml = (string) @\file_get_contents((string) $file); + + $previous = \libxml_use_internal_errors(true); + try { + $document = \simplexml_load_string($xml); + } finally { + \libxml_clear_errors(); + \libxml_use_internal_errors($previous); + } + + if ($document === false) { + $this->logger->warning('Package `%s`: registry file `%s` is not a valid XML, skipped.', $name, (string) $file); + return []; + } + + if ($document->xpath('/dload/registry') === []) { + $this->logger->warning('Package `%s`: registry file `%s` has no `` section.', $name, (string) $file); + return []; + } + + // A container of its own, without the application logger: a failure to hydrate an attribute + // would be logged with no package name. A required attribute it fails on is reported below. + $registry = new CustomSoftwareRegistry(); + (new ConfigInflector(xml: $xml))->inflect($registry, new ObjectContainer()); + + $result = []; + foreach ($registry->software as $software) { + $problem = self::problemOf($software); + if ($problem === null) { + $result[] = $software; + continue; + } + + $this->logger->warning('Package `%s`: software definition skipped, %s.', $name, $problem); + } + + return $result; + } + + /** + * @return array|null Decoded object, or null when the file is missing; an unreadable file is reported. + */ + private function readJson(Path $file): ?array + { + if (!$file->exists()) { + return null; + } + + try { + /** @var mixed $json */ + $json = \json_decode((string) \file_get_contents((string) $file), true, 512, \JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + $this->logger->warning('Skipping registries of Composer packages: `%s` is not valid JSON: %s', (string) $file, $e->getMessage()); + return null; + } + + if (!\is_array($json)) { + $this->logger->warning('Skipping registries of Composer packages: `%s` is not a JSON object.', (string) $file); + return null; + } + + return $json; + } +} diff --git a/src/Module/Software/Internal/Source/PackageSource.php b/src/Module/Software/Internal/Source/PackageSource.php new file mode 100644 index 00000000..c9f7f072 --- /dev/null +++ b/src/Module/Software/Internal/Source/PackageSource.php @@ -0,0 +1,37 @@ +`. + * + * @internal + * @psalm-internal Internal\DLoad\Module\Software + */ +final class PackageSource implements SoftwareSource +{ + public function __construct( + private readonly CustomSoftwareRegistry $registry, + private readonly ComposerPackages $packages, + ) {} + + public function collect(SoftwareCollection $collection, callable $next): SoftwareCollection + { + if ($this->registry->fromPackages) { + foreach ($this->packages->entries() as $entry) { + $collection = $collection->with($entry->origin, $entry->software); + } + } + + return $next($collection); + } +} diff --git a/src/Module/Software/ShadowReport.php b/src/Module/Software/ShadowReport.php new file mode 100644 index 00000000..a8a6ad4f --- /dev/null +++ b/src/Module/Software/ShadowReport.php @@ -0,0 +1,138 @@ +#` to pick one explicitly. + * + * Both are warnings, and only for entries with other repositories: the same repositories are no clash. + * + * A `dload.xml` entry shadows silently: that is the project's own choice. So does an explicit + * `#`. + * + * ```php + * $report->report($collection, 'gh'); + * ``` + * + * @internal + */ +final class ShadowReport +{ + public function __construct( + private readonly Logger $logger, + ) {} + + /** + * @param non-empty-string $id The id the download was requested with. + */ + public function report(SoftwareCollection $collection, string $id): void + { + $winner = \str_contains($id, '#') ? null : $collection->find($id); + if ($winner === null || $winner->origin->kind === OriginKind::Config) { + return; + } + + $packages = \array_values(\array_filter( + $collection->candidates($id), + static fn(Entry $entry): bool => $entry->origin->kind === OriginKind::Package, + )); + + $clash = \array_values(\array_filter( + $packages, + static fn(Entry $entry): bool => self::sourceKey($entry) !== self::sourceKey($winner), + )); + if ($clash === []) { + return; + } + + if ($winner->origin->kind === OriginKind::Official) { + $this->logger->warning( + "The official registry shadows `%s` declared by %s.\nTo download the package's one, use %s.", + $id, + \implode(', ', \array_map(static fn(Entry $entry): string => (string) $entry->origin, $clash)), + \implode(' or ', \array_map(static fn(Entry $entry): string => self::address($entry), $clash)), + ); + return; + } + + $width = \max(0, ...\array_map(static fn(Entry $entry): int => \strlen(self::address($entry)), $packages)); + $lines = \array_map( + static fn(Entry $entry): string => \sprintf( + ' %s %s', + \str_pad(self::address($entry), $width), + \implode(', ', \array_map( + static fn(Repository $repository): string => $repository->uri, + $entry->software->repositories, + )), + ), + $packages, + ); + + $this->logger->warning( + "Several packages declare `%s`, using %s.\nTo pick one explicitly, use one of:\n%s", + $id, + self::address($winner), + \implode("\n", $lines), + ); + } + + /** + * @return non-empty-string + */ + private static function address(Entry $entry): string + { + return (string) $entry->origin . '#' . $entry->getId(); + } + + /** + * Identifies where an entry downloads from: its set of repositories, order ignored. + * + * Other differences, such as the binary or asset patterns, do not change the source. + */ + private static function sourceKey(Entry $entry): string + { + $keys = \array_map( + static fn(Repository $repository): string => \strtolower($repository->type) . ' ' . self::normalizeUri($repository), + $entry->software->repositories, + ); + $keys = \array_unique($keys); + \sort($keys); + + return \implode("\n", $keys); + } + + /** + * Brings the spellings of one repository to one form: `owner/repo`, `github.com/owner/repo` + * and `https://github.com/owner/repo.git` are the same repository. + */ + private static function normalizeUri(Repository $repository): string + { + $uri = \strtolower(\trim($repository->uri)); + $uri = (string) \preg_replace('~^[a-z][a-z0-9+.-]*://~', '', $uri); + $uri = \rtrim($uri, '/'); + \str_ends_with($uri, '.git') and $uri = \substr($uri, 0, -4); + $uri = (string) \preg_replace('~^www\.~', '', $uri); + + $publicHost = match (\strtolower($repository->type)) { + 'github' => 'github.com/', + 'gitlab' => 'gitlab.com/', + default => null, + }; + + return $publicHost !== null && \str_starts_with($uri, $publicHost) + ? \substr($uri, \strlen($publicHost)) + : $uri; + } +} diff --git a/src/Module/Software/SoftwareCollection.php b/src/Module/Software/SoftwareCollection.php index aaa363b5..7e610d18 100644 --- a/src/Module/Software/SoftwareCollection.php +++ b/src/Module/Software/SoftwareCollection.php @@ -54,15 +54,52 @@ public function with(Origin $origin, Software ...$software): self /** * Finds the entry that wins for the given id. * - * @param non-empty-string $id Software id: alias or lowercased name. + * `#` addresses the entry of a Composer package, whatever shadows it. + * + * @param non-empty-string $id Software id: alias or lowercased name, or `#`. */ public function find(string $id): ?Entry { + if (\str_contains($id, '#')) { + $package = \strtolower((string) \strstr($id, '#', true)); + $id = \substr($id, \strlen($package) + 1); + foreach ($this->entries[$id] ?? [] as $entry) { + if ($entry->origin->package === $package) { + return $entry; + } + } + + return null; + } + return isset($this->entries[$id]) ? self::strongest($this->entries[$id]) : null; } /** - * @param non-empty-string $name Software id: alias or lowercased name. + * Every entry declared for the id, the shadowed ones included. + * + * @param non-empty-string $id Software id: alias or lowercased name. + * @return list In the order they were added. + */ + public function candidates(string $id): array + { + return $this->entries[$id] ?? []; + } + + /** + * Yields the winning entry of every id. + * + * @return \Generator + */ + public function entries(): \Generator + { + foreach ($this->entries as $id => $entries) { + yield $id => self::strongest($entries); + } + } + + /** + * @param non-empty-string $name Software id: alias or lowercased name, or `#`. */ public function findSoftware(string $name): ?Software { @@ -76,8 +113,8 @@ public function findSoftware(string $name): ?Software */ public function getIterator(): \Traversable { - foreach ($this->entries as $id => $entries) { - yield $id => self::strongest($entries)->software; + foreach ($this->entries() as $id => $entry) { + yield $id => $entry->software; } } diff --git a/src/Module/Software/SoftwareCollector.php b/src/Module/Software/SoftwareCollector.php index 80705f50..a9ff22f5 100644 --- a/src/Module/Software/SoftwareCollector.php +++ b/src/Module/Software/SoftwareCollector.php @@ -9,6 +9,7 @@ use Internal\DLoad\Module\Software\Internal\SoftwareSource; use Internal\DLoad\Module\Software\Internal\Source\ConfigSource; use Internal\DLoad\Module\Software\Internal\Source\OfficialSource; +use Internal\DLoad\Module\Software\Internal\Source\PackageSource; /** * Builds the {@see SoftwareCollection} by passing it through every software source. @@ -22,18 +23,21 @@ final class SoftwareCollector /** @var callable(SoftwareCollection): SoftwareCollection */ private $pipeline; - public function __construct(CustomSoftwareRegistry $registry) + /** + * @param ComposerPackages|null $packages Null leaves out the software of Composer packages. + */ + public function __construct(CustomSoftwareRegistry $registry, ?ComposerPackages $packages = null) { + $sources = [new ConfigSource($registry), new OfficialSource()]; + $packages === null or $sources[] = new PackageSource($registry, $packages); + /** * The config source goes first: it is the one that can cut the sources after it. * * @see SoftwareSource::collect() * @var callable(SoftwareCollection): SoftwareCollection $pipeline */ - $pipeline = Pipeline::prepare( - new ConfigSource($registry), - new OfficialSource(), - )->with(static fn(SoftwareCollection $collection): SoftwareCollection => $collection, 'collect'); + $pipeline = Pipeline::prepare(...$sources)->with(static fn(SoftwareCollection $collection): SoftwareCollection => $collection, 'collect'); $this->pipeline = $pipeline; } diff --git a/src/Service/Logger.php b/src/Service/Logger.php index f5c39ffe..b3b44ea1 100644 --- a/src/Service/Logger.php +++ b/src/Service/Logger.php @@ -37,6 +37,20 @@ public function __construct( $this->verbose = $output?->isVerbose() ?? false; } + /** + * Removes the control characters a terminal acts on from text that comes from outside, such as + * a Composer package or a release API: C0 controls but `\t` and `\n`, DEL, and C1 controls + * encoded in UTF-8 (`\xC2\x9B` is a CSI). + * + * ```php + * Logger::printable("\e[2Jtitle"); // "[2Jtitle" + * ``` + */ + public static function printable(string $text): string + { + return (string) \preg_replace('/[\x00-\x08\x0B-\x1F\x7F]|\xC2[\x80-\x9F]/', '', $text); + } + /** * Outputs a message with a newline. * @@ -118,7 +132,7 @@ public function exception(\Throwable $e, ?string $header = null, bool $important $r .= "\033[1;33m" . $header . "\033[0m\n"; } // Print exception message - $r .= $e->getMessage() . "\n"; + $r .= self::printable($e->getMessage()) . "\n"; // Print file and line using green color and italic font $r .= "In \033[3;32m" . $e->getFile() . ':' . $e->getLine() . "\033[0m\n"; // Print stack trace using gray @@ -142,7 +156,7 @@ private static function values(array $values): array foreach ($values as $k => $value) { $result[$k] = match (true) { \is_bool($value) => $value ? 'TRUE' : 'FALSE', - default => (string) $value, + default => self::printable((string) $value), }; } diff --git a/tests/Integration/Command/PackageRegistryCommandsTest.php b/tests/Integration/Command/PackageRegistryCommandsTest.php new file mode 100644 index 00000000..bd648390 --- /dev/null +++ b/tests/Integration/Command/PackageRegistryCommandsTest.php @@ -0,0 +1,364 @@ + 'Evil', + 'description' => 'click', + 'homepage' => 'home', + 'uri' => 'uri', + ]; + + private string $directory; + private string $cwd; + + /** `DLOAD_CACHE_DIR` before the test, false when it was not set */ + private string|false $cacheDir; + + #[Test] + #[Covers(ListSoftware::class)] + public function theSoftwareListShowsOriginsAndShadowedEntries(): void + { + $this->config(''); + + $tester = $this->run(['command' => 'software']); + + Assert::same($tester->getStatusCode(), Command::SUCCESS); + Assert::string(self::display($tester)) + ->contains("trap Trap [dload.xml]\nShadows: official, acme/fw#trap\n") + ->contains("acme-tool Acme Tool [acme/fw]\nShadows: x/b#acme-tool\n") + ->contains("rr RoadRunner [official]\n"); + } + + #[Test] + #[Covers(Show::class)] + public function aConfiguredPackageIdIsListedAsConfigured(): void + { + $bin = $this->directory . \DIRECTORY_SEPARATOR . 'bin'; + \mkdir($bin); + \touch($bin . \DIRECTORY_SEPARATOR . self::executable('acme-trap')); + $this->config(\sprintf('', $bin)); + + $tester = $this->run(['command' => 'show']); + + Assert::string(self::display($tester)) + ->contains("Configured software:\n acme/fw#trap (unknown) ") + ->notContains('No configured software found'); + } + + #[Test] + #[Covers(Show::class)] + public function aConfiguredShadowedEntryDoesNotHideTheWinnerOfItsId(): void + { + $bin = $this->directory . \DIRECTORY_SEPARATOR . 'bin'; + \mkdir($bin); + \touch($bin . \DIRECTORY_SEPARATOR . self::executable('xb-tool')); + \touch($this->directory . \DIRECTORY_SEPARATOR . self::executable('local-tool')); + $this->config(\sprintf( + '' + . '', + $bin, + )); + \chdir($this->directory); + + $tester = $this->run(['command' => 'show']); + + Assert::string(self::display($tester)) + ->contains("Configured software:\n x/b#acme-tool (unknown) ") + ->contains("Downloaded software (not configured):\n acme-tool (unknown) "); + } + + #[Test] + #[Covers(Show::class)] + public function unconfiguredPackageSoftwareIsNotRunToListIt(): void + { + $this->copyPhpTo('php-tool'); + \touch($this->directory . \DIRECTORY_SEPARATOR . self::executable('acme-tool')); + $this->config(''); + \chdir($this->directory); + + $tester = $this->run(['command' => 'show']); + + Assert::string(self::display($tester)) + ->notContains(' php-tool (') + ->notContains(' acme-tool (') + ->contains("Downloaded software (not configured):\n No additional downloaded software found"); + } + + #[Test] + #[Covers(Show::class)] + public function theVersionOfConfiguredPackageSoftwareIsShown(): void + { + $this->copyPhpTo('php-tool'); + $this->config(\sprintf('', $this->directory)); + + $tester = $this->run(['command' => 'show']); + + Assert::string(self::display($tester))->contains(\sprintf("Configured software:\n acme/fw#php-tool (%s) ", \PHP_VERSION)); + } + + #[Test] + #[Covers(Show::class)] + #[DataSet(['php-tool'], 'the bare id')] + #[DataSet(['acme/fw#php-tool'], 'the package id')] + public function theVersionOfRequestedPackageSoftwareIsShown(string $id): void + { + $this->copyPhpTo('php-tool'); + $this->config(''); + \chdir($this->directory); + + $tester = $this->run(['command' => 'show', 'software' => $id]); + + Assert::string(self::display($tester))->contains(\sprintf("Version: %s\n", \PHP_VERSION)); + } + + #[Test] + #[Covers(Show::class)] + #[DataSet([[]], 'the list')] + #[DataSet([['--all' => true]], 'the full list')] + #[DataSet([['software' => 'pwn']], 'a shell injection by id')] + #[DataSet([['software' => 'x/b#php-pwn']], 'a PHP option injection by package id')] + public function aPackageVersionCommandThatIsNotOneOptionIsRejected(array $input): void + { + $this->copyPhpTo('pwn-tool'); + $this->copyPhpTo('pwn2-tool'); + $this->config(''); + \chdir($this->directory); + + $tester = $this->run(['command' => 'show'] + $input); + + Assert::false(\file_exists($this->directory . '/pwned.txt')); + Assert::false(\file_exists($this->directory . '/PWNED.txt')); + Assert::string(self::display($tester)) + ->contains('the version command `--version & echo PWNED>pwned.txt` of `pwn` must be one option like `--version`') + ->contains("the version command `-r file_put_contents('PWNED.txt','x');` of `php-pwn` must be one option") + ->notContains(' pwn ') + ->notContains(' php-pwn '); + } + + #[Test] + #[Covers(Show::class)] + #[DataSet(['acme/fw#trap', 'acme/fw', 'Not included in project config'], 'the shadowed package entry')] + #[DataSet(['trap', 'official', 'Included in project config'], 'the official entry')] + public function showTellsTheSourceAndWhetherThatEntryIsConfigured(string $id, string $source, string $config): void + { + $this->config(''); + + $tester = $this->run(['command' => 'show', 'software' => $id]); + + Assert::same($tester->getStatusCode(), Command::SUCCESS); + Assert::string(self::display($tester)) + ->contains("Source: $source\n") + ->contains("Project configuration: $config\n"); + } + + #[Test] + #[Covers(ListSoftware::class)] + #[Covers(Show::class)] + #[DataSet([['command' => 'software'], ['name', 'description', 'homepage', 'uri']], 'the software list')] + #[DataSet([['command' => 'show', '--all' => true], ['description']], 'the full show list')] + #[DataSet([['command' => 'show', 'software' => 'evil'], ['name', 'description', 'homepage']], 'the details')] + public function registryTextIsPrintedLiterally(array $input, array $fields): void + { + $this->config(''); + + $plain = self::display($this->run($input)); + $decorated = self::display($this->run($input, decorated: true)); + + foreach ($fields as $field) { + Assert::string($plain)->contains(self::EVIL_TEXT[$field]); + Assert::string($decorated)->contains(self::EVIL_TEXT[$field]); + } + Assert::string($decorated)->notContains("\e]8;;"); + } + + #[Test] + #[Covers(ListSoftware::class)] + #[Covers(Show::class)] + #[DataSet([['command' => 'software'], ['C12J', 'before31mafter', 'https://c1.example/31m', 'bit31mbucket: a/b31m']], 'the software list')] + #[DataSet([['command' => 'show', '--all' => true], ['before31mafter']], 'the full show list')] + #[DataSet([['command' => 'show', 'software' => 'c1'], ['C12J', 'before31mafter', 'https://c1.example/31m']], 'the details')] + public function controlCharactersOfRegistryTextAreNotPrinted(array $input, array $texts): void + { + $this->config(''); + + $display = self::display($this->run($input, decorated: true)); + + foreach ($texts as $text) { + Assert::string($display)->contains($text); + } + Assert::string($display)->notContains("\u{9B}"); + } + + #[Test] + #[Covers(Get::class)] + #[DataSet([['c1'], 'bit31mbucket'], 'a repository type of a package')] + #[DataSet([["ev\e[2Jil\u{9B}31m"], 'ev[2Jil31m'], 'a requested id')] + public function theFailureReportOfGetHasNoControlCharacters(array $software, string $text): void + { + $this->config(''); + + $display = self::display($this->run(['command' => 'get', 'software' => $software, '--path' => $this->directory . '/bin'])); + + Assert::string($display) + ->contains('Failed to download') + ->contains($text) + ->notContains("\u{9B}") + ->notContains("\e[2J"); + } + + #[BeforeTest] + protected function prepare(): void + { + $this->cwd = (string) \getcwd(); + $this->directory = \sys_get_temp_dir() . \DIRECTORY_SEPARATOR . 'dload-package-registry-' . \bin2hex(\random_bytes(6)); + \mkdir($this->directory . '/vendor/composer', recursive: true); + \mkdir($this->directory . '/vendor/acme/fw', recursive: true); + \mkdir($this->directory . '/vendor/x/b', recursive: true); + \file_put_contents($this->directory . '/composer.json', (string) \json_encode(['require' => ['acme/fw' => '*', 'x/b' => '*']])); + \file_put_contents($this->directory . '/vendor/composer/installed.json', (string) \json_encode(['packages' => [ + ['name' => 'acme/fw', 'extra' => ['dload' => ['registry' => 'dload.xml']], 'install-path' => '../acme/fw'], + ['name' => 'x/b', 'extra' => ['dload' => ['registry' => 'dload.xml']], 'install-path' => '../x/b'], + ]])); + \file_put_contents($this->directory . '/vendor/acme/fw/dload.xml', <<<'XML' + + + + + + + + + + + + + + + + + + XML); + \file_put_contents($this->directory . '/vendor/x/b/dload.xml', <<<'XML' + + + + + + + + + + + + + + + + + + + + + + + + + + XML); + $this->cacheDir = \getenv('DLOAD_CACHE_DIR'); + \putenv('DLOAD_CACHE_DIR=' . $this->directory . '/cache'); + } + + #[AfterTest] + protected function cleanup(): void + { + \putenv($this->cacheDir === false ? 'DLOAD_CACHE_DIR' : 'DLOAD_CACHE_DIR=' . $this->cacheDir); + \chdir($this->cwd); + \is_dir($this->directory) and FS::removeDir(Path::create($this->directory)); + } + + /** + * @return string The command output with `\n` line endings: Console writes `PHP_EOL`. + */ + private static function display(ApplicationTester $tester): string + { + return \str_replace(\PHP_EOL, "\n", $tester->getDisplay()); + } + + private static function executable(string $name): string + { + return $name . (\PHP_OS_FAMILY === 'Windows' ? '.exe' : ''); + } + + /** + * Puts a working copy of PHP into the project under the binary name. + */ + private function copyPhpTo(string $binaryName): void + { + $file = $this->directory . \DIRECTORY_SEPARATOR . self::executable($binaryName); + @\copy(\PHP_BINARY, $file) or throw new SkipTest('Cannot copy the PHP binary.'); + \chmod($file, 0o755); + } + + private function config(string $content): void + { + \file_put_contents($this->directory . '/dload.xml', '' . $content . ''); + } + + /** + * @param array $input + */ + private function run(array $input, bool $decorated = false): ApplicationTester + { + $application = new Application(); + $application->setAutoExit(false); + $application->setCatchExceptions(false); + foreach ([new Get(), new ListSoftware(), new Show()] as $command) { + // Symfony Console 8 renamed `add()` to `addCommand()` + \method_exists($application, 'addCommand') + ? $application->addCommand($command) + : $application->add($command); + } + + $tester = new ApplicationTester($application); + $tester->run( + $input + ['--config' => $this->directory . '/dload.xml'], + ['interactive' => false, 'decorated' => $decorated], + ); + + return $tester; + } +} diff --git a/tests/Integration/Module/Software/ComposerPackagesBindingTest.php b/tests/Integration/Module/Software/ComposerPackagesBindingTest.php new file mode 100644 index 00000000..9882861f --- /dev/null +++ b/tests/Integration/Module/Software/ComposerPackagesBindingTest.php @@ -0,0 +1,144 @@ +project('elsewhere', 'acme/cwd'); + $this->project('project/sub', 'acme/app'); + \file_put_contents($this->directory . '/project/sub/dload.xml', ''); + \chdir($this->directory . '/elsewhere'); + + $collection = self::collection($this->directory . '/project/sub/dload.xml'); + + Assert::same(self::origins($collection, 'tool'), ['acme/app']); + } + + #[Test] + public function withoutAConfigFileTheProjectIsTheWorkingDirectory(): void + { + $this->project('elsewhere', 'acme/cwd'); + \chdir($this->directory . '/elsewhere'); + + Assert::same(self::origins(self::collection(null), 'tool'), ['acme/cwd']); + Assert::same(self::origins(self::collection(''), 'tool'), ['acme/cwd']); + } + + #[Test] + public function theRootRegistryThatIsTheConfigFileIsAddedOnce(): void + { + \mkdir($this->directory . '/project'); + \file_put_contents($this->directory . '/project/composer.json', (string) \json_encode([ + 'name' => 'acme/app', + 'extra' => ['dload' => ['registry' => 'dload.xml']], + ])); + \file_put_contents( + $this->directory . '/project/dload.xml', + '', + ); + \chdir($this->directory . '/project'); + + $collection = self::collection('./dload.xml'); + + Assert::same(self::origins($collection, 'tool'), ['dload.xml']); + } + + #[Test] + public function composerVendorDirOverridesTheVendorDirectory(): void + { + \mkdir($this->directory . '/project/libs/composer', recursive: true); + \mkdir($this->directory . '/project/libs/acme/lib', recursive: true); + \file_put_contents($this->directory . '/project/composer.json', (string) \json_encode(['require' => ['acme/lib' => '*']])); + \file_put_contents($this->directory . '/project/libs/composer/installed.json', (string) \json_encode(['packages' => [[ + 'name' => 'acme/lib', + 'extra' => ['dload' => ['registry' => 'dload.xml']], + 'install-path' => '../acme/lib', + ]]])); + \file_put_contents( + $this->directory . '/project/libs/acme/lib/dload.xml', + '', + ); + \file_put_contents($this->directory . '/project/dload.xml', ''); + + $collection = self::collection($this->directory . '/project/dload.xml', ['COMPOSER_VENDOR_DIR' => 'libs']); + + Assert::same(self::origins($collection, 'tool'), ['acme/lib']); + } + + #[BeforeTest] + protected function prepare(): void + { + $this->cwd = (string) \getcwd(); + $this->directory = \sys_get_temp_dir() . \DIRECTORY_SEPARATOR . 'dload-packages-binding-' . \bin2hex(\random_bytes(6)); + \mkdir($this->directory, recursive: true); + } + + #[AfterTest] + protected function cleanup(): void + { + \chdir($this->cwd); + \is_dir($this->directory) and FS::removeDir(Path::create($this->directory)); + } + + /** + * @param array $environment + */ + private static function collection(?string $xml, array $environment = []): SoftwareCollection + { + return Bootstrap::init() + ->withConfig(xml: $xml, environment: $environment) + ->finish() + ->get(SoftwareCollection::class); + } + + /** + * @param non-empty-string $id + * @return list Origin of every entry of the id. + */ + private static function origins(SoftwareCollection $collection, string $id): array + { + return \array_map(static fn(Entry $entry): string => (string) $entry->origin, $collection->candidates($id)); + } + + /** + * Writes a root package that declares `tool` in its own registry file. + */ + private function project(string $dir, string $name): void + { + $path = $this->directory . '/' . $dir; + \mkdir($path, recursive: true); + \file_put_contents($path . '/composer.json', (string) \json_encode([ + 'name' => $name, + 'extra' => ['dload' => ['registry' => 'registry.xml']], + ])); + \file_put_contents( + $path . '/registry.xml', + '', + ); + } +} diff --git a/tests/Unit/Module/Binary/Internal/AbstractBinaryTest.php b/tests/Unit/Module/Binary/Internal/AbstractBinaryTest.php index 1ddec3b1..e46be00d 100644 --- a/tests/Unit/Module/Binary/Internal/AbstractBinaryTest.php +++ b/tests/Unit/Module/Binary/Internal/AbstractBinaryTest.php @@ -7,12 +7,16 @@ use Internal\DLoad\Module\Binary\BinaryVersion; use Internal\DLoad\Module\Binary\Internal\AbstractBinary; use Internal\DLoad\Module\Binary\Internal\BinaryExecutor; +use Internal\DLoad\Module\Binary\Internal\LocalBinary; +use Internal\DLoad\Module\Common\FileSystem\FS; use Internal\DLoad\Module\Config\Schema\Embed\Binary as BinaryConfig; use Internal\DLoad\Service\Logger; use Internal\Path; use Testo\Assert; use Testo\Codecov\Covers; +use Testo\Core\Exception\SkipTest; use Testo\Data\DataProvider; +use Testo\Data\DataSet; use Testo\Test; #[Covers(AbstractBinary::class)] @@ -53,4 +57,60 @@ public function getPath(): Path Assert::same($binary->getVersionString(), $expected); } + + #[Test] + #[DataSet([['build', '-c', '{dir}/velox config.toml', '-o', '{dir}/out dir']], 'paths with spaces')] + #[DataSet([["it's", 'a&b', '*']], 'shell syntax')] + public function executePassesEachArgumentToTheBinaryOnce(array $arguments): void + { + $dir = self::directory('dload binary '); + $arguments = \array_map(static fn(string $argument): string => \str_replace('{dir}', $dir, $argument), $arguments); + $script = $dir . \DIRECTORY_SEPARATOR . 'argv.php'; + \file_put_contents($script, 'execute($script, ...$arguments); + } finally { + FS::remove(Path::create($dir)); + } + + Assert::same(\json_decode(\implode("\n", $output), true), $arguments); + } + + #[Test] + #[DataSet(['version --short', '1.2.3'], 'two arguments')] + #[DataSet([" version \t --short ", '1.2.3'], 'extra whitespace')] + #[DataSet(['version', '9.9.9'], 'one argument')] + public function getVersionSplitsTheVersionCommandIntoArguments(string $command, string $version): void + { + $dir = self::directory('dload-binary-'); + $script = $dir . \DIRECTORY_SEPARATOR . 'version.php'; + \file_put_contents( + $script, + ' 'php', 'version-command' => "$script $command"]); + + try { + $result = self::php($config)->getVersionString(); + } finally { + FS::remove(Path::create($dir)); + } + + Assert::same($result, $version); + } + + private static function php(BinaryConfig $config): LocalBinary + { + return new LocalBinary('php', $config, new BinaryExecutor(new Logger()), Path::create(\PHP_BINARY)); + } + + private static function directory(string $prefix): string + { + $dir = \sys_get_temp_dir() . \DIRECTORY_SEPARATOR . $prefix . \bin2hex(\random_bytes(6)); + \mkdir($dir); + + return $dir; + } } diff --git a/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php b/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php new file mode 100644 index 00000000..aff48421 --- /dev/null +++ b/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php @@ -0,0 +1,73 @@ +{marker}']], 'a command separator')] + #[DataSet([['--version & echo X>{marker}']], 'a command separator in one argument')] + #[DataSet([['--version | tee {marker}']], 'a pipe')] + #[DataSet([['$(echo X>{marker})']], 'a command substitution')] + #[DataSet([['`echo X>{marker}`']], 'backticks')] + #[DataSet([['>{marker}']], 'a redirect')] + #[DataSet([['^&']], 'a cmd escape')] + #[DataSet([['%PATH%']], 'a cmd variable')] + #[DataSet([['!PATH!']], 'a delayed expansion variable')] + #[DataSet([['$HOME']], 'a shell variable')] + #[DataSet([['*']], 'a glob')] + #[DataSet([['~']], 'a home directory')] + public function everyArgumentReachesTheBinaryVerbatim(array $arguments): void + { + $marker = (string) $this->dir->join('marker'); + $arguments = \array_map(static fn(string $argument): string => \str_replace('{marker}', $marker, $argument), $arguments); + + $output = (new BinaryExecutor(new Logger()))->execute(Path::create(\PHP_BINARY), $this->script, ...$arguments); + + Assert::same(\json_decode(\implode("\n", $output), true), $arguments); + Assert::false(\file_exists($marker)); + } + + #[BeforeTest] + protected function prepare(): void + { + $this->dir = Path::create(\sys_get_temp_dir())->join('dload executor ' . \bin2hex(\random_bytes(6))); + FS::mkdir($this->dir); + $this->script = (string) $this->dir->join('argv.php'); + \file_put_contents($this->script, 'dir); + } +} diff --git a/tests/Unit/Module/Binary/Internal/BinaryProviderImplTest.php b/tests/Unit/Module/Binary/Internal/BinaryProviderImplTest.php new file mode 100644 index 00000000..05b9db78 --- /dev/null +++ b/tests/Unit/Module/Binary/Internal/BinaryProviderImplTest.php @@ -0,0 +1,87 @@ +getLocalBinary($this->dir->join('bin'), BinaryConfig::fromArray(['name' => 'tool'])); + + Assert::same((string) $binary?->getPath(), (string) $this->dir->join('bin', 'tool')); + } + + #[DataSet(['../tool'], 'a parent traversal')] + #[DataSet(['..\\tool'], 'a Windows parent traversal')] + #[DataSet(['{absolute}'], 'an absolute path')] + public function aNameThatIsAPathFindsNoLocalBinary(string $name): void + { + $name === '{absolute}' and $name = (string) $this->dir->join('tool'); + + $binary = self::provider()->getLocalBinary($this->dir->join('bin'), BinaryConfig::fromArray(['name' => $name])); + + Assert::null($binary); + } + + #[DataSet(['{php}'], 'an absolute path')] + #[DataSet(['../php'], 'a parent traversal')] + #[DataSet(['bin/php'], 'a subdirectory')] + public function aLookedUpNameThatIsAPathFindsNoGlobalBinary(string $name): void + { + $name === '{php}' and $name = \PHP_BINARY; + + $binary = self::provider()->getGlobalBinary(BinaryConfig::fromArray(['name' => 'php']), $name); + + Assert::null($binary); + } + + public function aNameThatIsAPathFindsNoGlobalBinary(): void + { + $binary = self::provider()->getGlobalBinary(BinaryConfig::fromArray(['name' => \PHP_BINARY])); + + Assert::null($binary); + } + + #[BeforeTest] + protected function prepare(): void + { + $this->dir = Path::create(\sys_get_temp_dir())->join('dload-binary-provider-' . \bin2hex(\random_bytes(6))); + FS::mkdir($this->dir->join('bin')); + \touch((string) $this->dir->join('tool')); + \touch((string) $this->dir->join('bin', 'tool')); + } + + #[AfterTest] + protected function cleanup(): void + { + FS::remove($this->dir); + } + + private static function provider(): BinaryProviderImpl + { + return new BinaryProviderImpl(OperatingSystem::Linux, new BinaryExecutor(new Logger())); + } +} diff --git a/tests/Unit/Module/Common/FileSystem/FSTest.php b/tests/Unit/Module/Common/FileSystem/FSTest.php new file mode 100644 index 00000000..851777f4 --- /dev/null +++ b/tests/Unit/Module/Common/FileSystem/FSTest.php @@ -0,0 +1,84 @@ +tempDir . '/roadrunner-3.4.0-linux-amd64.tar.gz')); } + #[Test] + #[DataSet(['../rr-3.4.0-linux-amd64.tar.gz', '.._rr-3.4.0-linux-amd64.tar.gz'], 'a parent traversal')] + #[DataSet(['sub/rr-3.4.0-linux-amd64.tar.gz', 'sub_rr-3.4.0-linux-amd64.tar.gz'], 'a subdirectory')] + #[DataSet(['sub\\rr-3.4.0-linux-amd64.tar.gz', 'sub_rr-3.4.0-linux-amd64.tar.gz'], 'a Windows subdirectory')] + #[DataSet(['C:rr-3.4.0-linux-amd64.tar.gz', 'C_rr-3.4.0-linux-amd64.tar.gz'], 'a drive')] + #[DataSet(["rr\x1B-3.4.0-linux-amd64.tar.gz", 'rr_-3.4.0-linux-amd64.tar.gz'], 'a control character')] + #[DataSet(['rr-3.4.0-linux-amd64.tar.gz. ', 'rr-3.4.0-linux-amd64.tar.gz'], 'a trailing dot and space')] + #[DataSet(['aux.linux-amd64.tar.gz', '_aux.linux-amd64.tar.gz'], 'a reserved device name')] + public function anAssetIsDownloadedUnderAFileNameInsideTheTempDirectory(string $name, string $file): void + { + $repository = self::repoWithReleases(['v3.4.0' => [$name]]); + + $result = $this->run([$repository], self::software()); + + Assert::same($result->version->string, 'v3.4.0'); + Assert::same((string) Path::create($result->file->getPath()), (string) Path::create($this->tempDir)); + Assert::same($result->file->getFilename(), $file); + Assert::same(\array_values(\array_diff((array) \scandir($this->tempDir), ['.', '..'])), [$file]); + Assert::false(\file_exists(\dirname($this->tempDir) . '/rr-3.4.0-linux-amd64.tar.gz')); + } + #[DataProvider('provideGradualStrategies')] #[Test] public function gradualFilteringSelectsAssetByEachStrategy(?OperatingSystem $os, ?Architecture $arch): void diff --git a/tests/Unit/Module/Installer/InstallerTest.php b/tests/Unit/Module/Installer/InstallerTest.php index d126edff..51ab8302 100644 --- a/tests/Unit/Module/Installer/InstallerTest.php +++ b/tests/Unit/Module/Installer/InstallerTest.php @@ -26,7 +26,9 @@ use Symfony\Component\Console\Output\OutputInterface; use Testo\Assert; use Testo\Codecov\Covers; +use Testo\Core\Exception\SkipTest; use Testo\Data\DataProvider; +use Testo\Data\DataSet; use Testo\Expect; use Testo\Lifecycle\AfterTest; use Testo\Lifecycle\BeforeTest; @@ -234,6 +236,70 @@ public function downloadIsKeptWhenItIsNotToBeRemoved(): void Assert::true($this->destination->join('app')->isFile()); } + #[Test] + #[DataSet(['a.b', ['pkg/axb' => 'wrong', 'pkg/a.b' => 'right']], 'a dot')] + #[DataSet(['c++', ['pkg/ccc' => 'wrong', 'pkg/c++' => 'right']], 'a quantifier')] + #[DataSet(['t(1)', ['pkg/t1' => 'wrong', 'pkg/t(1)' => 'right']], 'a group')] + public function theBinaryNameMatchesOnlyItself(string $name, array $entries): void + { + $download = $this->download('tool.stub', ''); + $software = Software::fromArray(['name' => 'Tool', 'binary' => ['name' => $name]]); + + $result = $this->installer(archiveFactory: self::listed($entries)) + ->install($download, $software, null, $this->destination, removeDownload: true); + + Assert::same( + \array_map(static fn(Path $file): string => (string) \file_get_contents((string) $file), [...$result->files]), + ['right'], + ); + } + + #[Test] + #[DataSet(['a.b', 'pkg/a.b', 'a.b'], 'a dotted name')] + #[DataSet(['A.B', 'pkg/a.b', 'A.B'], 'a dotted name in another case')] + #[DataSet(['a.b', 'pkg/a.b.exe', 'a.b.exe'], 'a dotted name with an executable extension')] + #[DataSet(['rr', 'pkg/roadrunner.exe', 'rr.exe'], 'a renamed executable')] + public function theBinaryKeepsItsNameAndGainsOnlyAMissingExtension(string $name, string $entry, string $installed): void + { + $download = $this->download('tool.stub', ''); + $software = Software::fromArray(['name' => 'Tool', 'binary' => ['name' => $name, 'pattern' => '/./']]); + + $result = $this->installer(archiveFactory: self::listed([$entry => 'binary'])) + ->install($download, $software, null, $this->destination, removeDownload: true); + + Assert::same(self::paths($result->files), [(string) $this->destination->join($installed)]); + } + + #[Test] + #[DataSet([['files' => [['pattern' => '/^app$/', 'rename' => '../escaped']]]], 'a rename above the destination')] + #[DataSet([['files' => [['pattern' => '/^app$/', 'rename' => 'sub/app']]]], 'a rename into a subdirectory')] + #[DataSet([['files' => [['pattern' => '/^app$/', 'rename' => 'C:app']]]], 'a drive-relative rename')] + #[DataSet([['binary' => ['name' => '../escaped', 'pattern' => '/^app$/']]], 'a binary name above the destination')] + #[DataSet([['files' => [['pattern' => '/^app$/', 'rename' => 'nul']]]], 'a rename to a Windows device')] + public function aTargetNameThatIsAPathFailsTheInstallation(array $rules): never + { + $download = $this->download('tool.stub', ''); + $software = Software::fromArray(['name' => 'Tool'] + $rules); + + Expect::exception(\RuntimeException::class)->withMessageContaining('the name must be a file name, not a path'); + + $this->installer(archiveFactory: self::listed(['pkg/app' => 'evil'])) + ->install($download, $software, null, $this->destination, removeDownload: true); + } + + #[Test] + public function anEntryKeepsItsOwnNameWithoutARename(): void + { + \PHP_OS_FAMILY === 'Windows' and throw new SkipTest('A file name cannot contain `:` on Windows.'); + $download = $this->download('tool.stub', ''); + $software = Software::fromArray(['name' => 'Tool', 'files' => [['pattern' => '/^foo:bar$/']]]); + + $result = $this->installer(archiveFactory: self::listed(['pkg/foo:bar' => 'content'])) + ->install($download, $software, null, $this->destination, removeDownload: true); + + Assert::same(self::paths($result->files), [(string) $this->destination->join('foo:bar')]); + } + #[BeforeTest] protected function prepare(): void { @@ -263,6 +329,21 @@ private static function paths(iterable $files): array return $result; } + /** + * @param array $entries Content by entry path. + * @return ArchiveFactory Opens any `.stub` file as an archive of the entries. + */ + private static function listed(array $entries): ArchiveFactory + { + $archiveFactory = new ArchiveFactory(); + $archiveFactory->extend( + static fn(\SplFileInfo $file): ?ListedArchive => $file->getExtension() === 'stub' ? new ListedArchive($entries) : null, + ['stub'], + ); + + return $archiveFactory; + } + private function installer( ?BinaryProvider $binaryProvider = null, ArchiveFactory $archiveFactory = new ArchiveFactory(), diff --git a/tests/Unit/Module/Software/ComposerPackagesTest.php b/tests/Unit/Module/Software/ComposerPackagesTest.php new file mode 100644 index 00000000..0c9ccab9 --- /dev/null +++ b/tests/Unit/Module/Software/ComposerPackagesTest.php @@ -0,0 +1,567 @@ +dir = \sys_get_temp_dir() . \DIRECTORY_SEPARATOR . 'dload-composer-packages-' . \bin2hex(\random_bytes(6)); + \mkdir($this->dir, 0777, true); + $this->output = new BufferedOutput(); + } + + #[AfterTest] + public function removeProject(): void + { + self::remove($this->dir); + } + + public function readsTheRootPackageAndDirectDependenciesInPrecedenceOrder(): void + { + $this->root([ + 'name' => 'acme/app', + 'extra' => ['dload' => ['registry' => 'dload-registry.xml']], + 'require' => ['php' => '^8.1', 'b/second' => '*', 'a/first' => '*'], + 'require-dev' => ['c/dev' => '*'], + ]); + $this->file('dload-registry.xml', self::registry('')); + $this->installed( + $this->package('c/dev', self::registry('')), + $this->package('a/first', self::registry('')), + $this->package('t/transitive', self::registry('')), + $this->package('b/second', self::registry('')), + ); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), [ + 'acme/app#root-tool', + 'b/second#second-tool', + 'b/second#other', + 'a/first#first-tool', + 'c/dev#dev-tool', + ]); + Assert::same($this->output->fetch(), ''); + } + + public function aRootPackageWithoutNameIsNamedRoot(): void + { + $this->root(['extra' => ['dload' => ['registry' => 'registry.xml']]]); + $this->file('registry.xml', self::registry('')); + + Assert::same(self::describe($this->entries()), ['__root__#tool']); + } + + public function requireMatchesInstalledPackagesWhateverTheCase(): void + { + $this->root(['require' => ['Acme/Lib' => '*']]); + $this->installed($this->package('acme/lib', self::registry(''))); + + Assert::same(self::describe($this->entries()), ['acme/lib#tool']); + } + + public function honoursTheVendorDirOfTheRootPackage(): void + { + $this->root(['require' => ['acme/lib' => '*'], 'config' => ['vendor-dir' => 'libs']]); + $this->installedIn('libs', $this->package('acme/lib', self::registry(''), vendorDir: 'libs')); + + Assert::same(self::describe($this->entries()), ['acme/lib#tool']); + } + + public function readsOnlyTheRegistrySectionOfTheFile(): void + { + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed($this->package('acme/lib', <<<'XML' + + + + + + + + + + + + + XML)); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/lib#tool']); + $software = $entries[0]->software; + Assert::same($software->description, 'A tool'); + Assert::same($software->repositories[0]->uri, 'acme/tool'); + Assert::same($software->repositories[0]->assetPattern, '/^tool-.*/'); + Assert::same($software->binary?->name, 'tool'); + } + + public function packagesWithoutTheDeclarationAreIgnored(): void + { + $this->root(['require' => ['acme/lib' => '*', 'acme/plain' => '*', 'acme/missing' => '*']]); + $this->installed( + $this->package('acme/plain', null), + $this->package('acme/lib', self::registry('')), + ); + + Assert::same(self::describe($this->entries()), ['acme/lib#tool']); + Assert::same($this->output->fetch(), ''); + } + + public function noComposerJsonMeansNoEntries(): void + { + Assert::same($this->entries(), []); + Assert::same($this->output->fetch(), ''); + } + + public function withoutInstalledPackagesOnlyTheRootRegistryIsRead(): void + { + $this->root([ + 'name' => 'acme/app', + 'extra' => ['dload' => ['registry' => 'registry.xml']], + 'require' => ['acme/lib' => '*'], + ]); + $this->file('registry.xml', self::registry('')); + + Assert::same(self::describe($this->entries()), ['acme/app#tool']); + Assert::same($this->output->fetch(), ''); + } + + public function theRootRegistryThatIsTheLoadedConfigIsNotReadAgain(): void + { + $this->root([ + 'name' => 'acme/app', + 'extra' => ['dload' => ['registry' => 'dload.xml']], + 'require' => ['acme/lib' => '*'], + ]); + $this->file('dload.xml', self::registry('')); + $this->installed($this->package('acme/lib', self::registry(''))); + + $entries = $this->entries(configFile: 'dload.xml'); + + Assert::same(self::describe($entries), ['acme/lib#tool']); + Assert::same($this->output->fetch(), ''); + } + + public function theRootRegistryIsReadWhenAnotherConfigIsLoaded(): void + { + $this->root(['name' => 'acme/app', 'extra' => ['dload' => ['registry' => 'dload.xml']]]); + $this->file('dload.xml', self::registry('')); + $this->file('dload.local.xml', self::registry()); + + Assert::same(self::describe($this->entries(configFile: 'dload.local.xml')), ['acme/app#tool']); + } + + #[DataSet(['missing.xml', 'registry file `missing.xml` not found'], 'a missing file')] + #[DataSet(['../../outside.xml', 'registry file `../../outside.xml` is outside the package'], 'a path above the package')] + #[DataSet(['{outside}', 'is outside the package'], 'an absolute path')] + #[DataSet(['broken.xml', 'is not a valid XML'], 'an invalid XML')] + #[DataSet([['registry.xml'], '`extra.dload.registry` must be a path to a file'], 'not a string')] + #[DataSet(['', '`extra.dload.registry` must be a path to a file'], 'an empty string')] + #[DataSet(["dload.xml\0", '`extra.dload.registry` must be a path to a file'], 'a null byte')] + #[DataSet(['no-registry.xml', 'has no `` section'], 'an XML without a registry section')] + #[DataSet(['foreign.xml', 'has no `` section'], 'an XML of another format')] + public function rejectsABrokenDeclarationAndKeepsTheOtherPackages(mixed $registry, string $warning): void + { + $this->file('vendor/outside.xml', self::registry('')); + $registry === '{outside}' and $registry = $this->dir . \DIRECTORY_SEPARATOR . \implode(\DIRECTORY_SEPARATOR, ['vendor', 'outside.xml']); + $this->root(['require' => ['acme/broken' => '*', 'acme/lib' => '*']]); + $this->installed( + $this->package('acme/broken', null, ['dload' => ['registry' => $registry]]), + $this->package('acme/lib', self::registry('')), + ); + $this->file('vendor/acme/broken/broken.xml', ''); + $this->file('vendor/acme/broken/no-registry.xml', ''); + $this->file('vendor/acme/broken/foreign.xml', ''); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/lib#tool']); + Assert::string($this->output->fetch())->contains('Package `acme/broken`: ')->contains($warning); + } + + public function rejectsASymlinkLeadingOutOfThePackage(): void + { + $this->file('outside.xml', self::registry('')); + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed($this->package('acme/lib', null, ['dload' => ['registry' => 'link.xml']])); + $link = $this->dir . '/vendor/acme/lib/link.xml'; + if (!@\symlink($this->dir . \DIRECTORY_SEPARATOR . 'outside.xml', $link)) { + throw new SkipTest('Symlinks cannot be created here.'); + } + + $entries = $this->entries(); + + Assert::same($entries, []); + Assert::string($this->output->fetch())->contains('registry file `link.xml` is outside the package'); + } + + public function acceptsAPackageInstalledThroughASymlink(): void + { + $this->file('packages/lib/dload.xml', self::registry('')); + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed(['name' => 'acme/lib', 'extra' => ['dload' => ['registry' => 'dload.xml']], 'install-path' => '../acme/lib']); + \mkdir($this->dir . '/vendor/acme'); + if (!@\symlink($this->dir . \DIRECTORY_SEPARATOR . 'packages' . \DIRECTORY_SEPARATOR . 'lib', $this->dir . '/vendor/acme/lib')) { + throw new SkipTest('Symlinks cannot be created here.'); + } + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/lib#tool']); + Assert::same($this->output->fetch(), ''); + } + + #[DataSet(['','`` has no `name`'], 'no name')] + #[DataSet(['', 'a `` of `tool` has no `uri`'], 'a repository without uri')] + #[DataSet(['', 'the id `a#b` cannot be requested'], 'an alias with #')] + #[DataSet(['', 'the id `a#b` cannot be requested'], 'a name with #')] + #[DataSet(['', 'a `` of `tool` has no `name`'], 'a binary without name')] + #[DataSet(['', '`` has no `name`'], 'an empty name')] + #[DataSet(['', '`` has no `name`'], 'a blank name')] + #[DataSet(['', '`` has an empty id'], 'an empty alias')] + #[DataSet(['', '`` has an empty id'], 'a blank alias')] + #[DataSet(['', 'a `` of `tool` has no `uri`'], 'an empty uri')] + #[DataSet(['', 'a `` of `tool` has no `name`'], 'a blank binary name')] + #[DataSet(['', 'the id `a:b` cannot be requested'], 'an alias with :')] + #[DataSet(['', 'the id ` tool` cannot be requested'], 'an alias with a leading space')] + #[DataSet(['', 'the id `tool ` cannot be requested'], 'a name with a trailing space')] + #[DataSet(['', '`../x` of `tool` must be a file name, not a path'], 'a binary name above the destination')] + #[DataSet(['', '`bin\\x` of `tool` must be a file name, not a path'], 'a binary name with a backslash')] + #[DataSet(['', '`a/b` of `tool` must be a file name, not a path'], 'a file rename with a slash')] + #[DataSet(['', '`..` of `tool` must be a file name, not a path'], 'a file rename to the parent')] + #[DataSet(['', 'the pattern `/^tool(/` of `tool` is not a valid regular expression'], 'an invalid binary pattern')] + #[DataSet(['', 'the pattern `not a regex` of `tool` is not a valid regular expression'], 'an invalid asset pattern')] + #[DataSet(['', 'the pattern `/[/` of `tool` is not a valid regular expression'], 'an invalid file pattern')] + public function rejectsAnInvalidDefinitionAndKeepsTheValidOnes(string $invalid, string $warning): void + { + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed($this->package('acme/lib', self::registry($invalid, ''))); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/lib#valid']); + Assert::string($this->output->fetch()) + ->contains('Package `acme/lib`: software definition skipped, ') + ->contains($warning); + } + + #[DataSet(['composer.json', '{"name": '], 'an invalid root composer.json')] + #[DataSet(['vendor/composer/installed.json', '{"packages": ['], 'an invalid installed.json')] + public function rejectsInvalidJson(string $file, string $content): void + { + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed($this->package('acme/lib', self::registry(''))); + $this->file($file, $content); + + $entries = $this->entries(); + + Assert::same($entries, []); + Assert::string($this->output->fetch())->contains('is not valid JSON'); + } + + public function acceptsADefinitionThatOmitsOptionalAttributes(): void + { + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed($this->package('acme/lib', self::registry( + '', + ))); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/lib#tool']); + Assert::same($this->output->fetch(), ''); + } + + #[DataSet(['--version'], 'a long option')] + #[DataSet(['--short-version'], 'a long option with a dash')] + #[DataSet(['-v'], 'a short option')] + #[DataSet(['-V'], 'an upper-case short option')] + #[DataSet(['version'], 'a subcommand')] + public function acceptsAVersionCommandOfOneOption(string $command): void + { + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed($this->package('acme/lib', self::registry( + \sprintf('', $command), + ))); + + $entries = $this->entries(); + + Assert::same($entries[0]->software->binary?->versionCommand, $command); + Assert::same($this->output->fetch(), ''); + } + + #[DataSet(['version --short'], 'two arguments')] + #[DataSet(['-r file_put_contents(1,2);'], 'PHP code')] + #[DataSet(['-r1'], 'an option with a value')] + #[DataSet(['--version=1'], 'an option with an assigned value')] + #[DataSet(['---version'], 'three dashes')] + #[DataSet(['--'], 'dashes only')] + #[DataSet(['-'], 'one dash')] + #[DataSet(['self-update'], 'a subcommand that changes the binary')] + #[DataSet(['reset'], 'a subcommand that changes state')] + #[DataSet(['serve'], 'a subcommand that keeps running')] + #[DataSet(['Version'], 'the version subcommand in another case')] + #[DataSet(['versions'], 'a longer subcommand')] + #[DataSet(['v2'], 'a digit')] + #[DataSet(['/?'], 'a Windows switch')] + #[DataSet(['--version&id'], 'a shell operator')] + #[DataSet(['$(id)'], 'a command substitution')] + #[DataSet([''], 'empty')] + public function rejectsAVersionCommandOfMoreThanOneOption(string $command): void + { + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed($this->package('acme/lib', self::registry( + \sprintf('', \htmlspecialchars($command)), + '', + ))); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/lib#valid']); + Assert::string($this->output->fetch()) + ->contains(\sprintf('the version command `%s` of `tool` must be one option like `--version`', $command)); + } + + #[DataSet(['acme/lib'], 'a string')] + #[DataSet([true], 'a boolean')] + #[DataSet([42], 'a number')] + public function rejectsAnExtraDloadThatIsNotAnObject(mixed $dload): void + { + $this->root(['require' => ['acme/broken' => '*', 'acme/lib' => '*']]); + $this->installed( + $this->package('acme/broken', null, ['dload' => $dload]), + $this->package('acme/lib', self::registry('')), + ); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/lib#tool']); + Assert::string($this->output->fetch())->contains('Package `acme/broken`: `extra.dload` must be an object'); + } + + public function skipsAPackageWithANullByteInItsInstallPath(): void + { + $this->root(['require' => ['acme/broken' => '*', 'acme/lib' => '*']]); + $broken = $this->package('acme/broken', self::registry('')); + $broken['install-path'] = "../acme/broken\0/../broken"; + $this->installed($broken, $this->package('acme/lib', self::registry(''))); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/lib#tool']); + } + + #[DataSet(['composer.json', '"acme/app"'], 'a root composer.json string')] + #[DataSet(['composer.json', 'null'], 'a root composer.json null')] + #[DataSet(['vendor/composer/installed.json', '"packages"'], 'an installed.json string')] + #[DataSet(['vendor/composer/installed.json', '42'], 'an installed.json number')] + public function rejectsJsonThatIsNotAnObject(string $file, string $content): void + { + $this->root(['require' => ['acme/lib' => '*']]); + $this->installed($this->package('acme/lib', self::registry(''))); + $this->file($file, $content); + + $entries = $this->entries(); + + Assert::same($entries, []); + Assert::string($this->output->fetch())->contains('is not a JSON object'); + } + + #[DataSet([['libs']], 'a list')] + #[DataSet([['dir' => 'libs']], 'an object')] + #[DataSet([42], 'a number')] + #[DataSet([''], 'an empty string')] + #[DataSet(["libs\0"], 'a null byte')] + public function rejectsAVendorDirThatIsNotAPath(mixed $vendorDir): void + { + $this->root([ + 'name' => 'acme/app', + 'extra' => ['dload' => ['registry' => 'registry.xml']], + 'require' => ['acme/lib' => '*'], + 'config' => ['vendor-dir' => $vendorDir], + ]); + $this->file('registry.xml', self::registry('')); + $this->installed($this->package('acme/lib', self::registry(''))); + $phpErrors = []; + \set_error_handler(static function (int $level, string $message) use (&$phpErrors): bool { + $phpErrors[] = $message; + return true; + }); + + try { + $entries = $this->entries(); + } finally { + \restore_error_handler(); + } + + Assert::same(self::describe($entries), ['acme/app#root-tool']); + Assert::same($phpErrors, []); + Assert::string($this->output->fetch())->contains('`config.vendor-dir` is not a path'); + } + + #[DataSet(['libs', 'libs'], 'the override')] + #[DataSet(['', 'other'], 'the config when the override is empty')] + #[DataSet([null, 'other'], 'the config without an override')] + public function theVendorDirOverrideWinsOverTheConfig(?string $override, string $used): void + { + $this->root(['require' => ['acme/lib' => '*'], 'config' => ['vendor-dir' => 'other']]); + $this->installedIn('libs', $this->package('acme/lib', self::registry(''), vendorDir: 'libs')); + $this->installedIn('other', $this->package('acme/lib', self::registry(''), vendorDir: 'other')); + + $entries = $this->entries(vendorDir: $override); + + Assert::same(self::describe($entries), ["acme/lib#from-$used"]); + } + + public function theOverrideIsUsedWhenTheConfigVendorDirIsBroken(): void + { + $this->root(['require' => ['acme/lib' => '*'], 'config' => ['vendor-dir' => ['broken']]]); + $this->installedIn('libs', $this->package('acme/lib', self::registry(''), vendorDir: 'libs')); + + $entries = $this->entries(vendorDir: 'libs'); + + Assert::same(self::describe($entries), ['acme/lib#tool']); + Assert::same($this->output->fetch(), ''); + } + + public function aPackageInBothRequireAndRequireDevKeepsItsRequirePlace(): void + { + $this->root([ + 'require' => ['b/both' => '*', 'a/other' => '*'], + 'require-dev' => ['c/dev' => '*', 'B/Both' => '*'], + ]); + $this->installed( + $this->package('a/other', self::registry('')), + $this->package('b/both', self::registry('')), + $this->package('c/dev', self::registry('')), + ); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['b/both#both', 'a/other#other', 'c/dev#dev']); + } + + public function rejectsTheComposer1FormatOfInstalledJson(): void + { + $this->root(['name' => 'acme/app', 'extra' => ['dload' => ['registry' => 'registry.xml']], 'require' => ['acme/lib' => '*']]); + $this->file('registry.xml', self::registry('')); + $package = $this->package('acme/lib', self::registry('')); + $this->file('vendor/composer/installed.json', (string) \json_encode([$package])); + + $entries = $this->entries(); + + Assert::same(self::describe($entries), ['acme/app#root-tool']); + Assert::string($this->output->fetch())->contains('is not in the Composer 2 format'); + } + + private static function registry(string ...$software): string + { + return '' . \implode('', $software) . ''; + } + + /** + * @param list $entries + * @return list `#` of each entry. + */ + private static function describe(array $entries): array + { + return \array_map(static fn(Entry $entry): string => $entry->origin . '#' . $entry->getId(), $entries); + } + + /** + * Not {@see \Internal\DLoad\Module\Common\FileSystem\FS::removeDir()}: it descends into a symlinked directory, and the symlink tests make one. + */ + private static function remove(string $path): void + { + if (\is_link($path) || \is_file($path)) { + @\unlink($path) or @\rmdir($path); + return; + } + + if (!\is_dir($path)) { + return; + } + + foreach (\scandir($path) ?: [] as $item) { + $item === '.' || $item === '..' or self::remove($path . \DIRECTORY_SEPARATOR . $item); + } + + @\rmdir($path); + } + + /** + * @param non-empty-string|null $configFile Relative to the project. + * @return list + */ + private function entries(?string $configFile = null, ?string $vendorDir = null): array + { + $packages = new ComposerPackages( + Path::create($this->dir), + $configFile === null ? null : Path::create($this->dir)->join($configFile), + new Logger($this->output), + $vendorDir, + ); + + return $packages->entries(); + } + + private function root(array $composer): void + { + $this->file('composer.json', (string) \json_encode($composer)); + } + + /** + * Writes the package into the vendor directory. + * + * @param string|null $registry Content of `dload.xml` of the package, declared in `extra` unless `$extra` is given. + * @return array The package's record for `installed.json`. + */ + private function package(string $name, ?string $registry, ?array $extra = null, string $vendorDir = 'vendor'): array + { + \mkdir($this->dir . "/$vendorDir/$name", 0777, true); + $registry === null or $this->file("$vendorDir/$name/dload.xml", $registry); + $extra ??= $registry === null ? [] : ['dload' => ['registry' => 'dload.xml']]; + + return ['name' => $name, 'version' => '1.0.0', 'extra' => $extra, 'install-path' => "../$name"]; + } + + private function installed(array ...$packages): void + { + $this->installedIn('vendor', ...$packages); + } + + private function installedIn(string $vendorDir, array ...$packages): void + { + $this->file("$vendorDir/composer/installed.json", (string) \json_encode(['packages' => $packages, 'dev' => true])); + } + + private function file(string $path, string $content): void + { + $full = $this->dir . '/' . $path; + \is_dir(\dirname($full)) or \mkdir(\dirname($full), 0777, true); + \file_put_contents($full, $content); + } +} diff --git a/tests/Unit/Module/Software/ShadowReportTest.php b/tests/Unit/Module/Software/ShadowReportTest.php new file mode 100644 index 00000000..0d3231c3 --- /dev/null +++ b/tests/Unit/Module/Software/ShadowReportTest.php @@ -0,0 +1,195 @@ +output = new BufferedOutput(); + } + + public function warnsWhenTheOfficialRegistryShadowsPackageEntries(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::official(), self::software('gh', 'cli/cli')) + ->with(Origin::package('acme/framework'), self::software('gh', 'acme/gh')) + ->with(Origin::package('vendor/cms'), self::software('gh', 'vendor/gh')); + + $this->report($collection, 'gh'); + + Assert::string($this->output->fetch()) + ->contains('The official registry shadows `gh` declared by acme/framework, vendor/cms.') + ->contains('acme/framework#gh or vendor/cms#gh'); + } + + #[DataSet(['cli/cli'], 'the same spelling')] + #[DataSet(['https://github.com/CLI/cli.git'], 'another spelling')] + public function staysSilentWhenTheOfficialEntryPointsToTheSameRepositories(string $uri): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::official(), self::software('gh', 'cli/cli')) + ->with(Origin::package('acme/framework'), self::software('gh', $uri)); + + $this->report($collection, 'gh'); + + Assert::same($this->output->fetch(), ''); + } + + public function namesOnlyThePackagesThatDifferFromTheOfficialEntry(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::official(), self::software('gh', 'cli/cli')) + ->with(Origin::package('acme/framework'), self::software('gh', 'github.com/cli/cli')) + ->with(Origin::package('vendor/cms'), self::software('gh', 'vendor/gh')); + + $this->report($collection, 'gh'); + + Assert::string($this->output->fetch()) + ->contains('The official registry shadows `gh` declared by vendor/cms.') + ->contains('use vendor/cms#gh.') + ->notContains('acme/framework'); + } + + public function listsEveryPackageWhenPackagesDeclareDifferentRepositories(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::package('acme/framework'), self::software('gh', 'cli/cli')) + ->with(Origin::package('vendor/cms'), self::software('gh', 'vendor/gh-fork')); + + $this->report($collection, 'gh'); + + Assert::string($this->output->fetch()) + ->contains('Several packages declare `gh`, using acme/framework#gh.') + ->contains(" acme/framework#gh cli/cli\n") + ->contains(' vendor/cms#gh vendor/gh-fork'); + } + + #[DataSet([['github' => 'owner/repo'], ['github' => 'https://github.com/Owner/Repo.git']], 'https URL with .git')] + #[DataSet([['github' => 'owner/repo'], ['github' => 'github.com/owner/repo']], 'host without scheme')] + #[DataSet([['github' => 'owner/repo'], ['github' => 'https://www.github.com/owner/repo/']], 'www and trailing slash')] + #[DataSet([['gitlab' => 'group/repo'], ['gitlab' => 'https://gitlab.com/group/repo']], 'public GitLab host')] + #[DataSet([['github' => 'a/one', 'gitlab' => 'b/two'], ['gitlab' => 'b/two', 'github' => 'a/one']], 'order of repositories')] + #[DataSet([['github' => 'owner/repo'], ['GitHub' => 'owner/repo']], 'case of the type')] + public function staysSilentWhenPackagesPointToTheSameRepositories(array $first, array $second): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::package('acme/framework'), self::software('gh', ...self::repositories($first))) + ->with(Origin::package('vendor/cms'), self::software('gh', ...self::repositories($second))); + + $this->report($collection, 'gh'); + + Assert::same($this->output->fetch(), ''); + } + + #[DataSet([['github' => 'owner/repo'], ['gitlab' => 'owner/repo']], 'another type')] + #[DataSet([['github' => 'owner/repo'], ['github' => 'owner/other']], 'another repository')] + #[DataSet([['github' => 'owner/repo'], ['github' => 'owner/repo', 'gitlab' => 'owner/repo']], 'an extra repository')] + #[DataSet([['gitlab' => 'gitlab.example.com/owner/repo'], ['gitlab' => 'owner/repo']], 'a self-hosted host')] + public function warnsWhenPackagesPointToDifferentRepositories(array $first, array $second): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::package('acme/framework'), self::software('gh', ...self::repositories($first))) + ->with(Origin::package('vendor/cms'), self::software('gh', ...self::repositories($second))); + + $this->report($collection, 'gh'); + + Assert::string($this->output->fetch())->contains('Several packages declare `gh`'); + } + + public function aConfigEntryShadowsSilently(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::config(), self::software('gh', 'local/gh')) + ->with(Origin::official(), self::software('gh', 'cli/cli')) + ->with(Origin::package('acme/framework'), self::software('gh', 'acme/gh')) + ->with(Origin::package('vendor/cms'), self::software('gh', 'vendor/gh')); + + $this->report($collection, 'gh'); + + Assert::same($this->output->fetch(), ''); + } + + public function anExplicitPackageIdIsSilent(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::official(), self::software('gh', 'cli/cli')) + ->with(Origin::package('acme/framework'), self::software('gh', 'acme/gh')) + ->with(Origin::package('vendor/cms'), self::software('gh', 'vendor/gh')); + + $this->report($collection, 'acme/framework#gh'); + + Assert::same($this->output->fetch(), ''); + } + + #[DataSet(['gh'], 'a single package entry')] + #[DataSet(['rr'], 'an official entry without package rivals')] + #[DataSet(['unknown'], 'an unknown id')] + public function staysSilentWithoutShadowedPackageEntries(string $id): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::official(), self::software('rr', 'roadrunner-server/roadrunner')) + ->with(Origin::package('acme/framework'), self::software('gh', 'cli/cli')); + + $this->report($collection, $id); + + Assert::same($this->output->fetch(), ''); + } + + /** + * @param array $repositories Uri by type. + * @return list + */ + private static function repositories(array $repositories): array + { + $result = []; + foreach ($repositories as $type => $uri) { + $result[] = ['type' => $type, 'uri' => $uri]; + } + + return $result; + } + + /** + * @param non-empty-string $id + * @param non-empty-string|array{type: non-empty-string, uri: non-empty-string} ...$repositories + * A bare string is a GitHub repository. + */ + private static function software(string $id, string|array ...$repositories): Software + { + return Software::fromArray([ + 'name' => $id, + 'repositories' => \array_map( + static fn(string|array $repository): array => \is_string($repository) + ? ['type' => 'github', 'uri' => $repository] + : $repository, + $repositories, + ), + ]); + } + + /** + * @param non-empty-string $id + */ + private function report(SoftwareCollection $collection, string $id): void + { + (new ShadowReport(new Logger($this->output)))->report($collection, $id); + } +} diff --git a/tests/Unit/Module/Software/SoftwareCollectionTest.php b/tests/Unit/Module/Software/SoftwareCollectionTest.php index 87c1c5c1..25891f70 100644 --- a/tests/Unit/Module/Software/SoftwareCollectionTest.php +++ b/tests/Unit/Module/Software/SoftwareCollectionTest.php @@ -5,10 +5,12 @@ namespace Internal\DLoad\Tests\Unit\Module\Software; use Internal\DLoad\Module\Config\Schema\Embed\Software; +use Internal\DLoad\Module\Software\Entry; use Internal\DLoad\Module\Software\Origin; use Internal\DLoad\Module\Software\SoftwareCollection; use Testo\Assert; use Testo\Codecov\Covers; +use Testo\Data\DataSet; use Testo\Test; #[Covers(SoftwareCollection::class)] @@ -69,6 +71,82 @@ public function addingDoesNotChangeTheOriginalCollection(): void Assert::same(\count($empty), 0); } + #[Test] + #[DataSet(['acme/framework#gh', 'framework'], 'a shadowed package entry')] + #[DataSet(['ACME/Framework#gh', 'framework'], 'the package name in another case')] + #[DataSet(['vendor/cms#gh', 'cms'], 'the weakest package entry')] + public function aPackageIdAddressesThePackageEntryWhateverShadowsIt(string $id, string $description): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::config(), self::software('gh', 'local')) + ->with(Origin::official(), self::software('gh', 'official')) + ->with(Origin::package('acme/framework'), self::software('gh', 'framework')) + ->with(Origin::package('vendor/cms'), self::software('gh', 'cms')); + + $entry = $collection->find($id); + + Assert::same($entry?->software->description, $description); + Assert::same($collection->findSoftware($id)?->description, $description); + } + + #[Test] + #[DataSet(['other/package#gh'], 'another package')] + #[DataSet(['acme/framework#rr'], 'an id the package does not declare')] + #[DataSet(['dload.xml#rr'], 'a config origin')] + #[DataSet(['official#rr'], 'the official origin')] + #[DataSet(['#gh'], 'no package name')] + public function aPackageIdFindsNothingOutsideThatPackage(string $id): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::config(), self::software('rr', 'local')) + ->with(Origin::official(), self::software('rr', 'official')) + ->with(Origin::package('acme/framework'), self::software('gh', 'framework')); + + Assert::null($collection->find($id)); + } + + #[Test] + public function aFirstAddedPackageWinsOverLaterPackages(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::package('acme/framework'), self::software('gh', 'framework')) + ->with(Origin::package('vendor/cms'), self::software('gh', 'cms')); + + Assert::same($collection->findSoftware('gh')?->description, 'framework'); + } + + #[Test] + public function candidatesListEveryEntryOfTheIdInTheOrderOfAdding(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::package('acme/framework'), self::software('gh', 'framework')) + ->with(Origin::official(), self::software('gh', 'official'), self::software('rr', 'rr')) + ->with(Origin::config(), self::software('gh', 'local')); + + $candidates = $collection->candidates('gh'); + + Assert::same( + \array_map(static fn(Entry $entry): string => (string) $entry->origin, $candidates), + ['acme/framework', 'official', 'dload.xml'], + ); + Assert::same($collection->candidates('unknown'), []); + } + + #[Test] + public function entriesYieldTheWinningEntryOfEveryId(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::package('acme/framework'), self::software('gh', 'framework'), self::software('tool', 'tool')) + ->with(Origin::official(), self::software('gh', 'official')); + + $entries = \iterator_to_array($collection->entries()); + + Assert::same( + \array_map(static fn(Entry $entry): string => $entry->origin . ' ' . $entry->software->description, $entries), + ['gh' => 'official official', 'tool' => 'acme/framework tool'], + ); + } + /** * @param non-empty-string $id */ diff --git a/tests/Unit/Module/Software/SoftwareCollectorTest.php b/tests/Unit/Module/Software/SoftwareCollectorTest.php index 1ead1e59..b55725c6 100644 --- a/tests/Unit/Module/Software/SoftwareCollectorTest.php +++ b/tests/Unit/Module/Software/SoftwareCollectorTest.php @@ -6,15 +6,53 @@ use Internal\DLoad\Module\Config\Schema\CustomSoftwareRegistry; use Internal\DLoad\Module\Config\Schema\Embed\Software; +use Internal\DLoad\Module\Software\ComposerPackages; use Internal\DLoad\Module\Software\OriginKind; use Internal\DLoad\Module\Software\SoftwareCollector; +use Internal\DLoad\Service\Logger; +use Internal\Path; use Testo\Assert; use Testo\Codecov\Covers; +use Testo\Data\DataSet; +use Testo\Lifecycle\AfterTest; +use Testo\Lifecycle\BeforeTest; use Testo\Test; #[Covers(SoftwareCollector::class)] final class SoftwareCollectorTest { + private string $dir; + + #[BeforeTest] + public function createProject(): void + { + $this->dir = \sys_get_temp_dir() . \DIRECTORY_SEPARATOR . 'dload-collector-' . \bin2hex(\random_bytes(6)); + \mkdir($this->dir . '/vendor/acme/lib', 0777, true); + \mkdir($this->dir . '/vendor/composer', 0777, true); + \file_put_contents($this->dir . '/composer.json', (string) \json_encode(['require' => ['acme/lib' => '*']])); + \file_put_contents($this->dir . '/vendor/composer/installed.json', (string) \json_encode(['packages' => [[ + 'name' => 'acme/lib', + 'extra' => ['dload' => ['registry' => 'dload.xml']], + 'install-path' => '../acme/lib', + ]]])); + \file_put_contents( + $this->dir . '/vendor/acme/lib/dload.xml', + '', + ); + } + + #[AfterTest] + public function removeProject(): void + { + foreach (['vendor/acme/lib/dload.xml', 'vendor/composer/installed.json', 'composer.json'] as $file) { + @\unlink($this->dir . '/' . $file); + } + + foreach (['vendor/acme/lib', 'vendor/acme', 'vendor/composer', 'vendor', ''] as $dir) { + @\rmdir($this->dir . '/' . $dir); + } + } + #[Test] public function configEntriesShadowTheOfficialOnes(): void { @@ -37,6 +75,42 @@ public function overwriteDropsTheOfficialRegistry(): void Assert::same(\iterator_to_array($collection), ['tool' => $local]); } + #[Test] + public function packageEntriesComeAfterTheConfigAndTheOfficialOnes(): void + { + $local = Software::fromArray(['name' => 'Local']); + + $collection = (new SoftwareCollector(self::registry(false, $local), $this->packages()))->collect(); + + Assert::same((string) $collection->find('acme-tool')?->origin, 'acme/lib'); + Assert::same($collection->find('rr')?->origin->kind, OriginKind::Official); + Assert::same($collection->find('local')?->software, $local); + Assert::same((string) $collection->find('acme/lib#rr')?->origin, 'acme/lib'); + Assert::same((string) $collection->find('acme/lib#local')?->origin, 'acme/lib'); + } + + #[Test] + #[DataSet([false, false], 'from-packages="false"')] + #[DataSet([true, true], 'overwrite="true"')] + public function theRegistryCanSwitchPackageEntriesOff(bool $overwrite, bool $fromPackages): void + { + $registry = self::registry($overwrite); + $registry->fromPackages = $fromPackages; + + $collection = (new SoftwareCollector($registry, $this->packages()))->collect(); + + Assert::null($collection->find('acme-tool')); + Assert::null($collection->find('acme/lib#rr')); + } + + #[Test] + public function withoutComposerPackagesThereAreNoPackageEntries(): void + { + $collection = (new SoftwareCollector(self::registry(false)))->collect(); + + Assert::null($collection->find('acme-tool')); + } + private static function registry(bool $overwrite, Software ...$software): CustomSoftwareRegistry { $registry = new CustomSoftwareRegistry(); @@ -45,4 +119,9 @@ private static function registry(bool $overwrite, Software ...$software): Custom return $registry; } + + private function packages(): ComposerPackages + { + return new ComposerPackages(Path::create($this->dir), null, new Logger()); + } } diff --git a/tests/Unit/Service/LoggerTest.php b/tests/Unit/Service/LoggerTest.php new file mode 100644 index 00000000..a6b694ed --- /dev/null +++ b/tests/Unit/Service/LoggerTest.php @@ -0,0 +1,77 @@ +warning('Value: %s.', $value); + + Assert::same($output->fetch(), "\033[33mValue: $printed.\033[0m\n"); + } + + #[DataSet(["\e[2Jtitle", '[2Jtitle'], 'an escape sequence')] + #[DataSet(["\u{9B}2Jtitle", '2Jtitle'], 'a C1 CSI in UTF-8')] + #[DataSet(["\u{9D}8;;https://evil.example\u{9C}link", '8;;https://evil.examplelink'], 'a C1 OSC and ST in UTF-8')] + #[DataSet(["\u{85}next", 'next'], 'a C1 next line in UTF-8')] + #[DataSet(["tab\tline\n", "tab\tline\n"], 'a tab and a line feed')] + #[DataSet(["a\rb\x00c\x7Fd", 'abcd'], 'carriage return, null and delete')] + #[DataSet(['Größe: 10 €, ¡olé!', 'Größe: 10 €, ¡olé!'], 'non-ASCII text')] + #[DataSet(['привет 你好 ✓', 'привет 你好 ✓'], 'other scripts')] + public function printableRemovesWhatATerminalActsOn(string $text, string $printable): void + { + Assert::same(Logger::printable($text), $printable); + } + + public function anExceptionMessageIsPrintedWithoutControlCharacters(): void + { + $output = new BufferedOutput(); + + (new Logger($output))->exception(new \RuntimeException("Asset \e]8;;https://evil.example\e\\x\u{9B}31m failed"), important: true); + + Assert::string($output->fetch()) + ->contains("Asset ]8;;https://evil.example\\x31m failed\n") + ->notContains("\e]") + ->notContains("\u{9B}"); + } + + public function everyMessageKindCleansItsValues(): void + { + $output = new BufferedOutput(BufferedOutput::VERBOSITY_VERY_VERBOSE); + $logger = new Logger($output); + + $logger->info('%s', "\e[2Jinfo"); + $logger->error('%s', "\e[2Jerror"); + $logger->debug('%s', "\e[2Jdebug"); + $logger->status('sender', '%s', "\e[2Jstatus"); + + Assert::string($output->fetch()) + ->notContains("\e[2J") + ->contains('[2Jinfo') + ->contains('[2Jerror') + ->contains('[2Jdebug') + ->contains('[2Jstatus'); + } +} From 78b7bef3c06a2d28d01459f186698c0fab82cfcf Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 12 Oct 2026 19:28:37 +0400 Subject: [PATCH 2/5] fix(binary): report a binary that cannot start without a PHP warning fix(software): find a `dload.xml` alias containing `#` by its exact id docs: say the official registry warns only when it points to other repositories, and document `__root__#` On Windows proc_open() with an argument array warns when CreateProcess fails, for a truncated binary or one built for another OS; `bin/dload` has no error handler, so the warning reached the console. Assisted-By: Claude Opus 5.5 --- README-es.md | 4 +-- README-ru.md | 4 +-- README-zh.md | 4 +-- README.md | 4 +-- .../references/package-registry.md | 2 +- src/Module/Binary/Internal/BinaryExecutor.php | 19 ++++++++----- src/Module/Software/SoftwareCollection.php | 23 +++++++++------- .../Binary/Internal/BinaryExecutorTest.php | 27 +++++++++++++++++++ .../Software/SoftwareCollectionTest.php | 11 ++++++++ 9 files changed, 73 insertions(+), 25 deletions(-) diff --git a/README-es.md b/README-es.md index ba16ceb9..88ffdc80 100644 --- a/README-es.md +++ b/README-es.md @@ -700,8 +700,8 @@ Funciona igual con el paquete de Composer, el PHAR y el binario. - **Dónde puede estar el archivo**: solo dentro del paquete. Una ruta que sale de él, mediante `..`, una ruta absoluta o un enlace simbólico, se rechaza. Los archivos remotos no están soportados. - **Prioridad**: `dload.xml` > el registro oficial > los paquetes. Un paquete nunca reemplaza una entrada de los otros dos; - cuando el registro oficial oculta una entrada de un paquete que se está descargando, DLoad avisa de ello. -- **Elección explícita**: `#` selecciona la entrada de un paquete, aunque otra la oculte: + cuando el registro oficial oculta una entrada de un paquete que se está descargando y apunta a otros repositorios, DLoad avisa de ello. +- **Elección explícita**: `#` selecciona la entrada de un paquete, aunque otra la oculte (`__root__#` para un paquete raíz sin `name`): `dload get acme/framework#gh:^2.0` o ``. - **Varios paquetes, un mismo id**: si los paquetes apuntan a los mismos repositorios, coinciden y no hay nada que avisar. Si no, gana el paquete raíz, luego los paquetes en el orden de `require` y `require-dev`, diff --git a/README-ru.md b/README-ru.md index 87e5f925..fb3831d5 100644 --- a/README-ru.md +++ b/README-ru.md @@ -702,8 +702,8 @@ DLoad ищет `composer.json` рядом с загруженным `dload.xml` - **Где может лежать файл**: только внутри пакета. Путь, ведущий за его пределы через `..`, абсолютный путь или символическую ссылку, отклоняется. Удалённые файлы не поддерживаются. - **Приоритет**: `dload.xml` > официальный реестр > пакеты. Пакет никогда не заменяет запись из двух других источников; - если официальный реестр перекрывает загружаемую запись пакета, DLoad предупреждает об этом. -- **Явный выбор**: `#` выбирает запись пакета, что бы её ни перекрывало: + если официальный реестр перекрывает загружаемую запись пакета и указывает на другие репозитории, DLoad предупреждает об этом. +- **Явный выбор**: `#` выбирает запись пакета, что бы её ни перекрывало (`__root__#` — для корневого пакета без `name`): `dload get acme/framework#gh:^2.0` или ``. - **Несколько пакетов, один id**: если пакеты указывают на одни и те же репозитории, они согласованы и сообщать не о чем. Иначе побеждает корневой пакет, затем пакеты в порядке `require` и `require-dev`, diff --git a/README-zh.md b/README-zh.md index 2904b85a..2af76856 100644 --- a/README-zh.md +++ b/README-zh.md @@ -687,8 +687,8 @@ DLoad 在已加载的 `dload.xml` 旁边查找 `composer.json`（没有 `dload.x - **谁可以声明**：根包及其直接依赖（`require`、`require-dev`）。传递依赖不能添加软件。 - **文件可以放在哪里**：只能在包内部。通过 `..`、绝对路径或符号链接指向包外的路径会被拒绝。不支持远程文件。 - **优先级**：`dload.xml` > 官方注册表 > 包。包永远不会替换另外两者中的条目； - 当官方注册表遮蔽了正在下载的某个包条目时，DLoad 会发出警告。 -- **显式选择**：`#` 选择某个包的条目，无论它被什么遮蔽： + 当官方注册表遮蔽了正在下载的某个包条目且指向不同的仓库时，DLoad 会发出警告。 +- **显式选择**：`#` 选择某个包的条目，无论它被什么遮蔽（没有 `name` 的根包使用 `__root__#`）： `dload get acme/framework#gh:^2.0` 或 ``。 - **多个包使用同一个 id**：指向相同仓库的包彼此一致，无需报告。 否则根包优先，然后按 `require` 和 `require-dev` 中的顺序选择包， diff --git a/README.md b/README.md index 095e9431..c9be00c7 100644 --- a/README.md +++ b/README.md @@ -705,8 +705,8 @@ It works the same for the Composer package, the PHAR and the binary. - **Where the file can be**: only inside the package. A path that leads out of it, through `..`, an absolute path or a symlink, is rejected. Remote files are not supported. - **Priority**: `dload.xml` > the official registry > packages. A package never replaces an entry of the other two; - when the official registry shadows a package entry that is being downloaded, DLoad warns about it. -- **Explicit choice**: `#` picks the entry of a package, whatever shadows it: + when the official registry shadows a package entry that is being downloaded and points to other repositories, DLoad warns about it. +- **Explicit choice**: `#` picks the entry of a package, whatever shadows it (`__root__#` for a root package without a `name`): `dload get acme/framework#gh:^2.0` or ``. - **Several packages, one id**: packages pointing to the same repositories agree, nothing to report. Otherwise the root package wins, then the packages in the order of `require` and `require-dev`, diff --git a/skills/dload-fetch-tool/references/package-registry.md b/skills/dload-fetch-tool/references/package-registry.md index d3c0a78d..7d595e90 100644 --- a/skills/dload-fetch-tool/references/package-registry.md +++ b/skills/dload-fetch-tool/references/package-registry.md @@ -35,7 +35,7 @@ DLoad reads `composer.json` next to the loaded `dload.xml` (the working director | Situation | What DLoad does on download | |---|---| | `dload.xml` declares the same id | Uses it, silently. | -| The official registry declares the same id | Uses the official one and warns, naming `#` to get the package's one. | +| The official registry declares the same id | Uses the official one; when the repositories differ, warns and names `#` to get the package's one. | | Several packages declare the id with the same repositories | Uses the first, silently. | | Several packages declare the id with different repositories | Uses the first and lists every `#` to choose from. | diff --git a/src/Module/Binary/Internal/BinaryExecutor.php b/src/Module/Binary/Internal/BinaryExecutor.php index d813a09e..1706038b 100644 --- a/src/Module/Binary/Internal/BinaryExecutor.php +++ b/src/Module/Binary/Internal/BinaryExecutor.php @@ -30,19 +30,24 @@ public function __construct( */ public function execute(Path $binaryPath, string ...$arguments): array { - $command = \implode(' ', $arguments); - $this->logger->debug('Executing command: %s %s', (string) $binaryPath, $command); + // JSON keeps the boundaries of arguments with spaces visible + $command = (string) \json_encode($arguments, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE); + $this->logger->debug('Executing %s with arguments %s', (string) $binaryPath, $command); // An array command starts the binary without a shell: escapeshellarg() on Windows would replace // `"`, `%` and `!` in arguments with spaces. The closed stdin keeps an interactive binary from waiting. - $process = \proc_open( + // On Windows a binary that cannot start (a truncated file, an ELF) makes proc_open() warn and return false. + $process = @\proc_open( [(string) $binaryPath, ...$arguments], [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['redirect', 1]], $pipes, ); - $process === false and throw new BinaryExecutionException( - \sprintf('Failed to execute binary "%s" with command "%s".', $binaryPath, $command), - ); + $process === false and throw new BinaryExecutionException(\sprintf( + 'Failed to execute binary "%s" with arguments %s: %s', + $binaryPath, + $command, + \error_get_last()['message'] ?? 'the process did not start.', + )); \fclose($pipes[0]); $stdout = (string) \stream_get_contents($pipes[1]); @@ -57,7 +62,7 @@ public function execute(Path $binaryPath, string ...$arguments): array if ($returnCode !== 0) { throw new BinaryExecutionException( \sprintf( - 'Failed to execute binary "%s" with command "%s". Exit code: %d. Output: %s', + 'Failed to execute binary "%s" with arguments %s. Exit code: %d. Output: %s', $binaryPath, $command, $returnCode, diff --git a/src/Module/Software/SoftwareCollection.php b/src/Module/Software/SoftwareCollection.php index 7e610d18..8bbc3d17 100644 --- a/src/Module/Software/SoftwareCollection.php +++ b/src/Module/Software/SoftwareCollection.php @@ -60,19 +60,24 @@ public function with(Origin $origin, Software ...$software): self */ public function find(string $id): ?Entry { - if (\str_contains($id, '#')) { - $package = \strtolower((string) \strstr($id, '#', true)); - $id = \substr($id, \strlen($package) + 1); - foreach ($this->entries[$id] ?? [] as $entry) { - if ($entry->origin->package === $package) { - return $entry; - } - } + // An exact id first: a `dload.xml` alias may contain `#` too + if (isset($this->entries[$id])) { + return self::strongest($this->entries[$id]); + } + if (!\str_contains($id, '#')) { return null; } - return isset($this->entries[$id]) ? self::strongest($this->entries[$id]) : null; + $package = \strtolower((string) \strstr($id, '#', true)); + $id = \substr($id, \strlen($package) + 1); + foreach ($this->entries[$id] ?? [] as $entry) { + if ($entry->origin->package === $package) { + return $entry; + } + } + + return null; } /** diff --git a/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php b/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php index aff48421..47da20e5 100644 --- a/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php +++ b/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php @@ -4,6 +4,7 @@ namespace Internal\DLoad\Tests\Unit\Module\Binary\Internal; +use Internal\DLoad\Module\Binary\Exception\BinaryExecutionException; use Internal\DLoad\Module\Binary\Internal\BinaryExecutor; use Internal\DLoad\Module\Common\FileSystem\FS; use Internal\DLoad\Service\Logger; @@ -56,6 +57,32 @@ public function everyArgumentReachesTheBinaryVerbatim(array $arguments): void Assert::false(\file_exists($marker)); } + #[DataSet(['missing'], 'a missing binary')] + #[DataSet(['text'], 'a text file')] + public function aBinaryThatCannotStartFailsWithoutAPhpWarning(string $kind): void + { + $binary = $this->dir->join('tool' . (\PHP_OS_FAMILY === 'Windows' ? '.exe' : '')); + $kind === 'text' and \file_put_contents((string) $binary, 'not a binary'); + $warnings = []; + // A handler is called for a silenced error too; error_reporting() tells whether it would print + \set_error_handler(static function (int $level, string $message) use (&$warnings): bool { + (\error_reporting() & $level) === 0 or $warnings[] = $message; + return true; + }); + + $failed = false; + try { + (new BinaryExecutor(new Logger()))->execute($binary, '--version'); + } catch (BinaryExecutionException) { + $failed = true; + } finally { + \restore_error_handler(); + } + + Assert::true($failed); + Assert::same($warnings, []); + } + #[BeforeTest] protected function prepare(): void { diff --git a/tests/Unit/Module/Software/SoftwareCollectionTest.php b/tests/Unit/Module/Software/SoftwareCollectionTest.php index 25891f70..10706f1c 100644 --- a/tests/Unit/Module/Software/SoftwareCollectionTest.php +++ b/tests/Unit/Module/Software/SoftwareCollectionTest.php @@ -105,6 +105,17 @@ public function aPackageIdFindsNothingOutsideThatPackage(string $id): void Assert::null($collection->find($id)); } + #[Test] + public function anExactIdWithAHashWinsOverAPackageAddress(): void + { + $collection = SoftwareCollection::empty() + ->with(Origin::config(), Software::fromArray(['name' => 'Tool', 'alias' => 'acme/fw#gh', 'description' => 'local'])) + ->with(Origin::package('acme/fw'), self::software('gh', 'package')); + + Assert::same($collection->findSoftware('acme/fw#gh')?->description, 'local'); + Assert::same($collection->findSoftware('ACME/FW#gh')?->description, 'package'); + } + #[Test] public function aFirstAddedPackageWinsOverLaterPackages(): void { From aab833d51e44cba85b4c6a03024f7262882cc3c1 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 12 Oct 2026 22:02:54 +0400 Subject: [PATCH 3/5] fix(cli): escape console tags in log values, exception logs and the init command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix(cli): list a software with a numeric id in `dload show` and `dload init` docs: document the file name rule and the extension of `rename`, and the id rules of package entries Symfony's formatter reads `` and `` in any printed text, and an invalid style in it throws; package strings reach the log through warnings and through call arguments kept in a stack trace. Assisted-By: Claude Opus 5.5 --- README-es.md | 7 +- README-ru.md | 7 +- README-zh.md | 5 +- README.md | 5 +- dload.xsd | 2 +- resources/software.schema.json | 2 +- src/Command/Get.php | 5 +- src/Command/Init.php | 13 ++-- src/Command/ListSoftware.php | 19 ++---- src/Command/Show.php | 63 +++++++----------- src/Module/Binary/Internal/BinaryExecutor.php | 1 + src/Module/Software/ComposerPackages.php | 7 +- src/Service/Logger.php | 22 +++++-- .../Command/PackageRegistryCommandsTest.php | 62 +++++++++++++++++- .../Binary/Internal/BinaryExecutorTest.php | 16 +++++ .../Module/Software/ComposerPackagesTest.php | 19 ++++++ tests/Unit/Service/LoggerTest.php | 65 +++++++++++++++++++ 17 files changed, 237 insertions(+), 83 deletions(-) diff --git a/README-es.md b/README-es.md index 88ffdc80..c64ab413 100644 --- a/README-es.md +++ b/README-es.md @@ -608,6 +608,7 @@ El binario de RoadRunner construido incluirá solo los plugins especificados en #### Elementos File - **pattern**: Patrón regex para hacer match con archivos +- **rename**: Nuevo nombre opcional del archivo encontrado; un nombre de archivo, no una ruta. Se añade la extensión del archivo encontrado salvo que el nuevo nombre ya termine con ella - **extract-path**: Directorio de extracción opcional - Funciona en cualquier sistema (sin filtrado por SO/arquitectura) @@ -707,12 +708,12 @@ Funciona igual con el paquete de Composer, el PHAR y el binario. Si no, gana el paquete raíz, luego los paquetes en el orden de `require` y `require-dev`, y al descargar ese id se muestran todos los `#` entre los que elegir. - **Las declaraciones erróneas** se notifican como advertencias y se omiten; nunca detienen DLoad. Esto incluye un archivo inexistente, - un XML inválido, un atributo obligatorio vacío, una ruta donde se espera un nombre de archivo, una regex inválida - y un `version-command` que no sea una sola opción como `--version`, o `version`. + un XML inválido, un atributo obligatorio vacío, una ruta donde se espera un nombre de archivo, una regex inválida, + un `version-command` que no sea una sola opción como `--version`, o `version`, y un id con `#`, `:` o espacios alrededor. - **Desactivación**: `` descarta el software de los paquetes y mantiene el registro oficial; `` descarta ambos. -`dload software` muestra de dónde viene cada entrada: `official`, `dload.xml` o el nombre del paquete. Una entrada que oculta otras las lista en `Shadows:`, las de paquetes como `#`. +`dload software` muestra de dónde viene cada entrada: `official`, `dload.xml` o el nombre del paquete. Una entrada que oculta otras las lista en `Shadows:`, las de paquetes como `#`; `dload show ` muestra el origen como `Source:`. ## Casos de Uso diff --git a/README-ru.md b/README-ru.md index fb3831d5..ecea1774 100644 --- a/README-ru.md +++ b/README-ru.md @@ -610,6 +610,7 @@ DLoad автоматически управляет процессом сбор #### Элементы File - **pattern**: Regex-паттерн для соответствия файлам +- **rename**: Необязательное новое имя найденного файла — имя файла, а не путь. Расширение найденного файла дописывается, если новое имя на него ещё не оканчивается - **extract-path**: Опциональная папка извлечения - Работает на любой системе (без фильтрации по ОС/архитектуре) @@ -709,12 +710,12 @@ DLoad ищет `composer.json` рядом с загруженным `dload.xml` Иначе побеждает корневой пакет, затем пакеты в порядке `require` и `require-dev`, а при загрузке по этому id выводятся все варианты `#`, из которых можно выбрать. - **Ошибочные объявления** выводятся как предупреждения и пропускаются; они никогда не останавливают DLoad. Это отсутствующий файл, - невалидный XML, пустой обязательный атрибут, путь там, где ожидается имя файла, невалидное регулярное выражение - и `version-command`, который не является одной опцией вроде `--version` или словом `version`. + невалидный XML, пустой обязательный атрибут, путь там, где ожидается имя файла, невалидное регулярное выражение, + `version-command`, который не является одной опцией вроде `--version` или словом `version`, и id с `#`, `:` или пробелами по краям. - **Отключение**: `` отбрасывает ПО из пакетов и сохраняет официальный реестр; `` отбрасывает и то и другое. -`dload software` показывает, откуда взята каждая запись: `official`, `dload.xml` или имя пакета. Если запись перекрывает другие, они перечислены в строке `Shadows:`, записи пакетов — в виде `#`. +`dload software` показывает, откуда взята каждая запись: `official`, `dload.xml` или имя пакета. Если запись перекрывает другие, они перечислены в строке `Shadows:`, записи пакетов — в виде `#`; `dload show ` выводит источник в строке `Source:`. ## Сценарии использования diff --git a/README-zh.md b/README-zh.md index 2af76856..87ce0d3b 100644 --- a/README-zh.md +++ b/README-zh.md @@ -601,6 +601,7 @@ DLoad 会自动处理构建过程： #### 文件要素 - **pattern**：匹配文件的正则表达式模式 +- **rename**：可选，匹配文件的新名称；必须是文件名，而不是路径。除非新名称已以该扩展名结尾，否则会追加匹配文件的扩展名 - **extract-path**：可选的解压目录 - 在任何系统上都能工作（无操作系统/架构过滤） @@ -694,11 +695,11 @@ DLoad 在已加载的 `dload.xml` 旁边查找 `composer.json`（没有 `dload.x 否则根包优先，然后按 `require` 和 `require-dev` 中的顺序选择包， 并且下载该 id 时会列出所有可供选择的 `#`。 - **有问题的声明**会以警告形式报告并被跳过；它们永远不会使 DLoad 停止运行。包括：文件不存在、XML 无效、 - 必填属性为空、需要文件名的地方写了路径、无效的正则表达式，以及既不是单个选项（如 `--version`）也不是 `version` 的 `version-command`。 + 必填属性为空、需要文件名的地方写了路径、无效的正则表达式、既不是单个选项（如 `--version`）也不是 `version` 的 `version-command`，以及包含 `#`、`:` 或首尾有空格的 id。 - **关闭**：`` 丢弃来自包的软件并保留官方注册表； `` 两者都丢弃。 -`dload software` 会显示每个条目的来源：`official`、`dload.xml` 或包名。遮盖了其他条目的条目会在 `Shadows:` 中列出它们，包中的条目以 `#` 形式显示。 +`dload software` 会显示每个条目的来源：`official`、`dload.xml` 或包名。遮盖了其他条目的条目会在 `Shadows:` 中列出它们，包中的条目以 `#` 形式显示；`dload show ` 会在 `Source:` 中显示来源。 ## 使用场景 diff --git a/README.md b/README.md index c9be00c7..6b88eca0 100644 --- a/README.md +++ b/README.md @@ -613,6 +613,7 @@ This ensures consistent Velox versions across different environments and team me #### File Elements - **pattern**: Regex pattern to match files +- **rename**: Optional new name of the matched file, a file name, not a path. The extension of the matched file is appended unless the new name already ends with it - **extract-path**: Optional extraction directory - Works on any system (no OS/architecture filtering) @@ -713,11 +714,11 @@ It works the same for the Composer package, the PHAR and the binary. and a download of that id prints every `#` to choose from. - **Broken declarations** are reported as warnings and skipped; they never stop DLoad. That covers a missing file, an invalid XML, a required attribute left empty, a path where a file name is expected, an invalid regex, - and a `version-command` other than one option such as `--version`, or `version`. + a `version-command` other than one option such as `--version`, or `version`, and an id with `#`, `:` or spaces around it. - **Switching off**: `` drops the software of packages and keeps the official registry; `` drops both. -`dload software` shows where each entry comes from: `official`, `dload.xml` or the package name. An entry that hides others lists them under `Shadows:`, package ones as `#`. +`dload software` shows where each entry comes from: `official`, `dload.xml` or the package name. An entry that hides others lists them under `Shadows:`, package ones as `#`; `dload show ` prints the origin as `Source:`. ## Use Cases diff --git a/dload.xsd b/dload.xsd index e907a97c..b4010bb4 100644 --- a/dload.xsd +++ b/dload.xsd @@ -210,7 +210,7 @@ - Rename found file to this value with the same extension + Rename found file to this value, a file name, not a path; the extension of the found file is appended unless the value already ends with it diff --git a/resources/software.schema.json b/resources/software.schema.json index 549cb64c..400d42e0 100644 --- a/resources/software.schema.json +++ b/resources/software.schema.json @@ -100,7 +100,7 @@ }, "rename": { "type": "string", - "description": "Rename found file to this value with the same extension" + "description": "Rename found file to this value, a file name, not a path; the extension of the found file is appended unless the value already ends with it" }, "extract-path": { "type": "string", diff --git a/src/Command/Get.php b/src/Command/Get.php index 49392a95..d39ca221 100644 --- a/src/Command/Get.php +++ b/src/Command/Get.php @@ -17,7 +17,6 @@ use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\InvalidArgumentException; -use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -282,8 +281,8 @@ private function reportFailures(OutputInterface $output, array $failures, int $t { foreach ($failures as [$software, $error]) { $output->writeln(''); - $output->writeln(\sprintf(' Failed to download `%s` ', OutputFormatter::escape(Logger::printable($software)))); - $output->writeln(OutputFormatter::escape(Logger::printable(self::describeFailure($error)))); + $output->writeln(\sprintf(' Failed to download `%s` ', Logger::text($software))); + $output->writeln(Logger::text(self::describeFailure($error))); $output->isVeryVerbose() and $this->logger->exception($error, important: true); } diff --git a/src/Command/Init.php b/src/Command/Init.php index 4b248bd4..153e6987 100644 --- a/src/Command/Init.php +++ b/src/Command/Init.php @@ -8,6 +8,7 @@ use Internal\DLoad\Module\Config\Schema\Embed\Software; use Internal\DLoad\Module\Config\ConfigBuilder; use Internal\DLoad\Module\Software\SoftwareCollection; +use Internal\DLoad\Service\Logger; use Internal\Path; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -248,18 +249,18 @@ private function processSoftwareEntry( } // Show software information - $style->text("✓ Found: {$software->name}"); + $style->text('✓ Found: ' . Logger::text($software->name) . ''); if ($software->description !== '') { - $style->text(" Description: {$software->description}"); + $style->text(' Description: ' . Logger::text($software->description)); } if ($software->homepage !== null) { - $style->text(" Homepage: {$software->homepage}"); + $style->text(' Homepage: ' . Logger::text($software->homepage) . ''); } foreach ($software->repositories as $repo) { - $style->text(" Repository: {$repo->type}:{$repo->uri}"); + $style->text(' Repository: ' . Logger::text("{$repo->type}:{$repo->uri}") . ''); } return DownloadConfig::fromSoftwareId($softwareName); @@ -306,8 +307,8 @@ private function showSoftwareMenu( $softwareOptions = []; foreach ($availableSoftware as $id => $software) { - $description = $software->description !== '' ? " - {$software->description}" : ''; - $softwareOptions[] = "{$id}{$description}"; + $description = $software->description !== '' ? ' - ' . Logger::text($software->description) : ''; + $softwareOptions[] = Logger::text((string) $id) . $description; } $question = new ChoiceQuestion( diff --git a/src/Command/ListSoftware.php b/src/Command/ListSoftware.php index 4c6a4143..3fbc3e14 100644 --- a/src/Command/ListSoftware.php +++ b/src/Command/ListSoftware.php @@ -10,7 +10,6 @@ use Internal\DLoad\Service\Logger; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -59,17 +58,13 @@ protected function execute( $output->writeln('There are ' . $registry->count() . ' software available:'); $output->writeln(''); - // Definitions may come from Composer packages: their text must turn neither into console tags - // nor into terminal control sequences - $e = static fn(string $text): string => OutputFormatter::escape(Logger::printable($text)); - foreach ($registry->entries() as $entry) { $software = $entry->software; $output->writeln(\sprintf( '%s %s [%s]', - $e($software->getId()), - $e($software->name), - $e((string) $entry->origin), + Logger::text($software->getId()), + Logger::text($software->name), + Logger::text((string) $entry->origin), )); $shadowed = \array_map( @@ -81,14 +76,14 @@ protected function execute( static fn(Entry $other): bool => $other !== $entry, ), ); - $shadowed === [] or $output->writeln('Shadows: ' . $e(\implode(', ', $shadowed)) . ''); - $software->homepage and $output->writeln('Homepage: ' . $e($software->homepage) . ''); + $shadowed === [] or $output->writeln('Shadows: ' . Logger::text(\implode(', ', $shadowed)) . ''); + $software->homepage and $output->writeln('Homepage: ' . Logger::text($software->homepage) . ''); foreach ($software->repositories as $repo) { - $output->writeln('' . $e("{$repo->type}: {$repo->uri}") . ''); + $output->writeln('' . Logger::text("{$repo->type}: {$repo->uri}") . ''); } - $software->description and $output->writeln('' . $e(\wordwrap($software->description, 78)) . ''); + $software->description and $output->writeln('' . Logger::text(\wordwrap($software->description, 78)) . ''); $output->writeln(''); } diff --git a/src/Command/Show.php b/src/Command/Show.php index 591e8291..1db6fa68 100644 --- a/src/Command/Show.php +++ b/src/Command/Show.php @@ -9,13 +9,13 @@ use Internal\DLoad\Module\Common\Input\Destination; use Internal\DLoad\Module\Config\Schema\Action\Download; use Internal\DLoad\Module\Config\Schema\Actions; +use Internal\DLoad\Module\Config\Schema\Embed\Software; use Internal\DLoad\Module\Software\OriginKind; use Internal\DLoad\Module\Software\SoftwareCollection; use Internal\DLoad\Service\Logger; use Internal\Path; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -71,15 +71,6 @@ protected function execute( return $this->listAllSoftware($collection, $binaryProvider, $destination, $actions, $input, $output); } - /** - * Makes registry text safe to print: definitions may come from Composer packages, and their text - * must turn neither into console tags nor into terminal control sequences. - */ - private static function text(string $text): string - { - return OutputFormatter::escape(Logger::printable($text)); - } - private function listAllSoftware( SoftwareCollection $collection, BinaryProvider $binaryProvider, @@ -91,34 +82,28 @@ private function listAllSoftware( $showAll = (bool) $input->getOption('all'); $fallbackPath = $this->resolveDestinationPath($destination, null); - /** @var array $configDownloads */ - $configDownloads = []; - if ($actions !== null) { - foreach ($actions->downloads as $download) { - $configDownloads[$download->software] = $download; - } - } - // Definitions the downloads resolve to: `acme/fw#gh` may be a shadowed one - $configSoftware = []; - foreach (\array_keys($configDownloads) as $id) { - $software = $collection->findSoftware($id); - $software === null or $configSoftware[] = $software; + // Downloads of the project config with the definitions they resolve to: `acme/fw#gh` may be a shadowed one + /** @var list $configured */ + $configured = []; + foreach ($actions?->downloads ?? [] as $download) { + $software = $collection->findSoftware($download->software); + $software === null or $configured[] = [$download, $software]; } + $configSoftware = \array_column($configured, 1); // Track downloaded software to avoid showing them twice $downloadedSoftware = []; // BLOCK 1: Software configured in project $output->writeln('Configured software:'); - $configDownloads === [] and $output + $configured === [] and $output ->writeln(' No configured software found'); - if ($configDownloads !== []) { + if ($configured !== []) { $foundConfigured = false; - foreach ($configDownloads as $id => $download) { - $software = $collection->findSoftware($id); - if ($software === null || $software->binary === null) { + foreach ($configured as [$download, $software]) { + if ($software->binary === null) { continue; } @@ -133,8 +118,8 @@ private function listAllSoftware( $output->writeln(\sprintf( ' %s (%s) %s', - self::text($id), - self::text($binary->getVersionString() ?? 'unknown'), + Logger::text($download->software), + Logger::text($binary->getVersionString() ?? 'unknown'), $binary->getPath(), )); } @@ -182,8 +167,8 @@ private function listAllSoftware( $output->writeln(\sprintf( ' %s (%s) %s', - self::text($software->getId()), - self::text($binary->getVersionString() ?? 'unknown'), + Logger::text($software->getId()), + Logger::text($binary->getVersionString() ?? 'unknown'), $binary->getPath(), )); } @@ -212,8 +197,8 @@ private function listAllSoftware( $output->writeln(\sprintf( ' %s %s', - self::text($software->getId()), - self::text($software->description ? '- ' . $software->description : ''), + Logger::text($software->getId()), + Logger::text($software->description ? '- ' . $software->description : ''), )); } @@ -273,15 +258,15 @@ private function showSoftwareDetails( $destinationPath = $this->resolveDestinationPath($destination, $matchedDownload); // Display detailed information - $output->writeln(\sprintf('Software: %s', self::text($software->name))); + $output->writeln(\sprintf('Software: %s', Logger::text($software->name))); $software->alias === null or $software->alias === $software->name or $output - ->writeln(\sprintf('Alias: %s', self::text($software->alias))); + ->writeln(\sprintf('Alias: %s', Logger::text($software->alias))); $software->description and $output - ->writeln(\sprintf('Description: %s', self::text($software->description))); + ->writeln(\sprintf('Description: %s', Logger::text($software->description))); $software->homepage === null or $output - ->writeln(\sprintf('Homepage: %s', self::text($software->homepage))); - $output->writeln(\sprintf('Source: %s', self::text((string) $entry->origin))); + ->writeln(\sprintf('Homepage: %s', Logger::text($software->homepage))); + $output->writeln(\sprintf('Source: %s', Logger::text((string) $entry->origin))); // Show project config information $output->writeln(''); @@ -327,7 +312,7 @@ private function displayBinaryDetails(?Binary $binary, OutputInterface $output): $output->writeln('Binary information:'); $output->writeln(\sprintf(' Full path: %s', $binaryPath->absolute())); - $output->writeln(\sprintf(' Version: %s', self::text($binary->getVersionString() ?? 'unknown'))); + $output->writeln(\sprintf(' Version: %s', Logger::text($binary->getVersionString() ?? 'unknown'))); $output->writeln(\sprintf(' Size: %s', $this->formatSize($binary->getSize()))); $mtime = $binary->getMTime(); diff --git a/src/Module/Binary/Internal/BinaryExecutor.php b/src/Module/Binary/Internal/BinaryExecutor.php index 1706038b..b801332a 100644 --- a/src/Module/Binary/Internal/BinaryExecutor.php +++ b/src/Module/Binary/Internal/BinaryExecutor.php @@ -37,6 +37,7 @@ public function execute(Path $binaryPath, string ...$arguments): array // An array command starts the binary without a shell: escapeshellarg() on Windows would replace // `"`, `%` and `!` in arguments with spaces. The closed stdin keeps an interactive binary from waiting. // On Windows a binary that cannot start (a truncated file, an ELF) makes proc_open() warn and return false. + \error_clear_last(); $process = @\proc_open( [(string) $binaryPath, ...$arguments], [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['redirect', 1]], diff --git a/src/Module/Software/ComposerPackages.php b/src/Module/Software/ComposerPackages.php index 7381e0a8..de13b67a 100644 --- a/src/Module/Software/ComposerPackages.php +++ b/src/Module/Software/ComposerPackages.php @@ -189,7 +189,10 @@ private function installedPackages(array $root): array ? ((array) ($root['config'] ?? []))['vendor-dir'] ?? 'vendor' : $this->vendorDir; if (!\is_string($vendorDir) || $vendorDir === '' || \str_contains($vendorDir, "\0")) { - $this->logger->warning('Skipping registries of Composer packages: `config.vendor-dir` is not a path.'); + $this->logger->warning( + 'Skipping registries of Composer packages: %s is not a path.', + $this->vendorDir === null || $this->vendorDir === '' ? '`config.vendor-dir`' : 'COMPOSER_VENDOR_DIR', + ); return []; } @@ -294,7 +297,7 @@ private function registryFile(string $name, string $registry, Path $installPath) $packageDir = \realpath((string) $installPath); $file = Path::create($registry)->isAbsolute() ? null : \realpath((string) $installPath->join($registry)); - if ($packageDir === false || $file === false) { + if ($packageDir === false || $file === false || ($file !== null && !\is_file($file))) { $this->logger->warning( 'Package `%s`: registry file `%s` not found. Make sure it is not excluded from the package dist.', $name, diff --git a/src/Service/Logger.php b/src/Service/Logger.php index b3b44ea1..e598e93a 100644 --- a/src/Service/Logger.php +++ b/src/Service/Logger.php @@ -4,6 +4,7 @@ namespace Internal\DLoad\Service; +use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Output\OutputInterface; /** @@ -51,6 +52,15 @@ public static function printable(string $text): string return (string) \preg_replace('/[\x00-\x08\x0B-\x1F\x7F]|\xC2[\x80-\x9F]/', '', $text); } + /** + * Makes text from outside safe to print through a console output: neither console tags nor + * terminal control sequences take effect. + */ + public static function text(string $text): string + { + return OutputFormatter::escape(self::printable($text)); + } + /** * Outputs a message with a newline. * @@ -129,17 +139,17 @@ public function exception(\Throwable $e, ?string $header = null, bool $important $r = "----------------------\n"; // Print bold yellow header if exists if ($header !== null) { - $r .= "\033[1;33m" . $header . "\033[0m\n"; + $r .= "\033[1;33m" . self::text($header) . "\033[0m\n"; } // Print exception message - $r .= self::printable($e->getMessage()) . "\n"; + $r .= self::text($e->getMessage()) . "\n"; // Print file and line using green color and italic font - $r .= "In \033[3;32m" . $e->getFile() . ':' . $e->getLine() . "\033[0m\n"; + $r .= "In \033[3;32m" . self::text($e->getFile()) . ':' . $e->getLine() . "\033[0m\n"; // Print stack trace using gray $r .= "Stack trace:\n"; - // Limit stacktrace to 5 lines + // Limit stacktrace to 5 lines; their call arguments may carry text from outside $stack = \explode("\n", $e->getTraceAsString()); - $r .= "\033[90m" . \implode("\n", \array_slice($stack, 0, \min(5, \count($stack)))) . "\033[0m\n"; + $r .= "\033[90m" . self::text(\implode("\n", \array_slice($stack, 0, \min(5, \count($stack))))) . "\033[0m\n"; $r .= "\n"; $this->echo($r, !$important); } @@ -156,7 +166,7 @@ private static function values(array $values): array foreach ($values as $k => $value) { $result[$k] = match (true) { \is_bool($value) => $value ? 'TRUE' : 'FALSE', - default => self::printable((string) $value), + default => self::text((string) $value), }; } diff --git a/tests/Integration/Command/PackageRegistryCommandsTest.php b/tests/Integration/Command/PackageRegistryCommandsTest.php index bd648390..7fa8ca9e 100644 --- a/tests/Integration/Command/PackageRegistryCommandsTest.php +++ b/tests/Integration/Command/PackageRegistryCommandsTest.php @@ -5,12 +5,14 @@ namespace Internal\DLoad\Tests\Integration\Command; use Internal\DLoad\Command\Get; +use Internal\DLoad\Command\Init; use Internal\DLoad\Command\ListSoftware; use Internal\DLoad\Command\Show; use Internal\DLoad\Module\Common\FileSystem\FS; use Internal\Path; use Symfony\Component\Console\Application; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Tester\ApplicationTester; use Testo\Assert; use Testo\Codecov\Covers; @@ -237,6 +239,58 @@ public function theFailureReportOfGetHasNoControlCharacters(array $software, str ->notContains("\e[2J"); } + #[Test] + #[Covers(Show::class)] + public function aNumericIdIsListedAsConfigured(): void + { + $bin = $this->directory . \DIRECTORY_SEPARATOR . 'bin'; + \mkdir($bin); + \touch($bin . \DIRECTORY_SEPARATOR . self::executable('seven-tool')); + $this->config(\sprintf( + '' + . '', + $bin, + )); + + $tester = $this->run(['command' => 'show', '--all' => true]); + + Assert::same($tester->getStatusCode(), Command::SUCCESS); + Assert::string(self::display($tester))->contains("Configured software:\n 7 (unknown) "); + } + + #[Test] + #[Covers(Init::class)] + public function initPrintsTheTextOfAFoundEntryLiterally(): void + { + $this->config(''); + + $tester = $this->run(['command' => 'init', '--overwrite' => true], decorated: true, answers: ['evil', '']); + + Assert::same($tester->getStatusCode(), Command::SUCCESS); + $display = self::display($tester); + foreach (self::EVIL_TEXT as $text) { + Assert::string($display)->contains($text); + } + Assert::string($display)->notContains("\e]8;;"); + } + + #[Test] + #[Covers(Init::class)] + #[DataSet([''], 'without a numeric id')] + #[DataSet([''], 'with a numeric id')] + public function initMenuPrintsTheTextOfEntriesLiterally(string $registry): void + { + $this->config("$registry"); + + $tester = $this->run(['command' => 'init', '--overwrite' => true], decorated: true, answers: ['', 'y', 'evil - ' . OutputFormatter::escape(self::EVIL_TEXT['description'])]); + + Assert::same($tester->getStatusCode(), Command::SUCCESS); + Assert::string(self::display($tester)) + ->contains('evil - ' . self::EVIL_TEXT['description']) + ->notContains("\e]8;;"); + Assert::string((string) \file_get_contents($this->directory . '/dload.xml'))->contains('software="evil"'); + } + #[BeforeTest] protected function prepare(): void { @@ -340,13 +394,14 @@ private function config(string $content): void /** * @param array $input + * @param list|null $answers Answers to the questions; null runs the command non-interactively. */ - private function run(array $input, bool $decorated = false): ApplicationTester + private function run(array $input, bool $decorated = false, ?array $answers = null): ApplicationTester { $application = new Application(); $application->setAutoExit(false); $application->setCatchExceptions(false); - foreach ([new Get(), new ListSoftware(), new Show()] as $command) { + foreach ([new Get(), new Init(), new ListSoftware(), new Show()] as $command) { // Symfony Console 8 renamed `add()` to `addCommand()` \method_exists($application, 'addCommand') ? $application->addCommand($command) @@ -354,9 +409,10 @@ private function run(array $input, bool $decorated = false): ApplicationTester } $tester = new ApplicationTester($application); + $answers === null or $tester->setInputs($answers); $tester->run( $input + ['--config' => $this->directory . '/dload.xml'], - ['interactive' => false, 'decorated' => $decorated], + ['interactive' => $answers !== null, 'decorated' => $decorated], ); return $tester; diff --git a/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php b/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php index 47da20e5..bb8fa980 100644 --- a/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php +++ b/tests/Unit/Module/Binary/Internal/BinaryExecutorTest.php @@ -11,7 +11,9 @@ use Internal\Path; use Testo\Assert; use Testo\Codecov\Covers; +use Testo\Core\Exception\SkipTest; use Testo\Data\DataSet; +use Testo\Expect; use Testo\Lifecycle\AfterTest; use Testo\Lifecycle\BeforeTest; use Testo\Test; @@ -83,6 +85,20 @@ public function aBinaryThatCannotStartFailsWithoutAPhpWarning(string $kind): voi Assert::same($warnings, []); } + /** + * Only Windows reports a binary that cannot start through proc_open() itself; elsewhere the + * child process fails with an exit code instead. + */ + public function aBinaryThatCannotStartIsReportedWithItsOwnErrorNotAnEarlierOne(): never + { + \PHP_OS_FAMILY === 'Windows' or throw new SkipTest('proc_open() fails to start a binary only on Windows.'); + @\file_get_contents((string) $this->dir->join('missing.txt')); + + Expect::exception(BinaryExecutionException::class)->withMessageContaining('CreateProcess'); + + (new BinaryExecutor(new Logger()))->execute($this->dir->join('missing.exe'), '--version'); + } + #[BeforeTest] protected function prepare(): void { diff --git a/tests/Unit/Module/Software/ComposerPackagesTest.php b/tests/Unit/Module/Software/ComposerPackagesTest.php index 0c9ccab9..09d38e40 100644 --- a/tests/Unit/Module/Software/ComposerPackagesTest.php +++ b/tests/Unit/Module/Software/ComposerPackagesTest.php @@ -183,6 +183,9 @@ public function theRootRegistryIsReadWhenAnotherConfigIsLoaded(): void #[DataSet(["dload.xml\0", '`extra.dload.registry` must be a path to a file'], 'a null byte')] #[DataSet(['no-registry.xml', 'has no `` section'], 'an XML without a registry section')] #[DataSet(['foreign.xml', 'has no `` section'], 'an XML of another format')] + #[DataSet(['.', 'registry file `.` not found'], 'the package directory')] + #[DataSet(['sub', 'registry file `sub` not found'], 'a subdirectory')] + #[DataSet(['sub/', 'registry file `sub/` not found'], 'a subdirectory with a slash')] public function rejectsABrokenDeclarationAndKeepsTheOtherPackages(mixed $registry, string $warning): void { $this->file('vendor/outside.xml', self::registry('')); @@ -195,6 +198,7 @@ public function rejectsABrokenDeclarationAndKeepsTheOtherPackages(mixed $registr $this->file('vendor/acme/broken/broken.xml', ''); $this->file('vendor/acme/broken/no-registry.xml', ''); $this->file('vendor/acme/broken/foreign.xml', ''); + $this->file('vendor/acme/broken/sub/dload.xml', self::registry('')); $entries = $this->entries(); @@ -436,6 +440,21 @@ public function theVendorDirOverrideWinsOverTheConfig(?string $override, string Assert::same(self::describe($entries), ["acme/lib#from-$used"]); } + #[DataSet(["libs\0"], 'a null byte')] + #[DataSet([" \0"], 'a null byte after a space')] + public function anInvalidOverrideIsNamedInTheWarning(string $override): void + { + $this->root(['require' => ['acme/lib' => '*'], 'config' => ['vendor-dir' => 'vendor']]); + $this->installed($this->package('acme/lib', self::registry(''))); + + $entries = $this->entries(vendorDir: $override); + + Assert::same($entries, []); + Assert::string($this->output->fetch()) + ->contains('Skipping registries of Composer packages: COMPOSER_VENDOR_DIR is not a path.') + ->notContains('config.vendor-dir'); + } + public function theOverrideIsUsedWhenTheConfigVendorDirIsBroken(): void { $this->root(['require' => ['acme/lib' => '*'], 'config' => ['vendor-dir' => ['broken']]]); diff --git a/tests/Unit/Service/LoggerTest.php b/tests/Unit/Service/LoggerTest.php index a6b694ed..bcbe5756 100644 --- a/tests/Unit/Service/LoggerTest.php +++ b/tests/Unit/Service/LoggerTest.php @@ -57,6 +57,71 @@ public function anExceptionMessageIsPrintedWithoutControlCharacters(): void ->notContains("\u{9B}"); } + #[DataSet(['HIDDEN'], 'a hiding style')] + #[DataSet(['click'], 'a hyperlink')] + #[DataSet(['alarm'], 'a named style')] + public function tagsInValuesArePrintedLiterally(string $value): void + { + $output = new BufferedOutput(BufferedOutput::VERBOSITY_VERY_VERBOSE, decorated: true); + $logger = new Logger($output); + + $logger->warning('%s', $value); + $logger->info('%s', $value); + $logger->error('%s', $value); + $logger->debug('%s', $value); + $logger->status('sender', '%s', $value); + + $printed = $output->fetch(); + Assert::same(\substr_count($printed, $value), 5); + Assert::string($printed) + ->notContains("\e[30;40m") + ->notContains("\e]8;;") + ->notContains("\e[37;41malarm"); + } + + public function tagsInAnExceptionMessageArePrintedLiterally(): void + { + $output = new BufferedOutput(decorated: true); + $message = 'failed: HIDDEN click'; + + (new Logger($output))->exception(new \RuntimeException($message), important: true); + + Assert::string($output->fetch()) + ->contains($message . "\n") + ->notContains("\e[30;40m") + ->notContains("\e]8;;"); + } + + /** + * PHP keeps the first 15 characters of each string argument in the stack trace, which the + * exception log prints: an argument is as foreign as the message. + */ + #[DataSet(['red', "\e[31mred"], 'a style')] + #[DataSet(['x', 'Invalid "nope" color'], 'an invalid style')] + public function tagsInStackTraceArgumentsArePrintedLiterally(string $argument, string $rendered): void + { + $output = new BufferedOutput(decorated: true); + $exception = (static fn(string $id): \RuntimeException => new \RuntimeException('failed'))($argument); + + (new Logger($output))->exception($exception, important: true); + + Assert::string($output->fetch()) + ->contains("'$argument'") + ->notContains($rendered); + } + + #[DataSet(['C:\\dir\\'], 'a trailing backslash')] + #[DataSet(['a\\b'], 'an inner backslash')] + #[DataSet(['1 < 2 > 0'], 'comparison signs')] + public function valuesWithoutTagsArePrintedAsIs(string $value): void + { + $output = new BufferedOutput(decorated: true); + + (new Logger($output))->warning('[%s]', $value); + + Assert::same($output->fetch(), "\e[33m[$value]\e[0m\n"); + } + public function everyMessageKindCleansItsValues(): void { $output = new BufferedOutput(BufferedOutput::VERBOSITY_VERY_VERBOSE); From 6ce45a56d8cc9fa594cb2b95499d9cd16852417c Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 12 Oct 2026 22:19:11 +0400 Subject: [PATCH 4/5] fix(cli): print the binary path in `dload show` literally test(cli): keep call arguments in the trace whatever `zend.exception_ignore_args` is A binary name may contain `<` and `>` on POSIX, so the path of a package's binary could carry console tags. CI runs with php.ini-production, which drops call arguments from traces, so the trace escaping test found nothing to check there. Assisted-By: Claude Opus 5.5 --- src/Command/Show.php | 6 +++--- tests/Unit/Service/LoggerTest.php | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Command/Show.php b/src/Command/Show.php index 1db6fa68..68fe40a6 100644 --- a/src/Command/Show.php +++ b/src/Command/Show.php @@ -120,7 +120,7 @@ private function listAllSoftware( ' %s (%s) %s', Logger::text($download->software), Logger::text($binary->getVersionString() ?? 'unknown'), - $binary->getPath(), + Logger::text((string) $binary->getPath()), )); } @@ -169,7 +169,7 @@ private function listAllSoftware( ' %s (%s) %s', Logger::text($software->getId()), Logger::text($binary->getVersionString() ?? 'unknown'), - $binary->getPath(), + Logger::text((string) $binary->getPath()), )); } @@ -311,7 +311,7 @@ private function displayBinaryDetails(?Binary $binary, OutputInterface $output): $binaryPath = $binary->getPath(); $output->writeln('Binary information:'); - $output->writeln(\sprintf(' Full path: %s', $binaryPath->absolute())); + $output->writeln(\sprintf(' Full path: %s', Logger::text((string) $binaryPath->absolute()))); $output->writeln(\sprintf(' Version: %s', Logger::text($binary->getVersionString() ?? 'unknown'))); $output->writeln(\sprintf(' Size: %s', $this->formatSize($binary->getSize()))); diff --git a/tests/Unit/Service/LoggerTest.php b/tests/Unit/Service/LoggerTest.php index bcbe5756..0edd5c3a 100644 --- a/tests/Unit/Service/LoggerTest.php +++ b/tests/Unit/Service/LoggerTest.php @@ -101,7 +101,13 @@ public function tagsInAnExceptionMessageArePrintedLiterally(): void public function tagsInStackTraceArgumentsArePrintedLiterally(string $argument, string $rendered): void { $output = new BufferedOutput(decorated: true); - $exception = (static fn(string $id): \RuntimeException => new \RuntimeException('failed'))($argument); + // php.ini-production, as on CI, drops the arguments from traces + $ignoreArgs = \ini_set('zend.exception_ignore_args', '0'); + try { + $exception = (static fn(string $id): \RuntimeException => new \RuntimeException('failed'))($argument); + } finally { + $ignoreArgs === false or \ini_set('zend.exception_ignore_args', $ignoreArgs); + } (new Logger($output))->exception($exception, important: true); From 07067cd44e63076c3c6fe286ef8ab9f706dc2225 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 12 Oct 2026 22:49:40 +0400 Subject: [PATCH 5/5] fix(installer): print the installed file name and version literally fix(cli): print the output of a failed build literally test(cli): keep string arguments in the trace whatever `zend.exception_string_param_max_len` is A package's binary name may contain `<` and `>` on POSIX: an invalid style in it made the formatter throw after the file was written, before its chmod, so `get` reported a failure for an installed, non-executable binary. Assisted-By: Claude Opus 5.5 --- src/Command/Build.php | 3 ++- .../Installer/Internal/RuleExtraction.php | 6 +++--- .../Installer/Internal/Step/ArchiveStep.php | 4 ++-- tests/Unit/Module/Installer/InstallerTest.php | 19 ++++++++++++++++++- tests/Unit/Service/LoggerTest.php | 8 +++++--- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/Command/Build.php b/src/Command/Build.php index 3d724745..2bb3d8d7 100644 --- a/src/Command/Build.php +++ b/src/Command/Build.php @@ -13,6 +13,7 @@ use Internal\DLoad\Module\Velox\Exception\Dependency as DependencyException; use Internal\DLoad\Module\Velox\Result; use Internal\DLoad\Module\Velox\Task; +use Internal\DLoad\Service\Logger; use React\Promise\PromiseInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -122,7 +123,7 @@ private function processException(\Throwable $e, Task $task): void $this->logger->error('Build error: %s', $e->getMessage()); if ($e->buildOutput !== null) { $this->logger->info('Build Output:'); - $this->logger->print($e->buildOutput); + $this->logger->print(Logger::text($e->buildOutput)); } return; diff --git a/src/Module/Installer/Internal/RuleExtraction.php b/src/Module/Installer/Internal/RuleExtraction.php index 143de3bb..d49c9de3 100644 --- a/src/Module/Installer/Internal/RuleExtraction.php +++ b/src/Module/Installer/Internal/RuleExtraction.php @@ -79,10 +79,10 @@ public function __invoke(Installation $installation): DloadResult $this->output->writeln( \sprintf( '%s (%s) has been %sinstalled into %s', - $to->getFilename(), - $installation->download->version, + Logger::text($to->getFilename()), + Logger::text((string) $installation->download->version), $isOverwriting ? 're' : '', - $path, + Logger::text($path), ), ); diff --git a/src/Module/Installer/Internal/Step/ArchiveStep.php b/src/Module/Installer/Internal/Step/ArchiveStep.php index 8c6aaadf..5b4678f9 100644 --- a/src/Module/Installer/Internal/Step/ArchiveStep.php +++ b/src/Module/Installer/Internal/Step/ArchiveStep.php @@ -120,8 +120,8 @@ public function install(Installation $installation, callable $next): DloadResult \sprintf( '%d file(s) from %s have been installed into %s', \count($resultFiles), - $installation->download->version, - (string) $destination, + Logger::text((string) $installation->download->version), + Logger::text((string) $destination), ), ); diff --git a/tests/Unit/Module/Installer/InstallerTest.php b/tests/Unit/Module/Installer/InstallerTest.php index 51ab8302..516409b3 100644 --- a/tests/Unit/Module/Installer/InstallerTest.php +++ b/tests/Unit/Module/Installer/InstallerTest.php @@ -270,6 +270,22 @@ public function theBinaryKeepsItsNameAndGainsOnlyAMissingExtension(string $name, Assert::same(self::paths($result->files), [(string) $this->destination->join($installed)]); } + #[Test] + public function aBinaryNameWithConsoleTagsIsInstalledAndPrintedLiterally(): void + { + \PHP_OS_FAMILY === 'Windows' and throw new SkipTest('A Windows file name cannot contain `<` and `>`.'); + $name = 'xy'; + $download = $this->download('tool.stub', ''); + $software = Software::fromArray(['name' => 'Tool', 'binary' => ['name' => $name, 'pattern' => '/./']]); + $output = new BufferedOutput(decorated: true); + + $this->installer(archiveFactory: self::listed(['pkg/app' => 'binary']), output: $output) + ->install($download, $software, null, $this->destination, removeDownload: true); + + Assert::string($output->fetch())->contains($name); + Assert::true(\is_executable((string) $this->destination->join($name))); + } + #[Test] #[DataSet([['files' => [['pattern' => '/^app$/', 'rename' => '../escaped']]]], 'a rename above the destination')] #[DataSet([['files' => [['pattern' => '/^app$/', 'rename' => 'sub/app']]]], 'a rename into a subdirectory')] @@ -347,13 +363,14 @@ private static function listed(array $entries): ArchiveFactory private function installer( ?BinaryProvider $binaryProvider = null, ArchiveFactory $archiveFactory = new ArchiveFactory(), + OutputInterface $output = new BufferedOutput(), ): Installer { if ($binaryProvider === null) { $binaryProvider = \Mockery::mock(BinaryProvider::class); $binaryProvider->allows('getLocalBinary')->andReturnNull(); } - return new Installer(new Logger($this->log), new BufferedOutput(), $archiveFactory, $binaryProvider, OperatingSystem::Linux); + return new Installer(new Logger($this->log), $output, $archiveFactory, $binaryProvider, OperatingSystem::Linux); } /** diff --git a/tests/Unit/Service/LoggerTest.php b/tests/Unit/Service/LoggerTest.php index 0edd5c3a..01b4e3e0 100644 --- a/tests/Unit/Service/LoggerTest.php +++ b/tests/Unit/Service/LoggerTest.php @@ -101,16 +101,18 @@ public function tagsInAnExceptionMessageArePrintedLiterally(): void public function tagsInStackTraceArgumentsArePrintedLiterally(string $argument, string $rendered): void { $output = new BufferedOutput(decorated: true); - // php.ini-production, as on CI, drops the arguments from traces + // php.ini-production, as on CI, drops the arguments from traces and cuts strings to nothing; + // the trace is rendered when the logger prints it, so the settings hold until then $ignoreArgs = \ini_set('zend.exception_ignore_args', '0'); + $maxLength = \ini_set('zend.exception_string_param_max_len', '15'); try { $exception = (static fn(string $id): \RuntimeException => new \RuntimeException('failed'))($argument); + (new Logger($output))->exception($exception, important: true); } finally { $ignoreArgs === false or \ini_set('zend.exception_ignore_args', $ignoreArgs); + $maxLength === false or \ini_set('zend.exception_string_param_max_len', $maxLength); } - (new Logger($output))->exception($exception, important: true); - Assert::string($output->fetch()) ->contains("'$argument'") ->notContains($rendered);