Skip to content

Fix: Ensure unpacked data is an array before using array_key_exists() - #39

Merged
huangdijia merged 6 commits into
friendsofhyperf:mainfrom
guandeng:hotfix/fix
Oct 9, 2025
Merged

Fix: Ensure unpacked data is an array before using array_key_exists()#39
huangdijia merged 6 commits into
friendsofhyperf:mainfrom
guandeng:hotfix/fix

Conversation

@guandeng

@guandeng guandeng commented Nov 22, 2024

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes

    • Safer handling of malformed or unexpected JSON-RPC responses to prevent silent failures.
    • Improved error messages now include the raw response to aid troubleshooting.
    • Normal successful responses continue to return results without behavior changes.
  • Chores

    • Composer configuration updated to allow a package-normalization plugin for build/tooling consistency.

范冠登 and others added 2 commits November 22, 2024 19:19
Throws a RecvFailedException if the unpacked data is not an array, improving error handling for invalid responses.
@coderabbitai

coderabbitai Bot commented Oct 9, 2025

Copy link
Copy Markdown

Warning

Rate limit exceeded

@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 3 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between a4326ec and 0a50458.

📒 Files selected for processing (1)
  • src/Client.php (1 hunks)

Walkthrough

Adds a defensive cast and validation for JSON-RPC responses in src/Client.php to ensure decoded data is an array and throws RecvFailedException including the raw response when not. Also updates composer.json to add an allow-plugins block enabling ergebnis/composer-normalize.

Changes

Cohort / File(s) Summary of changes
Client JSON-RPC response handling
src/Client.php
Cast decoded response to array, capture raw response, add check to throw RecvFailedException if decoded data is not an array; unchanged return of result and preserved non-JSONRPC error handling path.
Composer config: allow plugin
composer.json
Add config.allow-plugins entry enabling ergebnis/composer-normalize (keeps sort-packages true).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Caller
    participant Client
    participant Transport
    participant JSON as JSON Decoder

    Caller->>Client: request(params)
    Client->>Transport: send RPC request
    Transport-->>Client: raw response (string) $ret
    Client->>JSON: decode($ret)
    JSON-->>Client: decoded value

    alt decoded is array and contains "result"
        Client-->>Caller: return decoded["result"]
    else decoded not an array
        note right of Client #FFDDAA: New validation — cast and check
        Client-->>Caller: throw RecvFailedException(msg includes raw $ret)
    else non-JSONRPC error path
        Client-->>Caller: existing error handling
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibbled the bytes with whiskered care,
Found arrays or tossed a stern glare.
If shapes look wrong, I thump—“Nope!”—
With raw reply in my envelope.
When results are right, I hop along,
Returning data swift and strong. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely summarizes the primary change by specifying that the patch adds a check to ensure unpacked data is an array before using array_key_exists, which accurately reflects the implementation of casting and error handling in the Client.php update.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3598c14 and b56d58b.

📒 Files selected for processing (1)
  • src/Client.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Client.php (6)
src/Functions.php (1)
  • with (111-114)
src/Contract/PackerInterface.php (1)
  • unpack (18-18)
src/Packer/JsonEofPacker.php (1)
  • unpack (41-44)
src/Packer/GrpcPacker.php (1)
  • unpack (23-26)
src/Packer/JsonLengthPacker.php (1)
  • unpack (57-66)
src/Exception/RecvFailedException.php (1)
  • RecvFailedException (14-16)

Comment thread src/Client.php Outdated
@huangdijia
huangdijia merged commit eecfabe into friendsofhyperf:main Oct 9, 2025
1 check was pending
huangdijia added a commit that referenced this pull request Oct 9, 2025
…#39)

* Fix: Ensure unpacked data is an array before using array_key_exists()

* Add validation for unpacked data in Client

Throws a RecvFailedException if the unpacked data is not an array, improving error handling for invalid responses.

* Fix: Ensure unpacked data is treated as an array in Client

* chore: Add allow-plugins configuration for composer-normalize

* Fix: Ensure unpacked data is treated as an array and improve error handling in Client

* Fix: Improve error message for invalid data in ServerException

---------

Co-authored-by: 范冠登 <guandeng@addcn.com>
Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
huangdijia added a commit that referenced this pull request Oct 9, 2025
…#39)

* Fix: Ensure unpacked data is an array before using array_key_exists()

* Add validation for unpacked data in Client

Throws a RecvFailedException if the unpacked data is not an array, improving error handling for invalid responses.

* Fix: Ensure unpacked data is treated as an array in Client

* chore: Add allow-plugins configuration for composer-normalize

* Fix: Ensure unpacked data is treated as an array and improve error handling in Client

* Fix: Improve error message for invalid data in ServerException

---------

Co-authored-by: 范冠登 <guandeng@addcn.com>
Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
huangdijia added a commit that referenced this pull request Oct 9, 2025
…#39)

* Fix: Ensure unpacked data is an array before using array_key_exists()

* Add validation for unpacked data in Client

Throws a RecvFailedException if the unpacked data is not an array, improving error handling for invalid responses.

* Fix: Ensure unpacked data is treated as an array in Client

* chore: Add allow-plugins configuration for composer-normalize

* Fix: Ensure unpacked data is treated as an array and improve error handling in Client

* Fix: Improve error message for invalid data in ServerException

---------

Co-authored-by: 范冠登 <guandeng@addcn.com>
Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
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.

2 participants