From 4000afa189f62087256a4972638299f977764f66 Mon Sep 17 00:00:00 2001 From: Rene Reimann Date: Mon, 2 Nov 2026 15:19:51 +0100 Subject: [PATCH 1/6] Build: raise PHPStan to ^2.2 in all three analysed projects (#176) --- composer.json | 2 +- test/Bridge/Laravel/composer.json | 2 +- test/Bridge/Symfony/composer.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index f22679c..34884a7 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "require-dev": { "phpunit/phpunit": "^11.0", "mockery/mockery": "^1.6", - "phpstan/phpstan": "^1.12", + "phpstan/phpstan": "^2.2", "league/openapi-psr7-validator": "^0.22", "squizlabs/php_codesniffer": "^4.0" }, diff --git a/test/Bridge/Laravel/composer.json b/test/Bridge/Laravel/composer.json index 84b2f80..999fcf4 100644 --- a/test/Bridge/Laravel/composer.json +++ b/test/Bridge/Laravel/composer.json @@ -9,7 +9,7 @@ "require-dev": { "zammad/zammad-api-client-php": "@dev", "phpunit/phpunit": "^11.0", - "phpstan/phpstan": "^1.12", + "phpstan/phpstan": "^2.2", "laravel/framework": "^12.0 || ^13.0" }, "autoload-dev": { diff --git a/test/Bridge/Symfony/composer.json b/test/Bridge/Symfony/composer.json index 37bf715..01e0e98 100644 --- a/test/Bridge/Symfony/composer.json +++ b/test/Bridge/Symfony/composer.json @@ -9,7 +9,7 @@ "require-dev": { "zammad/zammad-api-client-php": "@dev", "phpunit/phpunit": "^11.0", - "phpstan/phpstan": "^1.12", + "phpstan/phpstan": "^2.2", "symfony/config": "^6.4 || ^7.0 || ^8.0", "symfony/dependency-injection": "^6.4 || ^7.0 || ^8.0", "symfony/http-kernel": "^6.4 || ^7.0 || ^8.0" From 002b5d94162f080673daafabcaa656d4e9bb2878 Mon Sep 17 00:00:00 2001 From: Rene Reimann Date: Mon, 2 Nov 2026 15:19:54 +0100 Subject: [PATCH 2/6] Types: build string-keyed arrays instead of trusting get_object_vars() (#176) --- src/Core/Repository/AbstractRepository.php | 30 +++++++++++++------ src/Core/Traits/SerializesToArray.php | 18 ++++++++++-- src/Core/Transport/HttpPageFetcher.php | 34 +++++++++++++++++----- src/Endpoints/Tags/TagRepository.php | 24 +++++++++++---- src/Endpoints/Tickets/TicketUpdateDTO.php | 13 +++++++-- 5 files changed, 93 insertions(+), 26 deletions(-) diff --git a/src/Core/Repository/AbstractRepository.php b/src/Core/Repository/AbstractRepository.php index ffcc402..8446c0d 100644 --- a/src/Core/Repository/AbstractRepository.php +++ b/src/Core/Repository/AbstractRepository.php @@ -87,7 +87,7 @@ protected function getListKey(): string public function find(int $id): DTOInterface { return $this->dtoClass::fromArray( - $this->handler->get("{$this->resourcePath}/{$id}", ['expand' => 'true']), + ResponseParser::fields($this->handler->get("{$this->resourcePath}/{$id}", ['expand' => 'true'])), ); } @@ -198,7 +198,7 @@ public function allPage(int $page, int $perPage, array $query = []): array 'per_page' => (string) $perPage, ]); - return $this->hydrateList($this->handler->get($this->resourcePath, $params)); + return $this->hydrateList(ResponseParser::fields($this->handler->get($this->resourcePath, $params))); } /** @@ -217,7 +217,7 @@ public function searchPage(string $term, int $page, int $perPage, array $query = 'per_page' => (string) $perPage, ]); - return $this->hydrateList($this->handler->get("{$this->resourcePath}/search", $params)); + return $this->hydrateList(ResponseParser::fields($this->handler->get("{$this->resourcePath}/search", $params))); } /** @@ -247,7 +247,7 @@ protected function paginateWith( do { $params = array_merge($query, ['page' => (string) $page, 'per_page' => (string) $this->pageSize]); - $items = $this->extractItems($this->handler->get($endpoint, $params), $listKey); + $items = $this->extractItems(ResponseParser::fields($this->handler->get($endpoint, $params)), $listKey); foreach ($items as $item) { yield $dtoClass::fromArray($item); @@ -317,7 +317,9 @@ public function totalCount(): int */ public function create(DTOInterface $dto): DTOInterface { - return $this->dtoClass::fromArray($this->handler->post($this->resourcePath, $dto->toArray())); + return $this->dtoClass::fromArray( + ResponseParser::fields($this->handler->post($this->resourcePath, $dto->toArray())), + ); } /** @@ -346,13 +348,23 @@ public function patch(int $id, array|object $changes): DTOInterface $body = $changes->toArray(); } elseif ($changes instanceof PatchableInterface) { $body = $changes->toPatchArray(); - } elseif (is_object($changes)) { - $body = array_filter(get_object_vars($changes), fn($v) => $v !== null); } else { - $body = array_filter($changes, fn($v) => $v !== null); + // get_object_vars() and a caller-supplied array are both typed + // array; rebuild with string keys so the declared + // body type holds. See SerializesToArray::toArray(). + $body = []; + $source = is_object($changes) ? get_object_vars($changes) : $changes; + + foreach ($source as $key => $value) { + if ($value !== null) { + $body[(string) $key] = $value; + } + } } - return $this->dtoClass::fromArray($this->handler->put("{$this->resourcePath}/{$id}", $body)); + return $this->dtoClass::fromArray( + ResponseParser::fields($this->handler->put("{$this->resourcePath}/{$id}", $body)), + ); } /** diff --git a/src/Core/Traits/SerializesToArray.php b/src/Core/Traits/SerializesToArray.php index 8bd9b10..64c9fbb 100644 --- a/src/Core/Traits/SerializesToArray.php +++ b/src/Core/Traits/SerializesToArray.php @@ -84,14 +84,26 @@ public function toArray(): array $vars = get_object_vars($this); foreach ($vars as $key => $value) { if ($value !== null && $key !== 'customFields') { - $result[$key] = $value instanceof DateTimeImmutable ? $value->format('c') : $value; + // get_object_vars() is typed array: a numeric + // property name would come back as an int key. Promoted readonly + // properties cannot be numeric, but the trait is generic, so the + // cast keeps the declared array true either way. + $result[(string) $key] = $value instanceof DateTimeImmutable + ? $value->format('c') + : $value; } } if (array_key_exists('customFields', $vars) && is_array($vars['customFields'])) { foreach ($vars['customFields'] as $k => $v) { - if (!in_array($k, self::serverReadOnlyKeys(), true)) { - $result[$k] = $v; + // Zammad field names are strings, but a plain array permits + // integer keys. Casting keeps the declared array + // honest and makes the strict in_array() below meaningful for + // keys that arrived as integers. + $key = (string) $k; + + if (!in_array($key, self::serverReadOnlyKeys(), true)) { + $result[$key] = $v; } } } diff --git a/src/Core/Transport/HttpPageFetcher.php b/src/Core/Transport/HttpPageFetcher.php index 6ea6d00..614849d 100644 --- a/src/Core/Transport/HttpPageFetcher.php +++ b/src/Core/Transport/HttpPageFetcher.php @@ -54,9 +54,11 @@ public function fetch(int $page, int $perPage, array $baseQuery): array $data = $this->handler->get($this->endpoint, $params); + $fields = ResponseParser::fields($data); + return $isSearch - ? $this->extractSearchResults($data) - : $this->extractIndexResults($data); + ? $this->extractSearchResults($fields) + : $this->extractIndexResults($fields); } /** @@ -92,16 +94,34 @@ private function extractIndexResults(array $data): array } /** - * @param array> $items + * Hydrates whatever item arrays are in $items, ignoring everything else. + * + * The parameter is deliberately as loose as the method's own filtering: + * search responses hand over a raw `records` value, and claiming a narrower + * type here would only move the unchecked cast to the caller. + * + * @param array $items * @return list */ private function hydrateItems(array $items): array { + $hydrated = []; + + foreach ($items as $item) { + if (!is_array($item)) { + continue; + } + + $fields = []; + foreach ($item as $key => $value) { + $fields[(string) $key] = $value; + } + + $hydrated[] = $this->dtoClass::fromArray($fields); + } + /** @var list */ - return array_map( - fn(array $item): DTOInterface => $this->dtoClass::fromArray($item), - array_values(array_filter($items, 'is_array')), - ); + return $hydrated; } private function inferListKey(): string diff --git a/src/Endpoints/Tags/TagRepository.php b/src/Endpoints/Tags/TagRepository.php index 00c1267..a588f79 100644 --- a/src/Endpoints/Tags/TagRepository.php +++ b/src/Endpoints/Tags/TagRepository.php @@ -7,6 +7,7 @@ use Generator; use InvalidArgumentException; use ZammadAPIClient\Core\Repository\AbstractRepository; +use ZammadAPIClient\Core\Repository\ResponseParser; /** * Repository for the `/api/v1/tags` endpoint. @@ -83,7 +84,20 @@ protected function extractItems(array $data, ?string $key = null): array ); return array_map( - fn($v) => is_string($v) ? ['value' => $v] : $v, + static function (mixed $v): array { + if (is_string($v)) { + return ['value' => $v]; + } + + // Tag payloads arrive as array; re-key so the + // declared array item shape holds. + $item = []; + foreach ((array) $v as $key => $value) { + $item[(string) $key] = $value; + } + + return $item; + }, $filtered, ); } @@ -127,11 +141,11 @@ public function add( int $objectId, string $tag, ): array { - return $this->handler->post('tags/add', [ + return ResponseParser::fields($this->handler->post('tags/add', [ 'object' => $objectType, 'o_id' => $objectId, 'item' => $tag, - ]); + ])); } /** @@ -157,7 +171,7 @@ public function remove( 'item' => $tag, ]); - return $this->handler->delete($uri); + return ResponseParser::fields($this->handler->delete($uri)); } /** @@ -174,6 +188,6 @@ public function remove( */ public function tagSearch(string $term): array { - return $this->handler->get('tag_search', ['term' => $term]); + return ResponseParser::fields($this->handler->get('tag_search', ['term' => $term])); } } diff --git a/src/Endpoints/Tickets/TicketUpdateDTO.php b/src/Endpoints/Tickets/TicketUpdateDTO.php index 4b464be..0b59936 100644 --- a/src/Endpoints/Tickets/TicketUpdateDTO.php +++ b/src/Endpoints/Tickets/TicketUpdateDTO.php @@ -49,7 +49,16 @@ public function __construct( */ public function toPatchArray(): array { - $vars = get_object_vars($this); - return array_filter($vars, fn($v) => $v !== null); + $result = []; + + // See the note in SerializesToArray::toArray(): get_object_vars() does + // not guarantee string keys, so build the array rather than filtering it. + foreach (get_object_vars($this) as $key => $value) { + if ($value !== null) { + $result[(string) $key] = $value; + } + } + + return $result; } } From f49d647b192360ccaaa540f6a0c0f8c79b45be94 Mon Sep 17 00:00:00 2001 From: Rene Reimann Date: Mon, 2 Nov 2026 15:19:57 +0100 Subject: [PATCH 3/6] Transport: say that a decoded body may be a JSON array, and narrow where it must not be (#176) --- .../Contracts/RequestHandlerInterface.php | 10 +++--- src/Core/Repository/Resource.php | 2 +- src/Core/Repository/ResponseParser.php | 24 ++++++++++++++ src/Core/Transport/RequestHandler.php | 32 +++++++++++++------ src/Endpoints/Links/LinkRepository.php | 7 ++-- .../Organizations/OrganizationRepository.php | 5 +-- .../TextModules/TextModuleRepository.php | 5 +-- src/Endpoints/Users/UserRepository.php | 5 +-- 8 files changed, 65 insertions(+), 25 deletions(-) diff --git a/src/Core/Contracts/RequestHandlerInterface.php b/src/Core/Contracts/RequestHandlerInterface.php index 140c2de..39fbbce 100644 --- a/src/Core/Contracts/RequestHandlerInterface.php +++ b/src/Core/Contracts/RequestHandlerInterface.php @@ -29,7 +29,7 @@ interface RequestHandlerInterface * PSR-18 client, which allows setting custom headers or timeouts per call. * * @param array $options PSR-18 client options (e.g. `['json' => [...]]`). - * @return array JSON-decoded response body. + * @return array JSON-decoded response body. * @throws \ZammadAPIClient\Exceptions\AuthenticationException For 401 responses. * @throws \ZammadAPIClient\Exceptions\NotFoundException For 404 responses. * @throws \ZammadAPIClient\Exceptions\ValidationException For 422 responses. @@ -59,7 +59,7 @@ public function getRaw(string $uri, array $query = [], array $headers = []): str * Performs a GET request and returns the decoded JSON body. * * @param array $query URL query parameters appended to the URI. - * @return array + * @return array */ public function get(string $uri, array $query = []): array; @@ -67,7 +67,7 @@ public function get(string $uri, array $query = []): array; * Performs a POST request with a JSON body and returns the decoded response. * * @param array $body Request payload; serialised to JSON automatically. - * @return array + * @return array */ public function post(string $uri, array $body = []): array; @@ -75,7 +75,7 @@ public function post(string $uri, array $body = []): array; * Performs a PUT request with a JSON body and returns the decoded response. * * @param array $body Request payload; serialised to JSON automatically. - * @return array + * @return array */ public function put(string $uri, array $body = []): array; @@ -85,7 +85,7 @@ public function put(string $uri, array $body = []): array; * Zammad's DELETE endpoints return the deleted resource's data, which * callers may use to confirm the operation. * - * @return array + * @return array */ public function delete(string $uri): array; diff --git a/src/Core/Repository/Resource.php b/src/Core/Repository/Resource.php index 47da0e4..9c112ed 100644 --- a/src/Core/Repository/Resource.php +++ b/src/Core/Repository/Resource.php @@ -131,7 +131,7 @@ public function save(): self return $this; } - $this->attributes = array_merge($this->attributes, $data); + $this->attributes = array_merge($this->attributes, ResponseParser::fields($data)); $this->dto = $this->dto::fromArray($this->attributes); $this->changes = []; $this->newRecord = false; diff --git a/src/Core/Repository/ResponseParser.php b/src/Core/Repository/ResponseParser.php index 6e0b234..0376835 100644 --- a/src/Core/Repository/ResponseParser.php +++ b/src/Core/Repository/ResponseParser.php @@ -17,6 +17,30 @@ */ final class ResponseParser { + /** + * Narrows a decoded response body to the string-keyed shape DTOs expect. + * + * A JSON object decodes to string keys, but a decoded body is typed + * `array` because Zammad answers some endpoints with a + * top-level JSON array, which decodes to integer keys. Single-resource + * endpoints always answer with an object, so this states that expectation + * at the one place it is made instead of asserting it silently at each + * call site. + * + * @param array $data + * @return array + */ + public static function fields(array $data): array + { + $fields = []; + + foreach ($data as $key => $value) { + $fields[(string) $key] = $value; + } + + return $fields; + } + /** * Extracts item arrays from an API response. * diff --git a/src/Core/Transport/RequestHandler.php b/src/Core/Transport/RequestHandler.php index 0c40c04..67b9fde 100644 --- a/src/Core/Transport/RequestHandler.php +++ b/src/Core/Transport/RequestHandler.php @@ -5,6 +5,7 @@ namespace ZammadAPIClient\Core\Transport; use InvalidArgumentException; +use JsonException; use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestFactoryInterface; @@ -14,6 +15,7 @@ use Psr\Log\NullLogger; use ZammadAPIClient\Core\Auth; use ZammadAPIClient\Core\Contracts\RequestHandlerInterface; +use ZammadAPIClient\Core\Repository\ResponseParser; use ZammadAPIClient\Exceptions\AuthenticationException; use ZammadAPIClient\Exceptions\BadRequestException; use ZammadAPIClient\Exceptions\ForbiddenException; @@ -23,7 +25,6 @@ use ZammadAPIClient\Exceptions\ServerErrorException; use ZammadAPIClient\Exceptions\ValidationException; use ZammadAPIClient\Exceptions\ZammadException; -use JsonException; /** * Concrete PSR-18 based HTTP transport for the Zammad REST API. @@ -197,7 +198,7 @@ public function getLastResponse(): ?ResponseInterface * Zammad DELETE endpoints return 200 with no body). * * @param array $options Raw PSR-18 options forwarded verbatim to the HTTP client. - * @return array + * @return array */ public function request( string $method, @@ -252,7 +253,7 @@ public function getRaw(string $uri, array $query = [], array $headers = []): str * appended to the URI; they are not sent as a request body. * * @param array $query URL query parameters. - * @return array + * @return array */ public function get(string $uri, array $query = []): array { @@ -270,7 +271,7 @@ public function get(string $uri, array $query = []): array * Use this for resource creation (Zammad convention: POST → 201 or 200 with body). * * @param array $body Payload fields; serialised to JSON automatically. - * @return array + * @return array */ public function post(string $uri, array $body = []): array { @@ -287,7 +288,7 @@ public function post(string $uri, array $body = []): array * (patch), since Zammad uses PUT for both semantics. * * @param array $body Payload fields; serialised to JSON automatically. - * @return array + * @return array */ public function put(string $uri, array $body = []): array { @@ -303,7 +304,7 @@ public function put(string $uri, array $body = []): array * Zammad's API returns the deleted resource's state in the response body, * which can be used as a confirmation receipt. An empty body returns `[]`. * - * @return array + * @return array */ public function delete(string $uri): array { @@ -333,11 +334,22 @@ private function dispatch(string $method, string $uri, array $options): Response if (isset($options['headers']) && is_array($options['headers'])) { foreach ($options['headers'] as $name => $value) { - $request = $request->withHeader($name, $value); + // PSR-7 accepts string|array. Passing anything else + // raises an InvalidArgumentException from inside the message + // implementation, which names the transport rather than the + // caller's own input, so coerce what can be coerced. + if (is_scalar($value)) { + $request = $request->withHeader((string) $name, (string) $value); + } elseif (is_array($value)) { + $request = $request->withHeader( + (string) $name, + array_map(static fn(mixed $v): string => is_scalar($v) ? (string) $v : '', $value), + ); + } } } - if (isset($options['body']) && $options['body'] !== null) { + if (isset($options['body'])) { $body = is_string($options['body']) ? $options['body'] : json_encode($options['body'], JSON_THROW_ON_ERROR); @@ -388,7 +400,7 @@ private function validationError(string $raw): ValidationException { return new ValidationException( $this->extractErrorMessage($raw) ?? $this->extractValidationMessage($raw), - $this->extractValidationErrors($this->decodeLenient($raw)), + $this->extractValidationErrors(ResponseParser::fields($this->decodeLenient($raw))), ); } @@ -442,7 +454,7 @@ private function extractValidationMessage(string $raw): string return 'Validation failed — ' . substr($raw, 0, 200); } - /** @return array */ + /** @return array */ private function decodeLenient(string $body): array { if ($body === '') { diff --git a/src/Endpoints/Links/LinkRepository.php b/src/Endpoints/Links/LinkRepository.php index 374fb8b..9994fee 100644 --- a/src/Endpoints/Links/LinkRepository.php +++ b/src/Endpoints/Links/LinkRepository.php @@ -6,6 +6,7 @@ use InvalidArgumentException; use ZammadAPIClient\Core\Repository\AbstractRepository; +use ZammadAPIClient\Core\Repository\ResponseParser; /** * Repository for the `/api/v1/links` endpoint. @@ -71,13 +72,13 @@ public function add( string $targetType, int $targetId, ): array { - return $this->handler->post('links/add', [ + return ResponseParser::fields($this->handler->post('links/add', [ 'link_type' => $linkType, 'link_object_source' => $sourceType, 'link_object_source_number' => $sourceNumber, 'link_object_target' => $targetType, 'link_object_target_value' => $targetId, - ]); + ])); } /** @@ -110,6 +111,6 @@ public function remove( 'link_object_target_value' => $targetId, ]); - return $this->handler->delete($uri); + return ResponseParser::fields($this->handler->delete($uri)); } } diff --git a/src/Endpoints/Organizations/OrganizationRepository.php b/src/Endpoints/Organizations/OrganizationRepository.php index cbc0a2c..5b734aa 100644 --- a/src/Endpoints/Organizations/OrganizationRepository.php +++ b/src/Endpoints/Organizations/OrganizationRepository.php @@ -4,8 +4,9 @@ namespace ZammadAPIClient\Endpoints\Organizations; -use ZammadAPIClient\Core\Repository\AbstractRepository; use ZammadAPIClient\Core\Contracts\DeletableInterface; +use ZammadAPIClient\Core\Repository\AbstractRepository; +use ZammadAPIClient\Core\Repository\ResponseParser; /** * Repository for the `/api/v1/organizations` endpoint. @@ -33,7 +34,7 @@ final class OrganizationRepository extends AbstractRepository implements Deletab */ public function import(string $csv): array { - return $this->handler->post("{$this->resourcePath}/import", ['data' => $csv]); + return ResponseParser::fields($this->handler->post("{$this->resourcePath}/import", ['data' => $csv])); } public function delete(int $id): void diff --git a/src/Endpoints/TextModules/TextModuleRepository.php b/src/Endpoints/TextModules/TextModuleRepository.php index df1312f..b89f99f 100644 --- a/src/Endpoints/TextModules/TextModuleRepository.php +++ b/src/Endpoints/TextModules/TextModuleRepository.php @@ -4,8 +4,9 @@ namespace ZammadAPIClient\Endpoints\TextModules; -use ZammadAPIClient\Core\Repository\AbstractRepository; use ZammadAPIClient\Core\Contracts\DeletableInterface; +use ZammadAPIClient\Core\Repository\AbstractRepository; +use ZammadAPIClient\Core\Repository\ResponseParser; /** * Repository for the `/api/v1/text_modules` endpoint. @@ -30,6 +31,6 @@ public function delete(int $id): void */ public function import(string $csv): array { - return $this->handler->post("{$this->resourcePath}/import", ['data' => $csv]); + return ResponseParser::fields($this->handler->post("{$this->resourcePath}/import", ['data' => $csv])); } } diff --git a/src/Endpoints/Users/UserRepository.php b/src/Endpoints/Users/UserRepository.php index a87bae7..cc6cb55 100644 --- a/src/Endpoints/Users/UserRepository.php +++ b/src/Endpoints/Users/UserRepository.php @@ -4,8 +4,9 @@ namespace ZammadAPIClient\Endpoints\Users; -use ZammadAPIClient\Core\Repository\AbstractRepository; use ZammadAPIClient\Core\Contracts\DeletableInterface; +use ZammadAPIClient\Core\Repository\AbstractRepository; +use ZammadAPIClient\Core\Repository\ResponseParser; /** * Repository for the `/api/v1/users` endpoint. @@ -36,7 +37,7 @@ final class UserRepository extends AbstractRepository implements DeletableInterf */ public function import(string $csv): array { - return $this->handler->post("{$this->resourcePath}/import", ['data' => $csv]); + return ResponseParser::fields($this->handler->post("{$this->resourcePath}/import", ['data' => $csv])); } public function delete(int $id): void From 52e14646a0cc486f1eb331e367a5d443e215c92f Mon Sep 17 00:00:00 2001 From: Rene Reimann Date: Mon, 2 Nov 2026 15:19:59 +0100 Subject: [PATCH 4/6] Cleanup: drop guards the analyser proves unreachable (#176) --- CHANGELOG.md | 4 ++++ src/Bridge/SymfonyBundle.php | 2 +- src/Core/Repository/PaginatedList.php | 8 -------- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1756056..c27cd1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ - **Fixed:** `LaravelServiceProvider` read `ZAMMAD_URL` / `ZAMMAD_TOKEN` with `env()` from inside the container binding, i.e. at the moment the client was resolved. Under `php artisan config:cache` Laravel skips the `LoadEnvironmentVariables` bootstrapper altogether, so `.env` is never loaded and such a call returns its default — an application that kept its token in `.env` and never ran `vendor:publish` therefore worked in development and failed in production with *"API token must not be empty."*. `register()` now merges the packaged `config/zammad.php` via `mergeConfigFrom()`, which leaves the single `env()` call in the one file `config:cache` actually freezes. **Who is affected:** anyone setting an explicitly empty `url` or `token` in a published `config/zammad.php` while relying on the environment to fill it in — an explicit configuration value now wins over the environment instead of being treated as unset. The built-in URL default still applies to an empty value ([#169](https://github.com/zammad/zammad-api-client-php/issues/169)) - **Fixed:** the provider passed `mixed` configuration values straight into `ZammadClient::create()` and `Auth::token()`. Under `declare(strict_types=1)` a non-string value in a published `config/zammad.php` surfaced as a `TypeError` raised from inside the client, naming the client rather than the config file that had to be fixed. Values are now narrowed before use ([#169](https://github.com/zammad/zammad-api-client-php/issues/169)) - **Fixed:** `Cast::dateTime()` caught `DateMalformedStringException`, a class that only exists from PHP 8.3 on. The library supports PHP 8.1 and 8.2, where `new DateTimeImmutable()` throws a plain `Exception` for an unparsable value — the catch never matched and the exception escaped, contradicting both the method's documented `null` return and the class contract that these coercions never throw. It now catches `Exception`, which on 8.3 and later is the same thing. **Who is affected:** anyone on PHP 8.1 or 8.2 whose Zammad returns a timestamp the parser rejects ([#169](https://github.com/zammad/zammad-api-client-php/issues/169)) +- Static analysis runs on PHPStan 2.x in all three projects, and the 21 findings that the newer analyser surfaced are corrected rather than suppressed — no baseline file is introduced. Most came from one cause: `get_object_vars()` is typed `array`, because a numeric property name decodes to an integer key, so `toArray()` and `toPatchArray()` never actually delivered the `array` they promised ([#176](https://github.com/zammad/zammad-api-client-php/issues/176)) +- **Breaking:** the decoded-body returns of `RequestHandlerInterface` (`request()`, `get()`, `post()`, `put()`, `delete()`) are documented as `array` instead of `array`. That is what they have always returned: Zammad answers search endpoints with a top-level JSON array, which decodes to integer keys, and `ResponseParser` has handled that shape all along. The new `ResponseParser::fields()` narrows a body to the string-keyed shape where a single resource is expected. **Who is affected:** only code that runs static analysis against these methods and relied on the narrower declaration; there is no signature or runtime change ([#176](https://github.com/zammad/zammad-api-client-php/issues/176)) +- **Breaking:** `PaginatedList::offsetGet()` and `offsetExists()` no longer return `null` / `false` for a non-integer offset. The class declares `ArrayAccess`, so those guards were unreachable under the declared types; with them gone, `$list['some-string']` now raises a `TypeError` rather than silently yielding nothing. **Who is affected:** anyone indexing a `PaginatedList` with a non-integer offset, which was never part of the contract ([#176](https://github.com/zammad/zammad-api-client-php/issues/176)) +- **Fixed:** header values passed through `$options['headers']` reached PSR-7's `withHeader()` unchecked, so a non-string value surfaced as an `InvalidArgumentException` raised from inside the transport layer rather than naming the caller's own input. Scalars are now coerced to strings and arrays mapped element-wise ([#176](https://github.com/zammad/zammad-api-client-php/issues/176)) - **Fixed:** the Symfony bundle fell back to `%env(ZAMMAD_TOKEN)%` with no default, so an application that registered the bundle without setting that variable got Symfony's `EnvNotFoundException: Environment variable not found: "ZAMMAD_TOKEN"` when it first resolved the client — an error about Symfony's env layer rather than about the missing credential. It now falls back through a `zammad.default_token` parameter, the same shape the URL already used, so the failure is the library's own `InvalidArgumentException: API token must not be empty.` — identical to what `LaravelServiceProvider` has always done. **Who is affected:** anyone relying on the previous exception type or message in a `catch` block or a health check ([#169](https://github.com/zammad/zammad-api-client-php/issues/169)) - Both framework bridges are verifiable by contributors. They are tested from separate Composer projects under `test/Bridge/Symfony/` and `test/Bridge/Laravel/`, each with its own `vendor/`, its own PHPStan run at level max and its own PHPUnit configuration. The Symfony suite loads, compiles and dumps a real `ContainerBuilder` — the path on which the bundle has broken before — and the Laravel suite registers the provider against a real `Illuminate\Foundation\Application`; both bridges go from 0 % to full line coverage. CI runs them against Symfony 6.4, 7.4 and 8.x and Laravel 12 and 13, and those ranges are stated in `suggest` and in the README. The split is not cosmetic: resolved in one lock file the two frameworks deadlock, because every Laravel release that tolerates PHPUnit 11 requires `symfony/http-kernel ^7` or newer. It also keeps both frameworks out of the root `require-dev`, so a plain `composer install` stays small. A `.gitattributes` with `export-ignore` rules additionally cuts the Composer dist archive roughly in half ([#169](https://github.com/zammad/zammad-api-client-php/issues/169)) - **Fixed:** the Symfony service IDs in the custom HTTP client guide. `@Symfony\Component\HttpClient\Psr18Client` and `@Nyholm\Psr7\Factory\Psr17Factory` are class names, not registered services, so copying that YAML failed the build. FrameworkBundle aliases `Psr\Http\Client\ClientInterface`, and the `nyholm/psr7` Flex recipe registers `nyholm.psr7.psr17_factory`. The fallback for installations without Flex, in that same section, then declared the factory under its class name while the handler referenced `nyholm.psr7.psr17_factory` — reproducing the very *"has a dependency on a non-existent service"* build error it was meant to avoid. It now declares the ID the handler asks for diff --git a/src/Bridge/SymfonyBundle.php b/src/Bridge/SymfonyBundle.php index 1c3cd1b..999bc9b 100644 --- a/src/Bridge/SymfonyBundle.php +++ b/src/Bridge/SymfonyBundle.php @@ -87,7 +87,7 @@ */ final class SymfonyBundle extends Bundle { - public function getContainerExtension(): ?ExtensionInterface + public function getContainerExtension(): ExtensionInterface { return new class implements ExtensionInterface { /** diff --git a/src/Core/Repository/PaginatedList.php b/src/Core/Repository/PaginatedList.php index f4a6a16..7e4e8d6 100644 --- a/src/Core/Repository/PaginatedList.php +++ b/src/Core/Repository/PaginatedList.php @@ -64,10 +64,6 @@ public function first(): ?DTOInterface /** @return T|null */ public function offsetGet(mixed $offset): mixed { - if (!is_int($offset)) { - return null; - } - $page = (int) floor($offset / $this->perPage) + 1; $index = $offset % $this->perPage; @@ -78,10 +74,6 @@ public function offsetGet(mixed $offset): mixed public function offsetExists(mixed $offset): bool { - if (!is_int($offset)) { - return false; - } - $page = (int) floor($offset / $this->perPage) + 1; if (!isset($this->pageCache[$page])) { From 7afcf42d971eb0d31579c8f95f57e144bdc157ea Mon Sep 17 00:00:00 2001 From: Rene Reimann Date: Tue, 3 Nov 2026 07:48:09 +0100 Subject: [PATCH 5/6] Build: pin PHPStan to one exact version in all three analysed projects (#176) --- composer.json | 2 +- test/Bridge/Laravel/composer.json | 2 +- test/Bridge/Symfony/composer.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 34884a7..f3d2161 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "require-dev": { "phpunit/phpunit": "^11.0", "mockery/mockery": "^1.6", - "phpstan/phpstan": "^2.2", + "phpstan/phpstan": "2.3.4", "league/openapi-psr7-validator": "^0.22", "squizlabs/php_codesniffer": "^4.0" }, diff --git a/test/Bridge/Laravel/composer.json b/test/Bridge/Laravel/composer.json index 999fcf4..f719aee 100644 --- a/test/Bridge/Laravel/composer.json +++ b/test/Bridge/Laravel/composer.json @@ -9,7 +9,7 @@ "require-dev": { "zammad/zammad-api-client-php": "@dev", "phpunit/phpunit": "^11.0", - "phpstan/phpstan": "^2.2", + "phpstan/phpstan": "2.3.4", "laravel/framework": "^12.0 || ^13.0" }, "autoload-dev": { diff --git a/test/Bridge/Symfony/composer.json b/test/Bridge/Symfony/composer.json index 01e0e98..72490c4 100644 --- a/test/Bridge/Symfony/composer.json +++ b/test/Bridge/Symfony/composer.json @@ -9,7 +9,7 @@ "require-dev": { "zammad/zammad-api-client-php": "@dev", "phpunit/phpunit": "^11.0", - "phpstan/phpstan": "^2.2", + "phpstan/phpstan": "2.3.4", "symfony/config": "^6.4 || ^7.0 || ^8.0", "symfony/dependency-injection": "^6.4 || ^7.0 || ^8.0", "symfony/http-kernel": "^6.4 || ^7.0 || ^8.0" From c54225fae603967e013f7dd6467951f84a189336 Mon Sep 17 00:00:00 2001 From: Rene Reimann Date: Tue, 3 Nov 2026 07:48:16 +0100 Subject: [PATCH 6/6] Transport: reject header values PSR-7 cannot send instead of dropping them (#176) --- src/Core/Transport/RequestHandler.php | 66 ++++++++++++++++++++++----- test/Unit/Core/RequestHandlerTest.php | 47 +++++++++++++++++++ 2 files changed, 101 insertions(+), 12 deletions(-) diff --git a/src/Core/Transport/RequestHandler.php b/src/Core/Transport/RequestHandler.php index 67b9fde..82535fe 100644 --- a/src/Core/Transport/RequestHandler.php +++ b/src/Core/Transport/RequestHandler.php @@ -334,18 +334,10 @@ private function dispatch(string $method, string $uri, array $options): Response if (isset($options['headers']) && is_array($options['headers'])) { foreach ($options['headers'] as $name => $value) { - // PSR-7 accepts string|array. Passing anything else - // raises an InvalidArgumentException from inside the message - // implementation, which names the transport rather than the - // caller's own input, so coerce what can be coerced. - if (is_scalar($value)) { - $request = $request->withHeader((string) $name, (string) $value); - } elseif (is_array($value)) { - $request = $request->withHeader( - (string) $name, - array_map(static fn(mixed $v): string => is_scalar($v) ? (string) $v : '', $value), - ); - } + $request = $request->withHeader( + (string) $name, + self::headerValue((string) $name, $value), + ); } } @@ -371,6 +363,56 @@ private function dispatch(string $method, string $uri, array $options): Response throw $this->mapError($status, $uri, $response); } + /** + * Normalises one entry of the `headers` option into what PSR-7 accepts: + * a string, or a list of strings for a header sent more than once. + * + * Scalars are converted, since headers are often built from config values + * that arrive as ints or bools. Anything else — an object, a nested array, + * null — is rejected rather than dropped or flattened to an empty string: + * a header that silently goes missing, or silently arrives blank, surfaces + * much later as a puzzling API response instead of as a failed call here. + * + * @return string|list + * + * @throws InvalidArgumentException If $value is neither a scalar nor a list of scalars. + */ + private static function headerValue(string $name, mixed $value): string|array + { + if (is_scalar($value)) { + return (string) $value; + } + + if (!is_array($value)) { + throw self::unsupportedHeaderValue($name, $value); + } + + $values = []; + + foreach ($value as $element) { + if (!is_scalar($element)) { + throw self::unsupportedHeaderValue($name, $element); + } + + $values[] = (string) $element; + } + + return $values; + } + + /** + * Names the offending header and the type that was passed, so the message + * points at the caller's input rather than at this transport's internals. + */ + private static function unsupportedHeaderValue(string $name, mixed $value): InvalidArgumentException + { + return new InvalidArgumentException(sprintf( + 'Header "%s" must be a string or a list of strings, got %s.', + $name, + get_debug_type($value), + )); + } + /** * Maps an unsuccessful HTTP response to its corresponding domain exception. */ diff --git a/test/Unit/Core/RequestHandlerTest.php b/test/Unit/Core/RequestHandlerTest.php index cae047f..545d2b1 100644 --- a/test/Unit/Core/RequestHandlerTest.php +++ b/test/Unit/Core/RequestHandlerTest.php @@ -426,6 +426,53 @@ public function testPerRequestHeaderOverridesConfiguredAuth(): void ); } + public function testScalarHeaderValuesAreConvertedToStrings(): void + { + $this->httpClient->response = new Response(200, [], '{}'); + + $this->handler->request('GET', 'tickets', ['headers' => ['X-Retry-Count' => 3]]); + + self::assertSame('3', $this->httpClient->lastRequest?->getHeaderLine('X-Retry-Count')); + } + + public function testArrayHeaderValueIsSentAsRepeatedHeader(): void + { + $this->httpClient->response = new Response(200, [], '{}'); + + $this->handler->request('GET', 'tickets', ['headers' => ['X-Tag' => ['a', 'b']]]); + + self::assertSame(['a', 'b'], $this->httpClient->lastRequest?->getHeader('X-Tag')); + } + + /** + * These used to be dropped (top level) or flattened to an empty string + * (inside an array), which turned a caller's mistake into a request that + * looked fine and behaved wrongly. + * + * @param mixed $value + */ + #[DataProvider('unsupportedHeaderValues')] + public function testUnsupportedHeaderValuesAreRejected(mixed $value, string $expectedType): void + { + $this->httpClient->response = new Response(200, [], '{}'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage( + sprintf('Header "X-Bad" must be a string or a list of strings, got %s.', $expectedType), + ); + + $this->handler->request('GET', 'tickets', ['headers' => ['X-Bad' => $value]]); + } + + /** @return iterable */ + public static function unsupportedHeaderValues(): iterable + { + yield 'object' => [new \stdClass(), 'stdClass']; + yield 'null' => [null, 'null']; + yield 'nested array' => [[['a']], 'array']; + yield 'object inside array' => [[new \stdClass()], 'stdClass']; + } + public function testCredentialsOverPlainHttpAreRejected(): void { $this->expectException(InvalidArgumentException::class);