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

[PHP8] check if params given to API are really an array #35779

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

individual-it
Copy link
Contributor

Summary

A request that comes into this code but is not a JSON object but only a string will show only a Warning in PHP7 but in PHP8 count() will throw an exception that then might result in a HTTP Status 500 and log lines like "message": "count(): Argument #1 ($value) must be of type Countable|array, string given in file '/home/artur/www/nextcloud-server/lib/private/AppFramework/Http/Request.php' line 434",

This PR makes sure that $params is an array and if not set to null

Checklist

@individual-it
Copy link
Contributor Author

/backport to stable24

@individual-it individual-it changed the title catch TypeError that can be thrown by catch() [PHP8] check if params given to API are really an array Dec 15, 2022
Signed-off-by: Artur Neumann <artur@jankaritech.com>
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good!

@ChristophWurst ChristophWurst added bug 3. to review Waiting for reviews labels Dec 15, 2022
@ChristophWurst
Copy link
Member

@nextcloud nextcloud deleted a comment from individual-it Dec 15, 2022
@nextcloud nextcloud deleted a comment from individual-it Dec 15, 2022
@ChristophWurst
Copy link
Member

/backport to stable25

@individual-it
Copy link
Contributor Author

we are still supporting 22 and 23 in https://github.com/nextcloud/integration_openproject/ and that bug makes our API send too many 500
maybe we have to change the min supported version

@szaimen szaimen added this to the Nextcloud 26 milestone Dec 16, 2022
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81
Copy link
Member

failing tests unrelated (will be solved with #35789)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants