Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash for property with case difference and without docblock #222

Closed
wants to merge 1 commit into from

Conversation

jorrit
Copy link
Contributor

@jorrit jorrit commented Jan 30, 2024

7dc32f5 introduced a possible crash under a very specific situation:

  1. A property is matched with a case difference.
  2. The property has no docblock

In this situation, the following crash occurs:

Error: Call to a member function hasMethod() on bool[0] (caught throwable) at /var/www/admin/vendor/netresearch/jsonmapper/src/JsonMapper.php line 564.

The reason is that inside inspectProperty(), $class is null when the property is not found by exact name. Later, the code attempts to find the property case-insensitively, but then $class is left false. This leads to a crash in new code introduced by 7dc32f5.

The attached patch contains a test that fails when the change in JsonMapper.php is not applied.

cweiske pushed a commit that referenced this pull request Jan 31, 2024
@cweiske cweiske closed this in 132c75c Jan 31, 2024
@cweiske
Copy link
Owner

cweiske commented Jan 31, 2024

Thank you for the quick patch.

The unit test did actually not fail because it did not have casing differences in the test data, so I fixed it.
I also split the commit into two, so that the failing test can be run separate from the fix. (Which makes it easier to check if the test indeed fails, and makes finding the culprit very easy)

Released with 4.4.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.

None yet

2 participants