Skip to content

chore: bump interaction-design-foundation/coding-standard to ^1.0-RC1 - #76

Open
alies-dev wants to merge 3 commits into
mainfrom
update-coding-standard
Open

chore: bump interaction-design-foundation/coding-standard to ^1.0-RC1#76
alies-dev wants to merge 3 commits into
mainfrom
update-coding-standard

Conversation

@alies-dev

Copy link
Copy Markdown
Member

Bump coding-standard to ^1.0-RC1 and apply resulting php-cs-fixer changes.

alies-dev and others added 3 commits March 6, 2026 14:11
Replace standalone friendsofphp/php-cs-fixer and squizlabs/php_codesniffer
with the shared IxDF coding standard package. Update .php-cs-fixer.php to
use the shared Config factory and phpcs.xml to reference IxDFCodingStandard.
Auto-fixed violations detected by the new IxDF coding standard.
Kept HttpClient and AbstractService::boot() non-final to preserve
testability and extensibility.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the package's coding standards to use the Interaction Design Foundation coding standard, resulting in widespread formatting and styling changes across the codebase. However, several critical issues were identified in the review: making the boot() method final in AbstractService will cause a PHP Fatal Error as subclasses override it; using mb_substr in HttpClient to split headers and body based on a byte-count offset can lead to data corruption; and using mb_trim will cause fatal errors on PHP versions prior to 8.4.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

public function boot(): void
{
}
final public function boot(): void {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Making the boot() method final in the abstract class AbstractService will cause a PHP Fatal Error because subclasses like MaxMindDatabase, IPGeoLocation, and IPApi override this method to perform their own initialization.

Please remove the final keyword from the boot() method.

    public function boot(): void {}

Comment on lines +96 to +97
$header = mb_substr($response, 0, $header_size);
$body = mb_substr($response, $header_size);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Using mb_substr here is incorrect and can lead to data corruption or malformed HTTP parsing.

curl_getinfo($curl, CURLINFO_HEADER_SIZE) returns the header size in bytes, not characters. substr is a byte-safe function, whereas mb_substr is character-encoding-aware. If the HTTP response headers or the beginning of the body contain any multibyte characters, mb_substr will split the string at the wrong byte offset, resulting in corrupted headers or body.

Please revert this change to use the byte-safe substr function.

        $header = substr($response, 0, $header_size);
        $body = substr($response, $header_size);


if (count($header) === 2) {
$result[$header[0]] = trim($header[1]);
$result[$header[0]] = mb_trim($header[1]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The mb_trim function was introduced in PHP 8.4. Since this package specifies a minimum PHP version of ^8.2 in composer.json, calling mb_trim directly will result in a Fatal Error: Call to undefined function mb_trim() on PHP 8.2 and 8.3 environments.

Since HTTP headers are ASCII-compatible, the standard trim function is perfectly safe and fully compatible. Please revert this to trim.

                $result[$header[0]] = trim($header[1]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant