Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .php-cs-fixer.dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
'@PER-CS3x0' => true,
'@PER-CS3x0:risky' => true,
'@PHPUnit100Migration:risky' => true,
'declare_strict_types' => true,
'linebreak_after_opening_tag' => true,
'ordered_imports' => true,
'no_empty_phpdoc' => true,
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine;

/**
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/AbstractApi.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use InvalidArgumentException;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/Attachment.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
4 changes: 3 additions & 1 deletion src/Redmine/Api/CustomField.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down Expand Up @@ -149,7 +151,7 @@ public function listing($forceUpdate = false, array $params = [])
{
@trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . self::class . '::listNames()` instead.', E_USER_DEPRECATED);

return $this->doListing($forceUpdate, $params);
return $this->doListing((bool) $forceUpdate, $params);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/Group.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/Issue.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
4 changes: 3 additions & 1 deletion src/Redmine/Api/IssueCategory.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down Expand Up @@ -175,7 +177,7 @@ public function listing($project, $forceUpdate = false)
{
@trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . self::class . '::listNamesByProject()` instead.', E_USER_DEPRECATED);

return $this->doListing($project, $forceUpdate);
return $this->doListing($project, (bool) $forceUpdate);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/IssuePriority.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/IssueRelation.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
4 changes: 3 additions & 1 deletion src/Redmine/Api/IssueStatus.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down Expand Up @@ -148,7 +150,7 @@ public function listing($forceUpdate = false)
{
@trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . self::class . '::listNames()` instead.', E_USER_DEPRECATED);

return $this->doListing($forceUpdate);
return $this->doListing((bool) $forceUpdate);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/Membership.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/News.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
4 changes: 3 additions & 1 deletion src/Redmine/Api/Project.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use InvalidArgumentException;
Expand Down Expand Up @@ -170,7 +172,7 @@ public function listing($forceUpdate = false, $reverse = true, array $params = [
{
@trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . self::class . '::listNames()` instead.', E_USER_DEPRECATED);

return $this->doListing($forceUpdate, $reverse, $params);
return $this->doListing((bool) $forceUpdate, $reverse, $params);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/Query.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/Role.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/Search.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/TimeEntry.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/TimeEntryActivity.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
4 changes: 3 additions & 1 deletion src/Redmine/Api/Tracker.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down Expand Up @@ -147,7 +149,7 @@ public function listing($forceUpdate = false)
{
@trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . self::class . '::listNames()` instead.', E_USER_DEPRECATED);

return $this->doListing($forceUpdate);
return $this->doListing((bool) $forceUpdate);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/Redmine/Api/User.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down Expand Up @@ -168,7 +170,7 @@ public function listing($forceUpdate = false, array $params = [])
{
@trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . self::class . '::listLogins()` instead.', E_USER_DEPRECATED);

return $this->doListing($forceUpdate, $params);
return $this->doListing((bool) $forceUpdate, $params);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/Redmine/Api/Version.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down Expand Up @@ -177,7 +179,7 @@ public function listing($project, $forceUpdate = false, $reverse = true, array $
{
@trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . self::class . '::listNamesByProject()` instead.', E_USER_DEPRECATED);

return $this->doListing($project, $forceUpdate, $reverse, $params);
return $this->doListing($project, (bool) $forceUpdate, $reverse, $params);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Api/Wiki.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Api;

use Redmine\Client\Client;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Client/Client.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Client;

use Redmine\Api;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Client/ClientApiTrait.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Client;

use Redmine\Api;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Exception.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine;

use Throwable;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Exception/ClientException.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Exception;

use Exception;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Exception/InvalidApiNameException.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Exception;

use InvalidArgumentException;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Exception/InvalidParameterException.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Exception;

use InvalidArgumentException;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Exception/MissingParameterException.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Exception;

use InvalidArgumentException;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Exception/SerializerException.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Exception;

use Redmine\Exception as RedmineException;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Serializer/JsonSerializer.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Serializer;

use JsonException;
Expand Down
2 changes: 2 additions & 0 deletions src/Redmine/Serializer/PathSerializer.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Serializer;

use Stringable;
Expand Down
16 changes: 9 additions & 7 deletions src/Redmine/Serializer/XmlSerializer.php
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
<?php

declare(strict_types=1);

namespace Redmine\Serializer;

use Exception;
use JsonException;
use Redmine\Exception\SerializerException;
use SimpleXMLElement;
use Stringable;
use Throwable;

/**
* XmlSerializer.
Expand Down Expand Up @@ -75,7 +77,7 @@ private function deserialize(string $encoded): void

try {
$this->deserialized = new SimpleXMLElement($encoded);
} catch (Throwable $e) {
} catch (Exception $e) {
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Narrowing catch from Throwable to Exception may let TypeError escape uncaught.

With declare(strict_types=1), type mismatches now throw TypeError which extends Error, not Exception. This catch block will no longer capture:

  • TypeError from strict type violations during XML processing
  • Other Error subclasses that were previously caught

Call sites like AbstractApi::getResponseAsArray() and various API create()/update() methods expect SerializerException but will now receive raw TypeError if caller-provided data has incorrect types.

Proposed fix: Keep catching Throwable
-        } catch (Exception $e) {
+        } catch (\Throwable $e) {

Apply to both catch blocks at lines 80 and 129.

Also applies to: 129-129

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Redmine/Serializer/XmlSerializer.php` at line 80, The catch blocks in
XmlSerializer (the catch clauses currently written as "catch (Exception $e)")
are too narrow and will let TypeError/other Error subclasses escape under
strict_types; update both catch clauses in the XmlSerializer class (the blocks
that currently catch Exception and wrap/rethrow as SerializerException) to catch
Throwable instead, preserving the existing handling logic (wrap/throw
SerializerException with the same message/context) so TypeError and other Errors
are captured and converted to SerializerException as callers expect.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to catch only Exceptions thrown by SimpleXMLElement, not TypeError

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Art4, understood — that's a valid and intentional design choice. Since deserialize() is typed as string, any TypeError from a wrong argument type would be thrown at the call boundary (before entering the try block), not by SimpleXMLElement itself. The constructor only throws Exception-derived errors for malformed XML content, so catching Exception is both correct and sufficient here. I'll dismiss my concern.


✏️ Learnings added
Learnt from: Art4
Repo: kbsali/php-redmine-api PR: 474
File: src/Redmine/Serializer/XmlSerializer.php:80-80
Timestamp: 2026-04-10T22:04:24.727Z
Learning: In `src/Redmine/Serializer/XmlSerializer.php` (php-redmine-api), the catch blocks around `new SimpleXMLElement(...)` intentionally catch `Exception` (not `Throwable`). `TypeError` is expected to surface at the call boundary due to strict type hints, not inside the `SimpleXMLElement` constructor. The maintainer does not want `TypeError` swallowed and wrapped in `SerializerException`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

$errors = [];
$code = $e->getCode();

Expand Down Expand Up @@ -123,8 +125,8 @@ private function denormalize(array $normalized): void
$prevSetting = libxml_use_internal_errors(true);

try {
$this->deserialized = $this->createXmlElement($rootElementName, $this->normalized[$rootElementName]);
} catch (Throwable $e) {
$this->deserialized = $this->createXmlElement((string) $rootElementName, $this->normalized[$rootElementName]);
} catch (Exception $e) {
$errors = [];

foreach (libxml_get_errors() as $error) {
Expand Down Expand Up @@ -196,7 +198,7 @@ private function addChildToXmlElement(SimpleXMLElement $xml, $k, $v): void
$array = $xml->addChild($k, '');
$array->addAttribute('type', 'array');
foreach ($v as $id) {
$array->addChild($specialParams[$k], $id);
$array->addChild($specialParams[$k], (string) $id);
}
} elseif (is_array($v)) {
$array = $xml->addChild($k, '');
Expand Down Expand Up @@ -231,7 +233,7 @@ private function attachCustomFieldXML(SimpleXMLElement $xml, array $fields, stri
$_field->addAttribute('field_format', $field['field_format']);
}
if (isset($field['id'])) {
$_field->addAttribute('id', $field['id']);
$_field->addAttribute('id', strval($field['id']));
}
if (array_key_exists('value', $field) && is_array($field['value'])) {
$_field->addAttribute('multiple', 'true');
Expand All @@ -243,7 +245,7 @@ private function attachCustomFieldXML(SimpleXMLElement $xml, array $fields, stri
} else {
$_values->addAttribute('type', 'array');
foreach ($field['value'] as $val) {
$_values->addChild('value', $val);
$_values->addChild('value', (string) $val);
}
}
} else {
Expand Down
2 changes: 2 additions & 0 deletions tests/Unit/Api/AbstractApiTest.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Redmine\Tests\Unit\Api;

use InvalidArgumentException;
Expand Down
Loading
Loading